commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [MATH] Documenting test failures
Date Sun, 06 Sep 2009 20:23:32 GMT
sebb wrote:
> On 05/09/2009, Luc Maisonobe <Luc.Maisonobe@free.fr> wrote:
>> Phil Steitz a écrit :
>>
>>> sebb wrote:
>>  >> There are quite a lot of assertions that don't give any details if a test
fails.
>>  >>
>>  >> For example, the following currently fails when using Harmony:
>>  >>
>>  >> testIncreasingTolerance(org.apache.commons.math.ode.nonstiff.AdamsBashforthIntegratorTest)
>>  >> java.lang.AssertionError:
>>  >>      at org.junit.Assert.fail(Assert.java:74)
>>  >>      at org.junit.Assert.assertTrue(Assert.java:37)
>>  >>      at org.junit.Assert.assertTrue(Assert.java:46)
>>  >>      at org.apache.commons.math.ode.nonstiff.AdamsBashforthIntegratorTest.testIncreasingTolerance(AdamsBashforthIntegratorTest.java:89)
>>  >>
>>  >> Obviously one can find the code from the line number, but that does
>>  >> not actually get one much further as there's no indication what the
>>  >> values are:
>>  >>
>>  >>             assertTrue(handler.getMaximalValueError() < (36.0 *
>>  >> scalAbsoluteTolerance));
>>  >>
>>  >> Seems to me it might be useful to create some additional assert
>>  >> methods, for example:
>>  >>
>>  >> assertLessThan([message,] maximum, actual)
>>  >>
>>  >> which would report the max and actual values if the check failed.
>>  >>
>>  >> As far as I can tell, this is not currently supported by JUnit3/4 -
>>  >> only assertSame/assertEquals report the expected and actual values.
>>  >>
>>  >> WDYT?
>>  >>
>>  >> There are also lots of cases of assert() calls without a message;
>>  >> again this means digging into the test code to find the problem.
>>  >>
>>  >> I propose to add messages to at least the ones that Harmony triggers. OK?
>>  >
>>  > I am fine with adding messages, as long as they are not misleading.
>>
>>
>> I'm not a big fan of messages in Junit tests. These tests are mainly
>>  used in two successive contexts:
>>
>>   - first during code development (sometimes in a test driven process),
>>   - second during long terme maintainance
>>
>>  In the first case the tests may fail but the developer already knows
>>  exactly the purpose of each test.
> 
> Only the original developer is likely to know immediately what the problem is.

No, not if the code and test are well-written.

> If it has been a long time since the code was written - or if another
> developer is involved - then it is likely that the test failure cannot
> be resolved without adding debugging code, i.e. improving the error
> message.

Here again, I am not sure I agree.  Sometimes we do end up having to
add or refine test cases, and even resort to adding debugging code
to figure out what is going on in a failure in [math], but I am not
sure adding messages would help all that much.  By looking at the
test code, you can generally tell pretty quickly *what* has failed
(which is all a message will help elucidate).  The problem is
figuring out why, and to make that easier I think we get more bang
from adding more test cases, improving the readability of the test
cases and the code, and documenting the code better.  We really
appreciate contributions in any of these areas.

> 
>> In the second case the tests should
>>  never fail and when they do people always have to look at both the test
>>  code and library code, so the message is merely a convenience.
> 
> Not necessarily - sometimes upgrades to Java or OS can cause failures
> in tests that never failed before.

That should not happen in [math].  If it is happening, it needs to
be carefully investigated.  If you are finding the same failures for
Harmony regularly, I am OK adding messages to flag these; but I see
this as pathological. We have tested current and previous versions
of [math] on *many* OS/JDK combinations with only a tiny handful of
failures (jdk 1.3 IIRC had a bug with trig function evaluation that
caused some tests to fail), so this should not be a regular occurrence.

> 
>>  What
>>  bothers me is that these messages are a pain to maintain
> 
> If written correctly initially, there should be no maintenance involved.
> 
>>  and sometimes
>>  as new tests are added by copying an existing test and changing a few
>>  things, the messages are often not changed in the process.
> 
> A stitch in time saves nine ...
> 
>>  Last thing is
>>  that due to lazyness sometimes we get things like:
>>
>>   assertEquals("a != b", a, b);
>>
>>  which clearly is not informative.
> 
> Agreed, in this case JUnit shows the values anyway, so the message
> tells us nothing extra.
> 
> However, my concern is with assertions of the form:
> 
> assertTrue(a==b)
> or
> assertTrue(a<b)
> 
> where the JUnit message is (necessarily) no use.
> 
>>  So I would say that if you want to add such tests, you can do it but I
>>  think this will be a huge task for small improvements.
> 
> I was planning to add the messages for tests that break with Harmony.

As I said before, I am OK with this, though I see this as more of
interest to Harmony than [math] (unless there is reason to believe
that the failures are real failures in [math] code).
> 
> However, I think developers should try to ensure that should any new
> tests fail then the message is useful.

I agree with the principle that test failures should be as
informative as possible.  I am not sure adding messages everywhere
is the best way to ensure that in general.

Phi
> 
>>  Luc
>>
>>
>>  >
>>  > Phil
>>  >> ---------------------------------------------------------------------
>>  >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>  >> For additional commands, e-mail: dev-help@commons.apache.org
>>  >>
>>  >
>>  >
>>  > ---------------------------------------------------------------------
>>  > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>  > For additional commands, e-mail: dev-help@commons.apache.org
>>  >
>>  >
>>
>>
>>  ---------------------------------------------------------------------
>>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>  For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message