-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix support for package-private test methods #5100
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
Conversation
Prior to this commit, when the test super class wss declared in another package and declared a package-private test method with the same signature as the subclass, the method from the subclass was silently ignored. With the changes in this commit, both test methods are executed, albeit with the same display name. Fixes #5098.
...-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodSelectorResolver.java
Outdated
Show resolved
Hide resolved
junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed 31128d6 to make the build green. But I've got some reservations about the way I've done so. For a lack of a better word, I would expect a symmetry of concerns between the unique id segment of a method and the method selector. Currently that symmetry is broken.
junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java
Show resolved
Hide resolved
junit-platform-engine/src/main/java/org/junit/platform/engine/discovery/MethodSelector.java
Outdated
Show resolved
Hide resolved
Thanks! I have similar reservations. Therefore, I've pushed two commits that solve the issue by introducing a Jupiter-specific, internal @mpkorstanje WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks much neater!
We are still left with an adjacent issue that it is not possible to select all method based tests in a class with a collection of method selectors.
I.e. I would expect that while using the EngineDiscoveryRequestResolver I should be able to produce the same test plan by using either one class selector, or a collection of method selectors. Because the MethodSelector equals implementation lacks fidelity that isn't possible yet.
But I think we can fix that on a separate PR.
I agree but the scenario here is truly an edge case and I think we can postpone tackling the adjacent issue. When we do, we also need to think about |
And |
Prior to this commit, when the test super class wss declared in another package and declared a package-private test method with the same signature as the subclass, the method from the subclass was silently ignored. With the changes in this commit, both test methods are executed, albeit with the same display name. Fixes #5098.
Prior to this commit, when the test super class wss declared in another package and declared a package-private test method with the same signature as the subclass, the method from the subclass was silently ignored. With the changes in this commit, both test methods are executed, albeit with the same display name. Fixes #5098.
Prior to this commit, when the test super class wss declared in another
package and declared a package-private test method with the same
signature as the subclass, the method from the subclass was silently
ignored. With the changes in this commit, both test methods are
executed, albeit with the same display name.
Fixes #5098.
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotations