Support JSON as source for parametrized tests arguments

[filiphr]

This is an initial implementation fro #101. It currently only supports Jackson 2 for Json Parsing. However, it should be fairly simple to add support for different parsers.

I know the policy about no runtime dependencies. Therefore, I tried using Optional Dependencies from Gradle and its feature variants. If this is something interesting we can look into doing this with different source sets in order for complete isolation (currently the Checkstyle import-control is handling this).

There is still documentation missing. However, if you like the approach I will work on the documentation.

Proposed commit message:

Support for JSON as source for parametrized tests

Closes: #101
PR: #492

---

PR checklist

The following checklist shall help the PR's author, the reviewers and maintainers to ensure the quality of this project.
It is based on our contributors guidelines, especially the "writing code" section.
It shall help to check for completion of the listed points.
If a point does not apply to the given PR's changes, the corresponding entry can be simply marked as done.

Documentation (general)

• There is documentation (Javadoc and site documentation; added or updated)
• There is implementation information to describe why a non-obvious source code / solution got implemented
• Site documentation has its own .adoc file in the docs folder, e.g. docs/report-entries.adoc
• Only one sentence per line (especially in .adoc files)
• Javadoc uses formal style, while sites documentation may use informal style

Documentation (new extension)

• The docs/docs-nav.yml navigation has an entry for the new extension
• The package-info.java contains information about the new extension

Code

• Code adheres to code style, naming conventions etc.
• Successful tests cover all changes
• There are checks which validate correct / false usage / configuration of a functionality and there are tests to verify those checks
• Tests use AssertJ or our own PioneerAssert (which are based on AssertJ)

Contributing

• A prepared commit message exists
• The list of contributions inside README.md mentions the new contribution (real name optional)

---

I hereby agree to the terms of the JUnit Pioneer Contributor License Agreement.

[Bukama]

I would like to pass this to the other maintainers cause of the dependency thing and changes in the build script

[Michael1993]

Pioneer actually has a module-info.java and a modular build, so you need to update that if you want the build to succeed. You can find it at src/main/java/module/module-info.java.

(package com.fasterxml.jackson.databind is declared in module com.fasterxml.jackson.databind, but module org.junitpioneer does not read it)

[nipafx]

From just looking at the tests, this seems to be exactly what we had in mind. Good job! I think it makes sense to pursue this further. 👍🏾

[filiphr]

Pioneer actually has a module-info.java and a modular build, so you need to update that if you want the build to succeed. You can find it at src/main/java/module/module-info.java.

Thanks @Michael1993, I've updated the file. The modular build is now working correctly.

I've taken this a bit further, added some documentation, and did some improvements;

• Support for using @Param for single method arguments, by extracting only one specific element from it
• Support for deconstructing into complex types

[nipafx]

Thank you for this PR, @filiphr. You've seen my streams, you know how much time management challenges me. I will take a closer look at this, but it will take some more time before I do.

Also, we need to resolve #502 before we can merge/move this.

[nipafx]

This looks really good, I think we can merge it "soon" (modulo my very long latency). A few comments below.

Also, if I'm not missing something, this "only" addresses part of #101. The ArgumentConverter and a source that parses inline JSON (as opposed to from a file) are also mentioned there. If we close #101, we need to open new issues for those. (If we still want to have them.)

[filiphr]

This looks really good, I think we can merge it "soon" (module my very long latency).

Thanks for the review. I've applied the feedback and have answered some of your comments. I am not in a rush (as you can see I have a long latency as well).

Also, if I'm not missing something, this "only" addresses part of #101. The ArgumentConverter and a source that parses inline JSON (as opposed to from a file) are also mentioned there. If we close #101, we need to open new issues for those. (If we still want to have them.)

Indeed this solves only a part of #101, the one where data is read from a classpath resource or a file. Reading from an inline JSON would be quite trivial as well. I'll try to add it as part of this PR as well.

I am not sure about the ArgumentConverter though, what would you expect from it? Currently we extract the properties in the types that they are in the JSON (string, number, boolean, etc.)

[filiphr]

By the way, I forgot to mention, I watched the stream afterwards to see your thought process when reviewing this 😄 . Couldn't be there when you were doing it live

[nipafx]

Looking good, but we found some things still left to do:

• whatever we decide on the issue
• support new Cartesian tests instead of the old approach
• I still need to wrap my head around Discuss approach to optional dependencies #502 (this PR currently uses Gradle's feature variants).

[nipafx]

This error case comes up when users haven't configured a (supported) JSON parse, so we need to make sure, it's visible. Both Maven and Gradle will cut the error message very short, so it's unclear what the problem is.

Some options:

• throw NoJsonParserConfiguredException (extends PreconditionViolationException)
• print to System.err
• log a report entry with JUnit Jupiter

We should also mention which parsers we support.

[filiphr]

Which Maven did you use? Using 3.8.4 I get

[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.031 s <<< FAILURE! - in org.junitpioneer.demo.JsonSourceTests
[ERROR] org.junitpioneer.demo.JsonSourceTests.jsonTest(String, int) Time elapsed: 0.001 s <<< ERROR!
org.junit.platform.commons.PreconditionViolationException: There is no available Json parsing library

Nothing is cut short. I will add more information about which json parsers are currently supported.

For gradle when I do ./gradlew test I only get that test failed. In the test report you can see the full error with its stacktrace.

When I add

tasks.named<Test>("test") {
	useJUnitPlatform()

	testLogging {
		setExceptionFormat("full")
	}
}

then I get

JsonSourceTests > jsonTest(String, int) > org.junitpioneer.demo.JsonSourceTests.initializationError FAILED
org.junit.platform.commons.PreconditionViolationException: There is no available Json parsing library
at app//org.junitpioneer.jupiter.json.JsonConverterProvider.getJsonConverter(JsonConverterProvider.java:35)
.... There is actually more stacktrace

Regarding the options you mentioned:

• What is the benefit of throwing NoJsonParserConfiguredException?
• I am not a fan of printing to System.err. So will leave it to you to add it in case you really really want it
• I can log a report entry with JUnit Jupiter, need to see how to do that first.

[nipafx]

I don't recall the details any more (stupid me, I should've added what I saw as text or screenshot), so I'll rely on your analysis. Regarding the options:

• The exception name showed up prominently - if that's NoJsonParserConfiguredException, it might not even be necessary to read the text.
• Me neither, let's not do it then.
• you know what, ignore that. I think it makes sense to limit report entries to (mostly) successful operations, not for situations that abort the test run.

[filiphr]

• whatever we decide Support JSON as source for parameterized tests arguments #101 (comment)

I have added a comment in that linked issue;

• support new Cartesian tests instead of the old approach

This is already done in this PR

• I still need to wrap my head around Discuss approach to optional dependencies #502 (this PR currently uses Gradle's feature variants).

This was resolved

---

Still to do is the last comment in #492 (comment) and rebase the branch to fix the commits

[nipafx]

Looks really good. The documentation doesn't reflect the three annotations yet (I only updated the intro up until the bullet point list), but otherwise this looks good to merge. 👍🏾

[nipafx]

No tests for files? Too brittle?

[filiphr]

If you have an idea for how we can test files in an annotation (which doesn't accept dynamic input) then I am all ears and ready to do it. Currently I am not sure how I can test this.

[nipafx]

This should work, right?

@ParameterizedTest
@JsonFileSource("build/resources/test/org/junitpioneer/jupiter/json/jedis.json")
void deconstructCustomerMultipleLinesComplexType(@Property("name") String name, @Property("height") int height) {
	assertThat(Collections.singleton(tuple(name, height)))
		.containsAnyOf(tuple("Luke", 172), tuple("Yoda", 66));
}

[nipafx]

I just noticed that the module declaration (in src/modules) still needs to be updated.

[filiphr]

The documentation doesn't reflect the three annotations yet (I only updated the intro up until the bullet point list), but otherwise this looks good to merge.

What is missing in the documentation? I think that we have all the info there.

I just noticed that the module declaration (in src/modules) still needs to be updated.

What needs to be updated there? We require static (optional) access to com.fasterxml.jackson.core and com.fasterxml.jackson.databind

[filiphr]

I closed this by accident. I've added some replies to your comments. And I fixed some of the problems that you found. Let me know what you think.

I've also added myself to the contributors list to this list, I hope I didn't mess this up.

[nipafx]

What is missing in the documentation?

It still uses the old-style @JsonFileSource(resources = "/jedis.json") syntax. While udpating that, you could also create the snippets as a proper demo in src/demo/java, which gets compiled, so we see when things are outdated. See report-entries.adoc for how to reference them (note the attribute :xp-demo-dir: at the top).

I've also added myself to the contributors list to this list, I hope I didn't mess this up.

Nope, looks good. And you definitely belong there for this 12-month-juggernaut. :D

What needs to be updated there?

Nothing, looks good. 😬

[filiphr]

I've updated the documentation. It was a good thing to mention the use of proper compilable demo, because even the test annotation had a typo. I think everything is covered now, if there is something more missing, please do let me know

[nipafx]

Commit message:

Support JSON sources for parametrized tests (#101 / #492)

This extension needs a JSON parser. In order not do pull in a
specific run-time dependency, it makes use of Gradle's feature
variants as described in CONTRIBUTING.md (including Checkstyle
rules). For now, only Jackson is supported as JSON parser. Other
parsers can be added in future versions if users need them.

Closes: #101
PR: #492

[nipafx]

Great job, @filiphr, and big thanks for your patience!

[filiphr]

Thanks for merging this @nipafx. It was fun working on this, it doesn't matter how long it took :)