On Thu, Jul 1, 2010 at 09:20, Julian Foad <julian.foad_at_wandisco.com> wrote:
>...
> 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.
Yup. Of course, that could mean that sub-tests 7..10 are not run, but
the harness might even be able to restart the test and avoid the
crashing test.
More work than I think we care to do :-P ... especially given your
solution below.
> 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.
Yes.
> 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.
Okee doke. That's fair!
> 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?
It sure does, and I reviewed the changes. Looks great... thanks!!
Cheers,
-g
Received on 2010-07-01 22:53:00 CEST