You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Tony Wu <wu...@gmail.com> on 2006/11/02 03:41:09 UTC
Re: [classlib][tests] Junit best practice
hey guys,
I found there're same problem for assertSame like assertEquals.
should use assertNull, assertSame\s*\((.*,\s*null\s*|\s*null\s*,.*)\)\s*;
should use assertFalse, assertSame\s*\((.*,\s*false\s*|\s*false\s*,.*)\)\s*;
should use assertTrue, assertSame\s*\((.*,\s*true\s*|\s*true\s*,.*)\)\s*;
last argument should not be a constant,
assertSame\s*\(.*,\s*("[^"]*"|'[^']'|[+-]?\d+[0-9a-fA-FLlPp]*|[A-Z_]*)\s*\)\s*;
I'll update my checklist if no one object.
On 10/25/06, Mark Hindess <ma...@googlemail.com> 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
>
>
>
>
--
Tony Wu
China Software Development Lab, IBM
Re: [classlib][tests] Junit best practice
Posted by Richard Liang <ri...@gmail.com>.
On 11/2/06, Tony Wu <wu...@gmail.com> wrote:
> hey guys,
> I found there're same problem for assertSame like assertEquals.
>
> should use assertNull, assertSame\s*\((.*,\s*null\s*|\s*null\s*,.*)\)\s*;
> should use assertFalse, assertSame\s*\((.*,\s*false\s*|\s*false\s*,.*)\)\s*;
> should use assertTrue, assertSame\s*\((.*,\s*true\s*|\s*true\s*,.*)\)\s*;
> last argument should not be a constant,
> assertSame\s*\(.*,\s*("[^"]*"|'[^']'|[+-]?\d+[0-9a-fA-FLlPp]*|[A-Z_]*)\s*\)\s*;
>
> I'll update my checklist if no one object.
Please go for it ;-)
>
>
> On 10/25/06, Mark Hindess <ma...@googlemail.com> 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
> >
> >
> >
> >
>
>
> --
> Tony Wu
> China Software Development Lab, IBM
>
--
Richard Liang
China Development Lab, IBM