gradlex-org / java-module-testing

Gradle plugin to turn JVM Test Suites into Blackbox or Whitebox Test Suite for Java Modules
Apache License 2.0
15 stars 5 forks source link

testCompileOnly should extend compileOnly for whitebox test suites #51

Open xenoterracide opened 6 months ago

xenoterracide commented 6 months ago

whitebox won't compile if I don't set jspecify to compileOnlyApi regardless of whether I use requires static or requires static transient, and whether or not I actually use it in my test module. Attached is a reproducer.

reproducer.tar.gz

jjohannes commented 2 months ago

Thanks for reporting and sorry for the late reply.

This issue can be solved by adding this configuring:

configurations.testCompileOnly { extendsFrom(configurations.compileOnly) }

Which I think the plugin should do automatically, if a test suite is javaModuleTesting.whitebox(). It's a bit of an incosistency in Gradle itslef, because for other "scopes" like runtimeOnly the relationship is already like this.

xenoterracide commented 2 months ago

I can kind of see why you might not want testCompileOnly to extend compileOnly (not api) because test might not have any dependency on it. In this case with jspecify it's nullity annotations and so in theory they aren't required outside of the main module. Entirely possible there's something I don't understand about that though... It's somewhat confusing to me when an applied annotation needs to be exposed to consumers at compile time as it seems they can be optional.

jjohannes commented 2 months ago

I think the confusion is the "two ways" of whitebox testing I mentioned in #47

xenoterracide commented 2 months ago

what is the difference if

Vampire commented 2 months ago

And there it looks like it needs to see the compile time dependencies again (although the "module-info.java" is technically already compiled).

That the module-info.java is already compiled is irrelevant here, only its content is relevant. It's content define which other modules you need at which times. requires static org.jspecify means that org.jspecify is needed at compile time and optional at runtime of that module's source code. By using the --patch-module to patch the test sources into the module, the same rules apply for that test code as it is now part of that module and so you need the org.jspecify at compile time, whether you use it or not.

That Gradle does not add this extendsFrom is imho correct. By using that --patch-module this connection is drawn and the requirement added. So I agree that when setting up whitebox testing, this plugin should also add this configuration extending.

I do have a module-info.java in test and do not use this plugin? that does seem to apply at least some of the rules as it required my test code to live in a different package than what main exports. I do have a module-info.java in test and do use this plugin

If you do not use the plugin, then build\resources\test and build\resources\main are not considered part of the module as those, build\classes\java\test and build\classes\java\main are treated separately (which is one of the effects of incomplete ( faulty JPMS support by Gradle imho), so if you have some resources in test that your tests need to access or some resources in main that your main code needs to access during the test it will fail.

The plugin for blackbox testing adds a jar task that puts the test classes and resources into a jar if necessary and reconfigures the test classpath / modulepath, so that the test-jar is used instead of the separate directories.

Imho it would be better to instead add --patch-module arguments like for the compiling, then you do not get an additional task and it does not need to build the jar to run the tests.

But as long as you do not have resources or do not need to retrieve them during testing, there should not be any difference.

One difference though is, that if you use the default test test suite and want to use the plugins blackbox testing, you have to add an explicit dependency from testImplementation to project as the plugin breaks this connection that is special for the test test suite. No idea whether that is intentional or a bug.