On 26 October 2006 at 16:33, "Alexei Zakharov" <[EMAIL PROTECTED]> wrote: > > BTW - the script does run on Windows with ActiveState's Perl runtime. > > > > Number of Issues by module > > > > 45 beans > > I've also checked it on WinXP + cygwin + perl v5.8.7. Also works good > (HARMONY-1976).
Thanks Alexei. Your patch illustrates an issue that I forgot to mention... Another good reason not to automatically fix the "faults" raised by my script is that they are often indications that there are other similar but not so easily mechanically-spotted problems with the tests. For instance, you might have code like (this is a terrible example, sorry): assertEquals(actual, 1); assertEquals(actual, expected); My script will only spot the former because it is an obvious constant, but a human-being would (hopefully) spot the second when they go to fix the first one. Alexei has done a great job of reviewing the tests and fixing the non-obvious issues. I especially like the fact that fixing the exception handling shortens many tests and makes them much more readable. Alexei, thanks for taking the time to do this. Regards, Mark. > 2006/10/26, Nathan Beyer <[EMAIL PROTECTED]>: > > BTW - the script does run on Windows with ActiveState's Perl runtime. > > > > -Nathatn > > > > On 10/25/06, Richard Liang <[EMAIL PROTECTED]> wrote: > > > Awesome. I just plan to review our junit tests, now it seems your tool > > > has done what I want ;-) > > > > > > I highly recommend we read this article "JUnit best practices"[1] in java > world. > > > > > > [1] http://www.javaworld.com/javaworld/jw-12-2000/jw-1221-junit.html > > > > > > On 10/25/06, Mark Hindess <[EMAIL PROTECTED]> wrote: > > > > > > > > Earlier in the year we discussed junit best practice. For example, > > > > making sure assertEquals calls have the expected and actual arguments i > n > > > > the correct order to avoid getting confusing failure messages. > > > > > > > > Robert posted a script a week or so ago, to look for some of junit > > > > issues but it didn't handle asserts that spanned multiple lines so, > > > > unfortunately, it was missing the majority of them. I had a script tha > t > > > > I'd thrown together one evening that would handle multi-line asserts bu > t > > > > annoyingly (because it read the whole file at once) couldn't report the > > > > line number of the potential issue as Robert's script did. > > > > > > > > Inspired by Robert's post, I looked at my script again. I've now fixed > > > > it to report line numbers, added a little bit of documentation and > > > > attached it to a JIRA: > > > > > > > > https://issues.apache.org/jira/browse/HARMONY-1960 > > > > > > > > It finds quite a lot of potential problems (I've appended a summary of > > > > the findings below). (There will be a few false positives but hopefull > y > > > > not too many.) It would be nice to fix these issues - I fixed several > > > > hundred while testing the script - but more importantly we should make > > > > sure we avoid adding any new issues. > > > > > > > > Improvements to the script would be most welcome. > > > > > > > > Regards, > > > > Mark. > > > > > > > > Types of issue identified > > > > > > > > 4949 should possibly use assertEquals > > > > 815 actual may be a constant > > > > 437 consider using separate asserts for each '&&' component > > > > 330 exception may be left to junit > > > > 135 actual *may* be a constant > > > > 48 should be fail (always false) > > > > 40 should be fail (always true) > > > > 20 expected is null - should use assertNull > > > > 12 consider using separate asserts for each '||' component > > > > 8 expected is false - should use assertFalse > > > > 7 expected is true - should use assertTrue > > > > 1 should use assertNotNull > > > > > > > > > > > > Number of Issues by module > > > > > > > > 1907 luni > > > > 1440 swing > > > > 699 math > > > > 611 security > > > > 335 text > > > > 322 awt > > > > 222 sound > > > > 186 nio > > > > 178 jndi > > > > 123 archive > > > > 118 auth > > > > 117 crypto > > > > 116 logging > > > > 91 nio_char > > > > 87 print > > > > 74 regex > > > > 68 concurrent > > > > 45 beans > > > > 41 x-net > > > > 21 sql > > > > 1 rmi > > > -- > Alexei Zakharov, > Intel Enterprise Solutions Software Division >
