Support Hamcrest 3.0 in 'JUnit 4/5' classpath containers #1611

HannesWell · 2024-08-25T11:57:34Z

What it does

How to test

Make sure you have hamcrest 3.0 in your TP, e.g. from https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/2024-09/ and have a JDT project with the JUnit 4 or 5 classpath container.

I'm not exactly sure about the version range, the last time it was updated to only support hamcrest 2.2, but as written in #1610 (comment), hamcrest 3.0 is probably a drop-in preplacement and both could be supported.
Given that we are late in the release-cycle it might even happen that not everyone is able to update to hamcrest-3.0

CC @laeubi and @iloveeclipse and maybe @merks is also interested.

Author checklist

I have thoroughly tested my changes
The change is following the coding conventions
I have signed the Eclipse Contributor Agreement (ECA)

Fixes eclipse-jdt#1610 Co-authored-by: Christoph Läubrich <[email protected]>

HannesWell · 2024-08-25T12:32:33Z

At the moment hamcrest-core seems not to be available in version 3. I assume that's because Orbit-SimRel does not provide it (yet)? Or simply nobody pulls it into SimRel (which I think is good).

But since version 2, hamcrest-core is empty any way and only serves the purpose to redirect to the fat hamcrest artifact.
Adding the former in version 2.x or later to the JUnit classpath Container is therefore useless.
I created #1612 to get rid of it. But I don't know if that's something for this or only for the next release.

jukzi

As I20240825 seems to be broken since hamcrest 3.0 was added either hamcrest 3.0 hast to be removed or supported.

merks · 2024-08-26T07:50:40Z

@akurtakov

I think this needs to be merged as soon as possible The hamcrest 3.0 bundle has already landed in the release train repository and removing/excluding it is not a trivial exercise. Given we have little control over what is installed in the end, we had best not be broken when it's present. And what happens when 3.1 becomes available? Does this same problem come back?

merks · 2024-08-26T07:52:22Z

org.eclipse.jdt.junit.core/src/org/eclipse/jdt/internal/junit/buildpath/BuildPathSupport.java

@@ -305,10 +305,10 @@ public boolean accept(File dir, String name) {
 			"org.junit", new VersionRange("[4.13.0,5.0.0)"), null, "org.junit_4.*.jar", "org.junit.source", "source-bundle/", JUnitPreferencesConstants.JUNIT4_JAVADOC); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$

 	private static final JUnitPluginDescription HAMCREST_CORE_PLUGIN= new JUnitPluginDescription(
-			"org.hamcrest.core", new VersionRange("[2.2.0,2.3.0)"), null, "org.hamcrest.core_2.*.jar", "org.hamcrest.core.source", "source-bundle/", JUnitPreferencesConstants.HAMCREST_CORE_JAVADOC); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
+			"org.hamcrest.core", new VersionRange("[2.2.0,3.1.0)"), null, "org.hamcrest.core_2.*.jar", "org.hamcrest.core.source", "source-bundle/", JUnitPreferencesConstants.HAMCREST_CORE_JAVADOC); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$


Will the same problem resurface when 3.1 is released? It will happen in the future...

I don't know why so strict ranges are used here. The usual construct is '[current.min, next.major)' and as 3.0 is good I would assume all 3.x versions should be good.

Indeed my sense is we'd be better to assume that hamcrest will be well behaved with respect to semantic versions now that they are publishing OSGi bundles directly.

Does it actually fix the issue if 3.0 is NOT present?

As shown in #1610 (comment) the 2.2.0 seems to be completely empty for JUNIT 5 so don't we need 3.x as lower bound?

It's sort of not empty because it contains this abomination re-exporting a bundle regardless of whether it might be version 1000.0.0 of that bundle:

Automatic-Module-Name org.hamcrest.core.deprecated Bundle-ManifestVersion 2 Bundle-Name Bundle org.hamcrest : hamcrest-core Bundle-SymbolicName org.hamcrest.core Bundle-Version 2.2.0.v20230809-1000 Eclipse-Wrapped-Bundle org.hamcrest:hamcrest-core:2.2 Implementation-Title hamcrest-core Implementation-Vendor hamcrest.org Implementation-Version 2.2 Import-Package java.lang Manifest-Version 1.0 Require-Bundle org.hamcrest;bundle-version="2.2.0";visibility:=reexport Require-Capability osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.7))"

It's sort of not empty because it contains this abomination re-exporting a bundle regardless of whether it might be version 1000.0.0 of that bundle:

While this is relevant inside an OSGi framework it doesn't help for the classpath container because that one is just a collection of jars. So this entry actually is obsolete for 2.2... but we can remove it on the next cycle probably.

Does it actually fix the issue if 3.0 is NOT present?

As shown in #1610 (comment) the 2.2.0 seems to be completely empty for JUNIT 5 so don't we need 3.x as lower bound?

Yes, it does. The reason why the JUnit-5 container does not fail completely is that for JUnit-5 library entries that are not found and therefore null, are removed, while for JUnit-4 they are preserved. So if a desired library is not found it leads to an NPE when initializing a JUnit-4container, while for JUnit-5 the missing library is simply discarded, in this case hamcrest, which leads to compilation errors:

eclipse.jdt.ui/org.eclipse.jdt.junit.core/src/org/eclipse/jdt/internal/junit/buildpath/JUnitContainerInitializer.java

Line 150 in b83ffe1

entriesList.removeIf(e -> e == null);

It's sort of not empty because it contains this abomination re-exporting a bundle regardless of whether it might be version 1000.0.0 of that bundle:

While this is relevant inside an OSGi framework it doesn't help for the classpath container because that one is just a collection of jars. So this entry actually is obsolete for 2.2... but we can remove it on the next cycle probably.

That's right. Therefore I already created #1612.

Will the same problem resurface when 3.1 is released? It will happen in the future...

That's right. But this PR is inteded to just fix the linked issue in the presence of hamcrest-3.0. I assume there won't be a 3.1 within the next three months.
And on the long run I think the fetching of libraries should be be made more dynamic any ways (I assume no one what's to bump the version all the time?).
I'm thinking about a 'declarative' way where one declares the 'anchor/root'-packages to include (maybe an 'anchor-class' if there are split-packages) and the Eclipse OSGi runtime is queried for the bundles providing it.

We really need to move forward here for this release cycle so someone needs to push the magic button. Things are definitely badly broken without a fix. Please, please. 🙏

akurtakov · 2024-08-26T07:56:19Z

IMO we need JDT @eclipse-jdt/eclipse-jdt-project-leads to step up here.

jarthana · 2024-08-26T07:58:08Z

Copying @noopur2507

laeubi · 2024-08-26T08:23:14Z

The main problem seems that the JDT feature does not reflect the needed bundles (!) with version ranges in their feature like this:

   <requires>
      <import plugin="org.hamcrest" version="2.2.0" match="compatible"/>
   </requires>

That way P2 would have installed the "old" version as well and If it is not possible we would have seen an error at build-time instead of runtime.

noopur2507 · 2024-08-26T08:27:32Z

See also eclipse-platform/eclipse.platform.releng.aggregator#1245 where the changes were done after hamcrest update from 1.x to 2.x.

merks · 2024-08-26T08:31:26Z

See also eclipse-platform/eclipse.platform.releng.aggregator#1245 where the changes were done after hamcrest update from 1.x to 2.x.

It brings back bad memories of killing off a nice weekend. 😱

noopur2507 · 2024-08-26T08:37:09Z

Moving from hamcrest 2.x to 3.x in JDT/JUnit should be done when we have more time for testing. However, currently, it's OK to release the current PR to accommodate hamcrest 3.x. Removing hamcrest-core and other related enhancements can be taken up for the next release.

merks · 2024-08-26T08:51:53Z

Yes, we should be cautious. The 3.0 version does appear to be in most of the M3 packages:

HannesWell · 2024-08-26T17:46:04Z

However, currently, it's OK to release the current PR to accommodate hamcrest 3.x.

So can this then be submitted now in order to have it in an I-builds as soon as possible?

merks · 2024-08-26T19:38:24Z

@jukzi

Thank you.

Support Hamcrest 3.0 in 'JUnit 4/5' classpath containers

8a6019f

Fixes eclipse-jdt#1610 Co-authored-by: Christoph Läubrich <[email protected]>

HannesWell force-pushed the fix-1610 branch from cf29050 to 8a6019f Compare August 25, 2024 12:24

merks mentioned this pull request Aug 25, 2024

Invalid classpath container: 'JUnit 4' in project 'org.eclipse.xtext.xbase.lib.tests' eclipse-xtext/xtext#3178

Closed

jukzi approved these changes Aug 26, 2024

View reviewed changes

merks reviewed Aug 26, 2024

View reviewed changes

jukzi merged commit acb89e3 into eclipse-jdt:master Aug 26, 2024
9 checks passed

HannesWell deleted the fix-1610 branch August 26, 2024 20:16

pci-mthompson mentioned this pull request Sep 25, 2024

Hamcrest 3.0 is still breaking things in Eclipse 4.30 #1672

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Hamcrest 3.0 in 'JUnit 4/5' classpath containers #1611

Support Hamcrest 3.0 in 'JUnit 4/5' classpath containers #1611

HannesWell commented Aug 25, 2024

HannesWell commented Aug 25, 2024

jukzi left a comment

merks commented Aug 26, 2024

merks Aug 26, 2024

akurtakov Aug 26, 2024

merks Aug 26, 2024

laeubi Aug 26, 2024

merks Aug 26, 2024

laeubi Aug 26, 2024

HannesWell Aug 26, 2024

merks Aug 26, 2024

akurtakov commented Aug 26, 2024

jarthana commented Aug 26, 2024

laeubi commented Aug 26, 2024

noopur2507 commented Aug 26, 2024

merks commented Aug 26, 2024

noopur2507 commented Aug 26, 2024

merks commented Aug 26, 2024

HannesWell commented Aug 26, 2024

merks commented Aug 26, 2024

Support Hamcrest 3.0 in 'JUnit 4/5' classpath containers #1611

Support Hamcrest 3.0 in 'JUnit 4/5' classpath containers #1611

Conversation

HannesWell commented Aug 25, 2024

What it does

How to test

Author checklist

HannesWell commented Aug 25, 2024

jukzi left a comment

Choose a reason for hiding this comment

merks commented Aug 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akurtakov commented Aug 26, 2024

jarthana commented Aug 26, 2024

laeubi commented Aug 26, 2024

noopur2507 commented Aug 26, 2024

merks commented Aug 26, 2024

noopur2507 commented Aug 26, 2024

merks commented Aug 26, 2024

HannesWell commented Aug 26, 2024

merks commented Aug 26, 2024