ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mc1arke <...@git.apache.org>
Subject [GitHub] ant pull request: My 194
Date Tue, 23 Dec 2014 08:30:22 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message