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 javaworld. [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 in > 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 that > I'd thrown together one evening that would handle multi-line asserts but > 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 hopefully > 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 > > > > -- Richard Liang China Development Lab, IBM
