On 25 October 2006 at 7:41, "Geir Magnusson Jr." <[EMAIL PROTECTED]> wrote: > > Cool - but why not just put into SVN somewhere?
Okay. classlib/trunk/support/tools/bin perhaps? -Mark. > either in enhanced/tools or classlib/trunk somewhere where it can be > invoked as an option by people from ant (so that we can wire it into the > CI system...) > > geir > > > Mark Hindess 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 > > > > > > > > >