ant icon indicating copy to clipboard operation
ant copied to clipboard

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

Open changgengli opened this issue 11 years ago • 6 comments

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

changgengli avatar Dec 20 '14 02:12 changgengli

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

mc1arke avatar Dec 23 '14 08:12 mc1arke

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

changgengli avatar Dec 23 '14 21:12 changgengli

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.

mc1arke avatar Dec 27 '14 10:12 mc1arke

I've add an antunit test. Please review.

changgengli avatar Dec 31 '14 18:12 changgengli

Michael,

Have you got a chance to review the code?

Thanks, Changgeng

changgengli avatar Jan 06 '15 20:01 changgengli

@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?

mc1arke avatar Aug 30 '15 17:08 mc1arke