Skip to content

fix(compat): restore junit5 compatibility to LocallyRunOperatorExtension#3355

Merged
csviri merged 1 commit into
operator-framework:mainfrom
k-wall:issue-3354
May 15, 2026
Merged

fix(compat): restore junit5 compatibility to LocallyRunOperatorExtension#3355
csviri merged 1 commit into
operator-framework:mainfrom
k-wall:issue-3354

Conversation

@k-wall
Copy link
Copy Markdown
Contributor

@k-wall k-wall commented May 15, 2026

As described by #3354, this change restores Junit5 compatibility for users using LocallyRunOperatorExtension in a stack that has not upgraded to Junit6.

Copilot AI review requested due to automatic review settings May 15, 2026 12:33
@openshift-ci openshift-ci Bot requested review from csviri and xstefank May 15, 2026 12:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds a runtime fallback for registering CrdCleanup to retain compatibility across different JUnit 5 ExtensionContext.Store API versions.

Changes:

  • Introduced a try/catch (NoSuchMethodError) fallback from computeIfAbsent to getOrComputeIfAbsent.
  • Refactored store access into a local store variable for reuse.

Comment on lines +366 to +371
try {
store.computeIfAbsent(CrdCleanup.class, ignored -> new CrdCleanup(deleteCRDs));
} catch (NoSuchMethodError nsme) {
// Exists to retains compatibility with Junit5.
store.getOrComputeIfAbsent(CrdCleanup.class, ignored -> new CrdCleanup(deleteCRDs));
}
Copy link
Copy Markdown
Contributor Author

@k-wall k-wall May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not too convinced by Copilot's advice here. The reminder why the call to the deprecated method is made is helpful IMO. Using reflection would be overkill.

Copy link
Copy Markdown
Collaborator

@xstefank xstefank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, since this should not break JUnit 6, I don't see why not merge it.

Copy link
Copy Markdown
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@csviri csviri merged commit a442bb2 into operator-framework:main May 15, 2026
27 checks passed
@k-wall k-wall deleted the issue-3354 branch May 15, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change made to LocallyRunOperatorExtension in v5.3.3 means users must use Junit6

4 participants