eclipse-jdt / eclipse.jdt.ui

Eclipse Public License 2.0
37 stars 90 forks source link

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

Closed HannesWell closed 2 months ago

HannesWell commented 2 months ago

What it does

Fixes https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1610

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 https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1610#issuecomment-2308799284, 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

HannesWell commented 2 months ago

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 https://github.com/eclipse-jdt/eclipse.jdt.ui/pull/1612 to get rid of it. But I don't know if that's something for this or only for the next release.

merks commented 2 months ago

@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?

akurtakov commented 2 months ago

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

jarthana commented 2 months ago

Copying @noopur2507

laeubi commented 2 months ago

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 commented 2 months ago

See also https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1245 where the changes were done after hamcrest update from 1.x to 2.x.

merks commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

image

HannesWell commented 2 months ago

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 commented 2 months ago

@jukzi

Thank you.