Skip to content

Enable JUnit 5 usage in test framework #17475

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

Closed
wants to merge 1 commit into from

Conversation

dbwiddis
Copy link
Member

Description

Adds dependencies on JUnit 5 while maintaining compatibilitly with JUnit 4. JUnit 4 is still required for randomized testing.

Related Issues

Resolves #17445

Check List

  • Functionality includes testing.
  • [ ] API changes companion pull request created, if applicable.
  • [ ] Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions github-actions bot added Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. enhancement Enhancement or improvement to existing feature or request labels Feb 27, 2025
@dbwiddis dbwiddis added the backport 2.x Backport to 2.x branch label Feb 27, 2025
@andrross
Copy link
Member

Will this be disruptive to all plugins that declare their own JUnit5 dependencies?

@reta
Copy link
Contributor

reta commented Feb 27, 2025

@dbwiddis in addition to @andrross comment, what is the motivation for this change? I am all in towards modernization but there is no way we could jump off the JUnit 4 till Apache Lucene does. With that in mind, I don't see any benefits of porting tons of JUnit 4 related scaffolding to JUnit 5 (and maintaining both)

@dbwiddis
Copy link
Member Author

Will this be disruptive to all plugins that declare their own JUnit5 dependencies?

I don't think so -- the only impact I think would be jar hell unless/until they align versions or use the version catalog; however, given that the dependencies are runtimeOnly I don't think it will. For example, remote-metadata-sdk has 5.12 while flow-framework (which depends on it) has 5.11.4 and there are no conflicts.

what is the motivation for this change?

As mentioned in the linked issue, my primary motivation was just having a common version of JUnit in the version catalog to reduce the churn of version bump PRs (and their backports).

An alternative (which @andrross mentioned in #17445) was writing a new test-frameworkjunit5 with minimal test in it, but given the design for side-by-side operation that seemed a lot more complicated.

there is no way we could jump off the JUnit 4 till Apache Lucene does.

As noted in the linked issue; the randomized testing framework keeps them (us) on JUnit 4 unless/until someone rewrites it (which will likely not happen).

I don't see any benefits of porting tons of JUnit 4 related scaffolding to JUnit 5 (and maintaining both)

There is no intent to ever fully migrate off JUnit 4, and no need to even migrate anything, although new code could certainly choose. It is possible/by design to run them side by side and allow simultaneous usage of both which is what this enables, without changing any existing JUnit 4 code.

There could be some advantages for those familiar with Junit 5 who could use them in future OpenSearch tests. I personally have used many JUnit 5 features including:

  • a lot more assertion types, particularly assertThrows()
  • conditional tests based on different operating systems

Copy link
Contributor

❌ Gradle check result for e6a1bbd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for e6a1bbd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@reta
Copy link
Contributor

reta commented Feb 27, 2025

There could be some advantages for those familiar with Junit 5 who could use them in future OpenSearch tests. I personally have used many JUnit 5 features including:

This is covered by assert4j, it is super difficult to keep both frameworks, Gradle is notoriously deeply integrated with both and the fresh example - try to use (for real) latest JUnit 5 (5.12.0) with Gradle 8.12.x - it fails the build right away. I don't think we should bring JUnit 5 in any form without clear goal and vision to migrate to it, it will be another pain point.

Copy link
Contributor

❌ Gradle check result for e6a1bbd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@andrross
Copy link
Member

it is super difficult to keep both frameworks, Gradle is notoriously deeply integrated with both and the fresh example - try to use (for real) latest JUnit 5 (5.12.0) with Gradle 8.12.x - it fails the build right away. I don't think we should bring JUnit 5 in any form without clear goal and vision to migrate to it, it will be another pain point.

Hmm, I wasn't aware of the challenges of keeping both frameworks. Given the complexity of our gradle build I'm also very concerned about adding anything that might make that area of the project harder to maintain.

@reta
Copy link
Contributor

reta commented Feb 27, 2025

Hmm, I wasn't aware of the challenges of keeping both frameworks. Given the complexity of our gradle build I'm also very concerned about adding anything that might make that area of the project harder to maintain.

Example here opensearch-project/spring-data-opensearch#424 (and this one was/is JUnit 5 only)

@dbwiddis
Copy link
Member Author

try to use (for real) latest JUnit 5 (5.12.0) with Gradle 8.12.x - it fails the build right away.

Yeah, saw that (and fixed) on SDK, https://github.com/opensearch-project/opensearch-remote-metadata-sdk/pull/99/files

I don't think we should bring JUnit 5 in any form without clear goal and vision to migrate to it, it will be another pain point.

Given the complexity of our gradle build I'm also very concerned about adding anything that might make that area of the project harder to maintain.

Fair 'nuff. Closing this PR and the feature request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. enhancement Enhancement or improvement to existing feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Support JUnit 5
3 participants