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