Skip to content

Conversation

marcphilipp
Copy link
Member

@marcphilipp marcphilipp commented Aug 18, 2025

  • Inherit @TestMethodOrder to enclosed @Nested classes
  • Align implementations of ClassOrderingVisitor and MethodOrderingVisitor
  • Introduce MethodOrderer.Default for reverting back to default ordering
  • Introduce ClassOrderer.Default for reverting back to default ordering

Resolves #4731.


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

* Use a cache for the `DescriptorWrapperOrderer` in both cases
* Include test class in warning messages in both cases
@marcphilipp marcphilipp force-pushed the marc/4731-inherit-orderers branch from ec1f1d7 to 365bfa0 Compare August 18, 2025 13:07
@sbrannen sbrannen self-requested a review August 19, 2025 09:01
Copy link
Member

@sormuras sormuras left a comment

Choose a reason for hiding this comment

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

Implementation, refactoring, and tests look good to me.

Copy link
Member

@sbrannen sbrannen left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

It looks very good.

Although I reviewed the entire PR, I did not spend a lot of time reviewing the interim refactoring commit. Thus, I assume the refactoring part is "good to go" as well.

Otherwise, I've only requested a few minor changes to Javadoc and tests.

@marcphilipp marcphilipp requested a review from sbrannen August 19, 2025 15:54
@marcphilipp marcphilipp merged commit fc2168b into main Aug 20, 2025
14 checks passed
@marcphilipp marcphilipp deleted the marc/4731-inherit-orderers branch August 20, 2025 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inherit @TestClassOrder and @TestMethodOrder within @Nested hierarchies
3 participants