Skip to content
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

Add a new feature to let test methds in a class run as user specified order #1527

Closed
wants to merge 3 commits into from

Conversation

cosmos-fly
Copy link

@cosmos-fly cosmos-fly commented May 31, 2018

This is a new feature to let JUnit run as you specified order. What we need do is two steps:

  1. Adding annotation 'FixMethodOrder(MethodSorters.SPECIFIED_PRIORITY)'.
  2. Specify 'priority' value for methed annotation 'Test'.

This feature is an optional featue to let JUnit users specify method order in some special scenarios.
More details please refer to the comments in changed code.

@cosmos-fly cosmos-fly closed this May 31, 2018
@cosmos-fly cosmos-fly reopened this May 31, 2018
@cosmos-fly cosmos-fly changed the title Add a optional property priority for @Test Add a optional property priority for annotation 'Test' May 31, 2018
@cosmos-fly cosmos-fly changed the title Add a optional property priority for annotation 'Test' Add a new feature to let JUnit run as user specified order. May 31, 2018
@cosmos-fly cosmos-fly changed the title Add a new feature to let JUnit run as user specified order. Add a new feature to let JUnit run as user specified order May 31, 2018
@cosmos-fly cosmos-fly changed the title Add a new feature to let JUnit run as user specified order Add a new feature to let test methds in a class run as user specified order May 31, 2018
@kcooney
Copy link
Member

kcooney commented Jun 2, 2018

Thanks for the contribution.

I am hoping we could include #1130 in 4.13, which is a more general and flexible way to solve this problem. With that pull, you could create a customOrdering that ordered the methods by any criteria you specify (including annotations on the method). Some users will.want to specify order via a priority, others with a unique priority, and others via some dependency.

@cosmos-fly
Copy link
Author

cosmos-fly commented Jun 5, 2018

@kcooney Thanks for your comment.
I read the code of #1130, which it is a good solution. And as a JUnit user I hope to add the order feature ASAP, cause it is really a pressing need. But I think the solution in this PR is more convenient in some scenario, especially when I could not describe the order by programming. What's more, this solution is an optional way to describe the test method order.

Notice: This feature in this PR has the same behavior as another unit test framework TestNG. This is a convenient way as my experience.

  • There is no worry about about the license conflict with TestNG License Apache 2.0. My code is from my own design and there is no any code reference to TestNG, otherwise it will not work well.

@kcooney
Copy link
Member

kcooney commented Jun 5, 2018

I'm curious what the other maintainers think. @junit-team/junit-committers

While adding an attribute is convenient, I would prefer we don't merge this pull for 4.13, for these reasons:

  1. Some people strongly feel that tests should be independent and not depend on ordering. For those users, adding a priority or order attribute to @Test is too easy and therefore sends the wrong message
  2. This pull relies on @FixMethodOrder, which was a temporary work-around for Java no longer guaranteeing the order that methods are returned from the reflection APIs
  3. It's not that hard to write a custom annotation and an associated Ordering
  4. We could update@Test later after we see how people use Ordering

@dsaff
Copy link
Member

dsaff commented Jun 5, 2018

Momentarily delurking to say that I think it would be good for the JUnit design if @FixMethodOrder was used less and less, not more and more. (Basically everything @kcooney said.)

@cosmos-fly
Copy link
Author

cosmos-fly commented Jun 5, 2018

@kcooney @dsaff , thanks for your explanations.
I think I caught the key point. @FixMethodOrder is only a temporary work-around. @kcooney Do you mean this annotation is a deprecatory solution, or it will be removed or marked by @deprecated in the future?

@kcooney
Copy link
Member

kcooney commented Jun 5, 2018

@JacobCN if Ordering is added in 4.13 then I will probably deprecate @FixMethodOrder. If past behavior is any predictor of future behavior, we won't remove it until after the thermal death of our sun.

@cosmos-fly
Copy link
Author

Got it, I close this PR now.

@cosmos-fly cosmos-fly closed this Jun 6, 2018
@marcphilipp
Copy link
Member

FWIW I agree that we should rather get #1130 merged so this can be implemented as an extension.

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.

4 participants