On Mon, 2010-06-28 at 14:40 -0400, Greg Stein wrote:
> On Mon, Jun 28, 2010 at 10:09, <julianfoad_at_apache.org> wrote:
> > Author: julianfoad
> > Date: Mon Jun 28 14:09:33 2010
> > New Revision: 958583
> >
> > URL: http://svn.apache.org/viewvc?rev=958583&view=rev
> > Log:
> > * subversion/tests/svn_test_main.c
> > (main): Configure the Subversion library to raise an error on assertion
> > failure, so we can catch it and report a test failure instead of
> > aborting the test run.
>
> No.
>
> I said the same thing to Stefan a while back when he tried to make a
> similar change. Those assertions should continue to fail as they
> normally do. When I'm testing, I *want* that core dump.
I think the root of the problem is the C test harness code runs the code
under test in-process. Therefore if code-under-test crashes, test suite
fails to report results. The correct fix would be: test harness runs
code-under-test in a subprocess and catches any crashes.
The change I made is to make the harness less likely to bomb out, more
likely to report test results, when there's (a specific kind of)
undefined behaviour in the code under test. However, there are two use
cases: (1) a full test run with reporting; (2) running the test in a
debugger order to debug it. I was helping (1); sounds like (2) is your
concern.
To accommodate both (1) and (2) without rewriting the harness to run a
sub-process, I think the catch-assertion-failures should be enabled when
run within the (greater) test suite, and disabled when being run as an
individual executable (such as in your debugger).
Rationale for catching and rporting assertion failures in the code under
test is that such failures do happen during development, just like any
test failures, and there's no special reason why the developer who runs
into such a failure should have to go and fix it before he is able to
perform a full test suite run.
> The testing code should use SVN_TEST_ASSERT() or
> SVN_TEST_STRING_ASSERT(). These "assertions" will do the right thing,
> rather than dumping core. (Stefan was using normal assertions in his
> test code, which was the incorrect part)
I agree with that. My change was not intended to allow the use of
normal assertions in the test suite, but only to catch assertion
failures deep within the code under test.
I'll see if I can enable the trap via a flag passed in by the test
suite, and disable it by default, so it works as before in your
running-under-GDB scenario. Would that work for you?
- Julian
Received on 2010-07-01 15:28:10 CEST