On Thu, Jul 1, 2010 at 09:20, Julian Foad <julian.f...@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
