harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.com>
Subject Re: [classlib][tests] Junit best practice
Date Wed, 25 Oct 2006 11:41:08 GMT
Cool - but why not just put into SVN somewhere?

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
> 
> 
> 
> 

Mime
View raw message