Support JSON as source for parameterized tests arguments

[nipafx]

Implement an ArgumentConverter that parses JSON and an ArgumentSource that parses JSON files and passes them as arguments to a parameterized test.

This is pretty straightforward for a single file and parameter:

@ParameterizedTest
@JsonSource("customers.json")
void testCustomer(Customer customer) { /*...*/ }

It's a little unclear how to parse files for two parameters, though. There could be one file per parameter...

@ParameterizedTest
@JsonSource({ "customers.json", "contracts.json" })
void testCustomer(Customer customer, Contract contract) { /*...*/ }

... but then customer/contract pairs would be far apart and hard to identify by looking at the files.

Alternatively, there could be a top-level container...

{
	customer: { /*...*/ }
	contract: { /*...*/ }
}

... and the parser takes them apart when resolving parameter:

@ParameterizedTest
@JsonSource({ "customers-contracrs.json" })
void testCustomer(Customer customer, Contract contract) { /*...*/ }

[NPException]

Since this issue is more than a year old now, which of the suggested approaches would be preferred?
I personally would prefer the "single-file-with-top-level-container" variant. I feel one file per parameter could lead people to an unnecessarily large number of small JSON files.

[aepfli]

i like the idea of a top level container, as it is quiet intuitive, and the testdata always belongs together. furthermore with the toplevel container in an array, we could also define a default object outside of the container, with default values. Sometimes you need all to have some data, and it needs to be the same. So we could utilize this, without duplicating data. just an idea.

{
	defaults: {
		customer: {},
		contracts: {}
	}
	data: [{
		customer: {},
		contracts: {}
	}]
}

But i think multiple files are impractical, as you do not see data belonging together, and you always should ensure that they do contain the same amount of datasets anyways.

[nipafx]

JSON-parsing should not be reimplemented for this issue, but also JUnit Pioneer doesn't want to take on additional run-time dependencies, which means this issue can not be resolved within this project. If a PR is offered for it, we will not merge it but instead take the new sources, documentation, etc. and spin it out into its own project. (That might of course take some time, so don't pick this up if you're looking for a quick win. 😉 )

[filiphr]

I'd like to give this a go. What is my current thinking about this is the following.

We would expect the json files to contain array of values or a single object:

Array of values:

[
    { name: 'Luke', height: 172 },
    { name: 'Yoda', height: 66 }
]

Single object:

{
    name: 'Luke',
    height: 172
}

Then using @JsonSource we would support the following:

@ParameterizedTest
@JsonSource("customers.json")
void testCustomer(Customer customer) { /*...*/ }

@ParameterizedTest
@JsonSource("customers.json")
void testCustomerFields(String name, int height) { /*...*/ }

Basically if you have one parameter we try to map into that object. If you have multiple parameters then it would be treated like you want to deconstruct that object.

If you have multiple files then it would be treated as CsvFileSource from JUnit Jupiter, basically using all files to create tests for them.

But i think multiple files are impractical, as you do not see data belonging together, and you always should ensure that they do contain the same amount of datasets anyways.

I agree with @aepfli about this one. Trying to combine different files makes it more complex and I am not sure that it is worth it.

Btw regarding JUnit Pioneer doesn't want to take on additional run-time dependencies. What about doing it with an optional dependency and having support for using different json parsing tools. Something similar to what JsonUnit does with its Converter. In any case, I would work in such a way that you can provide your implementation and have different ways of parsing the JSON.

[nipafx]

I like your approach, @filiphr. Having a single JS object per invocation makes more sense than what I proposed above. I also like deconstruction in testCustomerFields. 👍🏾

Regarding @aepfli's proposal for default data, I think we should not implement that in the first try but that raises the question whether we already want to prepare for such a scenario by requiring the actual data on a top-level data field. That may be needlessly cumbersome for the 9x% of cases that only use data, though. To address this, we could give @JsonSource an optional argument that specifies the attribute name that contains the value or value array, e.g:

ParameterizedTest
@JsonSource(value = "customers.json", data = "the-customers")
void testCustomer(Customer customer) { /*...*/ }

{
	"the-customers": [
		{ ... }
	]
}

It could default to a constant ROOT that specifies the data is in the document's root.

[nipafx]

The thing with the dependencies is a bit more complicated. I'd need to look into your proposal at another time. But that wouldn't touch the code you write, so you can already go ahead with that. Let us know when you do. :)

[filiphr]

I like the data idea approach.

Let us know when you do. :)

I went ahead and did PR #492. Documentation is missing, but I wanted to first get your feeling about the proposal before diving into docs.

[nipafx]

As I see it, we essentially offer three ways to provide JSON:

• inline
• from files
• from class path resources

At the moment #492 splits this across two annotations and two provider classes, just like Jupiter does with the CSV source. I think there's a more uniform approach possible. First and foremost, I think there should only be one "core provider" that, given some JSON (node?) and other arguments (location?) turns JSON into Jupiter's Arguments.

As to annotations, I see three options:

• one per use case: @JsonSource, @JsonFileSource, @JsonClassPathReSource (lol) - has the advantage that the value attribute can capture the essential argument (e.g. inline JSON or file names) without defaults
• as it is now, i.e. parallel to CSV sources
• one in total: @JsonSource with attributes inline, files, resources

Jupiter's machinery requires there to be one AnnotationConsumer per annotation, so there will be as many implementations of those as we have annotations. If that's more than one, they should probably extend the then-abstract "core provider" mentioned above.

We usually try to stay close to Jupiter and one could argue that class path resources are also files and should thus be lumped into the same annotation, but I find that latter argument somewhat unconvincing. The former holds more water. Then again, if we think we can do better, we should try.

[filiphr]

I thought that we were waiting for other people from the JUnit Pioneer team to respond to the last comment.

I personally prefer to be as close as possible to JUnit Jupiter, since it reduces the complexity and it keeps it simple for the users to use the annotations. JUnit Jupiter currently has:

• @CsvSource with value and the recently added textBlock
• @CsvFileSource with resources and files

Therefore for the JSON support I think that we need to have the following:

• @JsonSource only with value - This will also support text blocks, so nothing special needs to be done for this
• @JsonFileSource with resources and files

Apart from the argument that this is similar to Jupiter, the @JsonSource with value makes it readable to write something like:

@JsonSource("""[
         { name: 'Luke', height: 172 },
         { name: 'Yoda', height: 66 }
        ]
""")

or even something like (if text blocks are not OK)

@JsonSource({
         "{ name: 'Luke', height: 172 }",
         "{ name: 'Yoda', height: 66 }"
        })

Jupiter's machinery requires there to be one AnnotationConsumer per annotation, so there will be as many implementations of those as we have annotations. If that's more than one, they should probably extend the then-abstract "core provider" mentioned above.

Sharing the common logic in an abstract "core provider" is completely fine and makes sense to me.

[nipafx]

I 100% agree with your @JsonSource argument: It should accept a string (or array thereof - that looks good, too!) as value, so the attribute doesn't need to be specified.

Regarding two-vs-three annotations, I definitely like three annotations, i.e. one per use case, better. It seems cleaner and makes those easier to use as well (because they can rely on value). First question: Do you agree?

Then of course, there's the symmetry with JUnit Jupiter, which is definitely nice to have as it makes it easier for users to adopt these features. Weighing ease-of-learning (two annotations) against ease-of-use (three annotations), I lean towards the latter and would go with three annotations. Second question: What do you think?

If both answers are on line with mine, you can just go ahead and refactor accordingly. Otherwise let's fight discuss.