commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [MATH] Documenting test failures
Date Sun, 06 Sep 2009 20:39:16 GMT
On 06/09/2009, Phil Steitz <phil.steitz@gmail.com> wrote:
> 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.
>

Which is my point exactly ;-)

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

If the failure is due to:

assertTrue(a<b):

then you have to add debug code to find out what a and b are.

However, if the failure is due to

assertTrue("Expected a <"+a+"> to be less than <"+b+">b, a<b);

Then one already knows what a and b are, and this should simplify
further debugging - indeed may even eliminate it, e.g. if a or b are
values that may depend on the host.

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

In general, I agree.

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

There were a lot of failures with Harmony...

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

I'm just saying that when adding a test, developers should consider
whether the message that is produced should a failure occur will be
sufficient to determine the cause of the failure.

It's a lot easier to do this as tests are added rather than retrospectively.

There are some Math test cases where the bounds have been determined
by experimentation (this is in the comments); these are clearly tests
that have worked on some hosts and not on others, but even these don't
always have good error messages; fixing these should they fail again
will necessarily entail adding more "debugging" - aka better error
messages.

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

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


Mime
View raw message