Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testCompileOnly should extend compileOnly for whitebox test suites #51

Open
xenoterracide opened this issue Mar 14, 2024 · 5 comments
Open
Labels
enhancement New feature or request
Milestone

Comments

@xenoterracide
Copy link

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
Copy link
Member

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.

@jjohannes jjohannes added the enhancement New feature or request label Jun 25, 2024
@jjohannes jjohannes changed the title compileOnlyApi required testCompileOnly should extend compileOnly for whitebox test suites Jun 25, 2024
@xenoterracide
Copy link
Author

xenoterracide commented Jun 25, 2024

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
Copy link
Member

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

  • If you do not have a modue-info.java and do not use this plugin, Gradle runs things on the classpath and it works
  • If you do not have a module-info.java ad do use thins plugin, the plugin changes the test setup such that the main and test code are merged into one Module (in the Java sense). So you are able to test things "inside the module". But then, your main/module-info.java (or the compiled version of it) is used by javac in the patching process (--patch-module). And there it looks like it needs to see the compile time dependencies again (although the "module-info.java" is technically already compiled). At least I think that's what's happening (The --patch-module functionality is a bit crazy).

@xenoterracide
Copy link
Author

xenoterracide commented Jun 25, 2024

what is the difference if

  • 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

@Vampire
Copy link

Vampire commented Jun 27, 2024

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.

@jjohannes jjohannes added this to the 1.5 milestone Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants