Mark Hindess wrote:
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?
Sure. Whatever you feel is best. I have no strong opinion. We do have
junit tests in DRLVM too, but we can "reach over" and use from there for
now.
geir
-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