Github user mc1arke commented on the pull request:
https://github.com/apache/ant/pull/5#issuecomment-67930586
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
comparator](http://docs.oracle.com/javase/1.5.0/docs/api/java/util/Comparator.html#compare(T,%20T))s
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org
