commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <>
Subject Re: svn commit: r1066176 - /commons/proper/math/trunk/src/test/java/org/apache/commons/math/analysis/function/
Date Wed, 02 Feb 2011 09:25:23 GMT
On Wed, Feb 02, 2011 at 01:07:28AM +0000, sebb wrote:
> On 2 February 2011 00:24, Gilles Sadowski <> wrote:
> > Hi.
> >
> >> Tidy up test
> >
> > Hmm. Not convinced.
> > Factorizing that way all the CM unit tests is a lot of work for a dubious
> > (IMO) improvement in readability.
> I'm not suggesting others do the work.

I didn't suggest that you did. Even if you do it, that doesn't change the
fact that it's a lot of work.

> Nor am I suggesting that all CM tests must be refactored this way.


> However, this particular test had a lot of unused variables.
> Furthermore, it did not actually test that the exceptions were thrown,
> because there were no fail() calls for the successful cases.
> As such, the test was not very useful.

Doh, sorry for that!
But your log message didn't mention it. [I guess that it is not as important
a change as the removal of redundant parentheses...]

> > I find it clearer to have a single "@Test" named "testPreconditions()" that
> > groups all the preconditions.
> But the test did not achieve anything...
> I think it's clear from this exercise that the new style makes it
> easier to write valid tests.

OK. I'm convinced now.

> > Also, sometimes, setting up the data to be passed (here to the constructor)
> > is not as easy to set up as in this case, so that you'd have to the setup in
> > every method (or use instance variables).
> I was not suggesting that.
> However, a test that has multiple potential failure points is not in
> general a good test, as the first failure hides any others.

Yes, but any failure must be corrected before committing, so that when the
first part passes, the next one is not hidden anymore, and if failing, one
can tackle that one, etc.
Anyway, from now on, I'll create tests as small as possible.

> [...]


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message