eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
157 stars 125 forks source link

Support additional module-info.java for tests, if that ends up being de-facto standard #1465

Open hohwille opened 12 months ago

hohwille commented 12 months ago

Problem

If a module requires additional modules only for its test cases, the de-facto standard is to define an additional module-info.java file in src/test/java that should be declared open and has a test specific identifier and requires the main module from src/main/java/module-info.java. Here is an official example from Maven for this use-case: https://maven.apache.org/surefire/maven-surefire-plugin/examples/jpms.html

However, Eclipse does not support that a single project may have more than one module-info.java. Therefore you get the following error:

Build path contains duplicate entry: 'module-info.java' for project '«project-name»'

Why is this important?

The JPMS is very strict. As soon as I define modules, I need to declare any required module or I can not even import an according class. So in a library project, I might want to have very minimal required module dependencies (e.g. none at all). However, for implementing my JUnit tests, I might need some logging (java.util.logging or org.slf4j). I do not want to be forced to require that logging in my main module just to be able to use it only in my tests. But otherwise, I can not even add the import to the Logger in my JUnit test class in Eclipse. The logging is just a famous example - this also applies to whatever else you might need additionally for your tests. With JPMS the entire concept of test scoped dependencies becomes more or less pointless with Eclipse due to this lacking feature.

Impact

Developers of JPMS modules are forced to use a different IDE than Eclipse. A workaround may be to create an extra projects only for testing. However this has the following drawbacks:

Related issues

Sources

stephan-herrmann commented 12 months ago

the de-facto standard is to define an additional module-info.java file in src/test/java that should be declared open and has a test specific identifier and requires the main module from src/main/java/module-info.java.

It's a pity, that no real standard has emerged, and so we are left with various concepts that cannot easily be reconciled.

If I understand the maven approach correctly, then src/main/java and src/test/java should be seen as two regular modules (JMPS) that just happen to share a common pom (which in itself may have a test-scoped slice that doesn't apply to src/main/java).

This differs from another concept that has been proposed: to consider src/test/java as an overlay, which is merged with src/main/java for compilation and execution of tests and ignored otherwise.

I think it's fair to call the first approach black-box tests whereas the second approach (merging) support white-box testing (of private innards of a module).

Traditionally, in Eclipse the black-box approach most closely corresponds to two separate projects. The key point being: in Eclipse a classpath is defined exactly once per project. Eclipse does not genuinely define any conditional logic in .classpath. There are two backdoors, though:

Let's briefly dwell on the latter feature. References (in addition to the N&N entry above):

For a first approximation of the semantics this is what @brychcy wrote:

For test code I implemented the following in my patch:

  • production sources see the workspace as if test sources and test dependencies don't exist
  • test sources see the workspace as if the whole mechanism doesn't exist - (actually it sees the production sources in the same project only as binaries instead of sources, but this shouldn't make a difference)

And then any dependency can be marked as visible only for the "test" scope. Tada!

With respect to JPMS this feature implements the suggestions from http://openjdk.java.net/jeps/261 (which may be seen as authoritive, no?) as to use options like --add-reads and --patch-module to integrate test code with module main code.

As you can see, the internal semantics of marking folders and dependencies as "test" or not test, is already quite sophisticated, but it seems to solve "the other problem": enabling white box testing while avoiding the need of a secondary module-info.java.

Back to the request to support a two-module black-box approach, the above seems to imply, that we first need to disable parts of the existing solution, viz. we need a mode without any privileges for the test folder. OTOH, we need to retain the ability to compile in two separate invocations for main vs test.

So in the implementation we'd need to extend enum CompilationGroup { MAIN, TEST } with a third option like TEST_MODULE. Then every code location asking for TEST needs to differentiate what needs to be done for the new mode. If requirements at this level can be specified in terms of a compiler command line then tweaking this part accordingly sounds manageable.

In the UI I would prefer not to surface all three modes, so as a quick idea, we could distinguish the two test modes based on existence of src/test/java/module-info.java (not based on the name of the source folder, but based on the classpath attribute "test").

Finally, there is the API perspective, where JDT promises that each modular IJavaProject has exactly one IModuleDescription. It looks like the test module simply can't be accessed via this interface. This will make it very hard to implement any additional support for the test module beyond just compilation. Inspecting the incoming call hierarchy of org.eclipse.jdt.core.IJavaProject.getModuleDescription() will give an impression of the impact.

I should also mention, that JDT is build-tool agnostic, i.e., we can only provide very generic capabilities, so that m2e can setup a project's configuration according to maven rules. For m2e the main concept for communicating with JDT is through classpath entries, where things like "test" are communicated via classpath attributes on those entries. More specifically, m2e can

It needs to be determined here, if JDT needs to understand additional classpath attributes to serve m2e right.

_On a personal note: I will continue to discuss this and give hints into the implementation if desired, but I will not actively drive this development, in part because I feel that supporting just another project layout for black box testing (which can already be done by individual Eclipse projects) is not really a huge advance. I think white box testing in the presence of modules is the thing calling for really new concepts, like merging module-info.java from two sources or such._

stephan-herrmann commented 11 months ago

In an extra project I can not access the internals of the actual project, I want to test, so I am limited to black-box-testing. However, I commonly want to write JUnits as white-box tests.

Rereading that sentence from the issue description I wonder if I may have misunderstood anything in how maven supports testing vis-a-vis JPMS: In my book defining a separate module for tests is black-box testing (see above), since a regular (test-)module does not have any privilege over other modules. In that sense the JPMS module is stronger than the colocation in the same "project", because "project" is not a concept known to Java/JPMS.

Is white-box testing of modular code supported by maven?

SentryMan commented 11 months ago

Surefire 3 lets you disable the module path for easy white-box testing, which is what I end up doing most of the time. It is annoying that eclipse doesn't respect it though.

stephan-herrmann commented 11 months ago

Surefire 3 lets you disable the module path for easy white-box testing, which is what I end up doing most of the time. It is annoying that eclipse doesn't respect it though.

This is the opposite request from this current issue (which is about respecting two module-info.java for what to me looks like black box testing).

@SentryMan Please file a separate issue for your use case. I believe "the spirit of" your use case is already supported in the IDE, so we'd need to see the exact configuration of a sample Eclipse project, to figure out the differences between a working configuration for white box testing vs. what you have in mind.

hohwille commented 8 months ago

Surefire 3 lets you disable the module path for easy white-box testing, which is what I end up doing most of the time. It is annoying that eclipse doesn't respect it though.

This is the opposite request from this current issue (which is about respecting two module-info.java for what to me looks like black box testing).

@SentryMan Please file a separate issue for your use case. I believe "the spirit of" your use case is already supported in the IDE, so we'd need to see the exact configuration of a sample Eclipse project, to figure out the differences between a working configuration for white box testing vs. what you have in mind.

I can confirm this. Regular white-box testing works fine. But assuming, I need to add a requires or provides or whatever to my module-info.java that I only want to have for testing, I am left in the rain. A workaround is add a second project/module but then I also have to be careful that my build is not accidentally deploying this test artifact as a release. Also the workaround is causing quite some overhead. But this is what I currently do to make it work. The status quo is quite confusing for developers if maven documents some approach that is however not supported by IDEs what makes the entire approach kind of useless. IMHO the suggested approach sounds totally obvious and straight forward. It might be that the internal design of eclipse is not ready for this so huge refactoring effort would be required. I still see tons of other things in Eclipse that are broken for decades that may be of higher importance then...

hohwille commented 8 months ago

In my book defining a separate module for tests is black-box testing (see above), since a regular (test-)module does not have any privilege over other modules. In that sense the JPMS module is stronger than the colocation in the same "project", because "project" is not a concept known to Java/JPMS.

I am unsure how maven behaves here exactly but my expectation would be that in both cases you are doing white-box testing and only if you define an additional module-info.java in src/test/java as well, then during the tests this will "override" the one from src/main/java.

stephan-herrmann commented 8 months ago

A workaround is add a second project/module but then I also have to be careful that my build is not accidentally deploying this test artifact as a release. Also the workaround is causing quite some overhead. But this is what I currently do to make it work.

This approach sounds very straight forward, if you ask me: a new module (JPMS test module) sits in a new module (maven), d'uh. Can you say more about the problems you encounter in that case?

It might be that the internal design of eclipse is not ready for this so huge refactoring effort would be required.

Indeed it looks like nobody is eager to use my hints above to implement a new mode of compilation. Actually, from those hints the change shouldn't be exorbitantly huge. Perhaps there's a lack of enthusiasm because all that work would enable eclipse to do something which is already possible (define a module for black box testing) only in a different layout of projects / folders. Not something that makes you famous if you implement it :)

danthe1st commented 1 week ago

Perhaps there's a lack of enthusiasm because all that work would enable eclipse to do something which is already possible (define a module for black box testing) only in a different layout of projects / folders.

To be honest, I think that most (at least non-Eclipse/OSGi-related which aren't using JPMS anyways AFAIK) Java projects would prefer using the layout with both sources and tests in the same project. I think there are many people who would like to use JPMS but wouldn't be able to do that in Eclipse due to this issue. While the approach with <useModulePath>false</useModulePath> in the <configuration> of the maven-surefire-plugin seems to work (at least for me) with Eclipse, I think this option isn't really known and most projects would probably rather have a module-info.java for both main and test sources.

Even though many projects still don't use module-info.java files, this seems to be requested more and more.

Also, if a project is using that approach, that would pretty much lock out Eclipse users from that project.

Given these points, I think many people would appreciate (or benefit from) this being implemented.

stephan-herrmann commented 1 week ago

For all those interested in simply hosting two regular JPMS modules in src/main/java and src/test/java with no further tweaks, please have a look at this simple example: MainAndTest-layout.zip

It does the simplest thing that could possibly work: create a nested project located in src/test. Import the projects in 2 steps:

I left one detail as an exercise to the reader: link the output folder src/test/target/classes to target/test-classes (I believe linked resources should help here, or an actual link in the filesystem, whatever works better).

I think m2e should be able to conveniently configure all this behind the scenes, i.e., it seems JDT is already able to support the requirements, no?

In the Project Explorer you may want to select Projects Presentation > Hierarchical. image

danthe1st commented 1 week ago

I think m2e should be able to conveniently configure all this behind the scenes, i.e., it seems JDT is already able to support the requirements, no?

Do you mean with a distinct pom.xml in src/test with a bit of configuration in it? I think importing as a non-Maven project (and importing as a Maven project requires a pom.xml) wouldn't result in m2e picking up anything.

stephan-herrmann commented 1 week ago

I think m2e should be able to conveniently configure all this behind the scenes, i.e., it seems JDT is already able to support the requirements, no?

Do you mean with a distinct pom.xml in src/test with a bit of configuration in it? I think importing as a non-Maven project (and importing as a Maven project requires a pom.xml) wouldn't result in m2e picking up anything.

No, I mean m2e could implement a smarter project import: when "Import as Maven Project" of the outer project (where pom.xml exists) finds src/test/java/module-info.java it could create the eclipse project configuration as shown in my example without any user interaction (or controlled by some checkbox in the wizard).