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

Shuffle the test methods in the same class(for junit4) #5

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

changgengli
Copy link

Hello,

This PR is to randomize the excution order the junit test method in the same class. I came to this idea during the work we are upgrading from java 6 to java 8. A lot of test cases were broken after upgrade because those tests are relying on the execution order which is changed after upgrade.

This is actually just a hack for several hours. I can rework on it after I get more suggestion and more understanding to ant(eg. have a parameter to turn it off). But first, would this be valuable for others?

Thanks,
Changgeng

@mc1arke
Copy link
Member

mc1arke commented Dec 23, 2014

Some pointers for you:

  • You're introducing a runtime dependency on JUnit 4 by referencing a JUnit class directly. There are still users of Ant who only use JUnit 3, so JUnit 4 classes should be loaded reflectively.
  • Your method of using a random number in a comparator may work, but it breaks the contract of comparators being reversible (e.g. sgn(compareTo(o1, o2) == -sgn(compareTo(o2, o1))), and could therefore break with future versions of Java or JUnit. You'll probably need to make your algorithm deterministic to overcome this, which may mean your method order ends up only being pseudo-random. I'm happy to discuss this point to help find a technical solution, since it will have the biggest outcome on what this feature does.
  • Ideally I'd want a way to be able to re-run the suite in the order of a previous execution (e.g. where I'd seen a new failure that I needed to investigate), so would probably want a way of seeding any random numbers used in this solution, and have the seed reported to me during execution so I can replay it at a later point
  • I know this is only an initial concept, but I'd want to see some tests that prove this feature and protect it in future changes
  • Use the generics in comparator - you may not currently make use of the parameters passed into the compareTo method, but that doesn't mean someone wont want to in the future, and future changes shouldn't have to require refactoring existing code before introducing a feature.
  • As you've mentioned, this would definitely need a flag to turn it on (it should default to off since it could break tests that only work due to the order they're currently being run)

Whilst I've raised a few points above, I can see merit in the concept of what you're doing here and think there would be value in it being progressed.

Thanks,
Michael

Changgeng Li added 4 commits December 23, 2014 12:58
* semi-pesudo randomized comparator to sort the Description instances. By supplying the same seed, the order is deterministic
* Using reflection to invoke the comparator.apply method so that it won't break for junit 3 users
* add shuffleTests switch to turn it on
* add shuffleSeed switch to supply the seed
* semi-pesudo randomized comparator to sort the Description instances. By supplying the same seed, the order is deterministic
* Using reflection to invoke the comparator.apply method so that it won't break for junit 3 users
* add shuffleTests switch to turn it on
* add shuffleSeed switch to supply the seed
@changgengli
Copy link
Author

Hello Michael,

Except for "some tests that prove this feature and protect it in future changes", all other concerns should have been addressed.

Please let me know how do you think of it now. I should be able to work out the tests during holiday break.

Thanks,
Changgeng

Don't sort if the test methods are specified
@mc1arke
Copy link
Member

mc1arke commented Dec 27, 2014

Your changes look reasonable from a quick scan of them, and I'm impressed with the speed you made them. I'll give it a more thorough review once you've got some tests in.

@changgengli changgengli changed the title My 194 Shuffle the unit tests order in the same class(for junit4) Dec 31, 2014
@changgengli
Copy link
Author

I've add an antunit test. Please review.

@changgengli changgengli changed the title Shuffle the unit tests order in the same class(for junit4) Shuffle the test methods in the same class(for junit4) Dec 31, 2014
@changgengli
Copy link
Author

Michael,

Have you got a chance to review the code?

Thanks,
Changgeng

@mc1arke
Copy link
Member

mc1arke commented Aug 30, 2015

@changgengli sorry, I forgot about this and your change now doesn't merge (I think it just needs master pulled back on to your branch). If you're still interested in contributing this functionality then could you get the change back into a merge-able state and I'll give it another review for you?

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.

2 participants