Bug 573534 - Support the concept of "test-bundles"
Summary: Support the concept of "test-bundles"
Status: NEW
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.18   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: PDE-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-14 04:40 EDT by Christoph Laeubrich CLA
Modified: 2021-07-15 11:16 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Laeubrich CLA 2021-05-14 04:40:57 EDT
--- General Description ---

Currently in PDE a TargetBundle can either be a 'normal' bundle or it can be a 'source' bundle by TargetBundle#isSourceBundle method (even though this is not enough to make it actually work).

JDT on the other hand support marking source-folders and classpath elements as "test", and makes those only available to other test source folders for compile.

This currently leads to different behavior if one imports the source of a bundle, or import is via a target platform as a binary artifact (see [1] as an example).

My proposal would be to add support for the "test" flag to PDE as well, a very first step would be to allow to actually mark a TargetBundle as a test-bundle e.g. with a new method TargetBundle#isTestBundle.

Then the TargetPlatformLocation has to get a mean how it identifies those ones, e.g. P2 could have a flag in its metadata, the m2e maven location could use the maven.scope and projects might be flagged if the only contain test-source folders.

This would also solve a long outstanding issue that actually test-fragments can't be distinguished from "normal" fragments if they are deployed to an updatesite (common in Platform builds as I have recently learned), or automatic tycho-pomless detection.


[1] https://github.com/eclipse-m2e/m2e-core/pull/196
Comment 1 Mickael Istria CLA 2021-05-16 07:34:41 EDT
For Plugin Development, the test flag does solve no problem and adds complexity. Instead of trying to workaround the issus, we should revisit the approach and just prevent the issue from occurring (eg stop using "test" flag where it shouldn't be set)
Setting "test" flag on folders that are packaged inside the bundle is fundamentally wrong: "test" mean extra restrictions, expectation and extensions that don't apply to tests inside bundles at the moment.
So IMO, we should just ensure that those flag are not set erroneously and stop confusing PDE.
Comment 2 Christoph Laeubrich CLA 2021-05-16 11:14:38 EDT
As great effort was taken for JDT to support 'test' sources and 'test' dependencies I don't see a reason not to support it for bundles.

And as PDE refuses to support plain JDT approach it seems logical to request first-class support for it on the plugin level.
Comment 3 Mickael Istria CLA 2021-05-31 05:58:37 EDT
> This would also solve a long outstanding issue that actually test-fragments can't be distinguished from "normal" fragments if they are deployed to an updatesite (common in Platform builds as I have recently learned), or automatic tycho-pomless detection.

Maven has a concept of test/not test.
OSGi doesn't have such concept, and a bundle can be test or not test. A "test" bundle can be a regular dependency for another bundle.
Unlike source bundles, test bundles can export packages and be referenced in other pieces of code as any other bundle. There is no real difference between those bundles, and no strong need to make them different. What makes then test bundles or not is whether they're only used to run tests, without exporting any package.
For this reason, I don't think that it makes sense to try to add such semantic about test or not. I also don't get how this proposal would boost the productivity of the developers or the quality of their production compared to the current state.
Comment 4 Christoph Laeubrich CLA 2021-05-31 06:28:27 EDT
(In reply to Mickael Istria - away from work until June from comment #3)
> Maven has a concept of test/not test.

JDT as well ...

> OSGi doesn't have such concept, and a bundle can be test or not test.

Its all a matter of definition, and the flag won't propagate to any runtime but compile/'assemble' time e.g. help PDE to make the right choices when running a test.

> I also don't get how this proposal would boost the productivity
> of the developers or the quality of their production compared
> to the current state.

Just take the following trivial use case:

1) You have a bundle A
2) You have a fragment containing test and thus bundle A is the host
3) You create a a new run/product/feature/whatever... and use the "add additional dependencies"
4) Because PDE do not knows that your fragment is 'test-only' it happily pulls this in with all the test-stuff (junit and alike)
Comment 5 Holger Voormann CLA 2021-07-15 08:15:14 EDT
(In reply to Christoph Laeubrich from comment #4)

The concept of test/not-test exists in JDT only because of Maven.

The problem is when there are encapsulated modules/bundles as in OSGi or as in the Java Platform Module System (JPMS). Encapsulation and testing is a trade-off between testing the public API only and breaking the encapsulation to test also internal stuff.

In JPMS, there are command line arguments to partially relax the encapsulation or you can disable JPMS completely by using the classpath instead of the modulepath to run your tests (only possible if you do not use JPMS services). Since in OSGi there is no flat class/modulepath and since bundles can be installed, started, stopped and uninstalled at run time, there are no such dirty command line arguments hacks.

Even if it would be possible, the current best practices for testing OSGi/Eclipse bundles/plugins are by far cleaner and better than the test/not-test concept that exists only because of Maven and which requires dirty tricks to run the tests.

Using fragments for testing breaks encapsulation and is therefore not best practice. If necessary at all, better use fragments only to test internal stuff or - to avoid to have an additional test fragment - use "x-internal" (which became best practice at Eclipse after there has been in-depth discussion about this a long time ago).
Comment 6 Christoph Laeubrich CLA 2021-07-15 09:34:11 EDT
This is not fixed nor resolved!

(In reply to Holger Voormann from comment #5)
> (In reply to Christoph Laeubrich from comment #4)
> 
> The concept of test/not-test exists in JDT only because of Maven.

This doesn't mean it is not useful outside of maven, also maven is commonly used to develop OSGi-Bundles so I don't see any problem here.

> Since in OSGi there is no flat class/modulepath and since
> bundles can be installed, started, stopped and uninstalled at run time,
> there are no such dirty command line arguments hacks.

This doesn't mean that a test always requires a running OSGi-Framework, or require any "hacks" e.g. enhancing the (test) classpath as a mean of bundle-classpath is always possible, leagal and not a hack, just the tool support is poor here 


> Even if it would be possible, the current best practices for testing
> OSGi/Eclipse bundles/plugins are by far cleaner and better than the
> test/not-test concept that exists only because of Maven and which requires
> dirty tricks to run the tests.

Just because it is currently not possible to do it better don't mean its "best-practice". There are a lot of projects and frameworks (bnd, maven-bundle-plugin, pax-exam) that support much cleaner/richer testing capabilities that the limited support of PDE that simply do not allow any good practice when it comes to testing.

> Using fragments for testing breaks encapsulation and is therefore not best
> practice. If necessary at all, better use fragments only to test internal
> stuff or - to avoid to have an additional test fragment - use "x-internal"
> (which became best practice at Eclipse after there has been in-depth
> discussion about this a long time ago).

All of these are just hack because PDE lacks supporting basic java/osgi techniques to do it better ...
Comment 7 Holger Voormann CLA 2021-07-15 11:16:05 EDT
(In reply to Christoph Laeubrich from comment #6)
> This is not fixed nor resolved!
> 
Sorry for accidentally closing this bug. I don't know why this happened, I didn't intend it.

In Maven, I don't like having main and test code in the same project, which adds complexity and makes the encapsulation more unclear.

Probably, I have not yet got your point. Could you please show the same example with and without the concept of "test-bundles" and explain what "test-bundles" simplifies from the developer's point of view? It is also not clear to me which artifacts PDE/Tycho should generate from a plugin with only main code, only test code or with main and test code.