Skip to content

Avoid Adding Duplicated JUnit Entries on Classpath #712

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dnestoro
Copy link
Collaborator

@dnestoro dnestoro commented Apr 2, 2025

Fix for: #305

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Apr 2, 2025
@dnestoro dnestoro force-pushed the dnestoro/fix-duplicated-junit-classpath-entries branch 2 times, most recently from 90d0b96 to 095a84d Compare April 10, 2025 13:56
@dnestoro dnestoro self-assigned this Apr 11, 2025
@dnestoro dnestoro requested review from melix and olpaw April 11, 2025 09:06
@melix
Copy link
Collaborator

melix commented Apr 11, 2025

I am a bit confused by this issue. IMO, the version of JUnit declared by users should take precedence. This means that whatever JUnit version we need should only be needed at compile time of native build tools, not runtime. So it's probably a dependency scope mistake (implementation instead of compileOnly). We shouldn't rely on dirty tricks to remove jars by hand from classpath.

@dnestoro dnestoro force-pushed the dnestoro/fix-duplicated-junit-classpath-entries branch from 095a84d to b841baa Compare May 13, 2025 12:53
@dnestoro dnestoro force-pushed the dnestoro/fix-duplicated-junit-classpath-entries branch from d455439 to d80a948 Compare May 13, 2025 12:56
@dnestoro dnestoro force-pushed the dnestoro/fix-duplicated-junit-classpath-entries branch from 99c40a9 to 3f0fb62 Compare May 16, 2025 09:35
@melix
Copy link
Collaborator

melix commented May 16, 2025

@sbrannen @sdeleuze We need your inputs in order to accept or reject this PR. In order to fix this issue, we had to make several changes to dependencies. In particular, the junit platform native project is now compiled against one version of JUnit (whatever NBT wants to use), but will not "leak" these dependencies to consumers, so that they can use whatever version of JUnit they prefer (which may be a downgrade).

While this works, this is a breaking change, because now consumers have to add the junit-platform-launcher dependency to their projects in order to run native tests, similar to how we changed our own tests. Is this acceptable?

Second, and more problematic, our code depends on Junit Platform legacy reporting. I have no idea why this code is here, because our tests seem to show that it works well without. Therefore, what we've done is simply catch the error at runtime, and avoid failing if junit-platform-reporting is not on classpath. If it is required on a user project then the dependency would have to be added explicitly. We haven't found a condition that we could use to display a warning if the dependency is missing (we don't want to warn if it's redundant).

Any ideas?

@sdeleuze
Copy link
Collaborator

Let's discuss that in our next meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants