Subject: Handling precondition failures

From: Alan Griffiths <[email-masked]>
Date: Thu, 13 Jun 2013 15:46:25 +0100

>From some review comments (reproduced below) it seems we're not entirely
in agreement on the policy of handling precondition failures.

I don't think my views are contentious, but before drafting a guideline
I'd like to poll opinions.

Essentially, if a precondition is violated then the code isn't
responsible for doing anything in particular. It could hide the problem,
it could detect and report the problem it could segfault.

Clearly, if it is cheap to test an error condition then it can be worth
doing so and reporting the problem. The meaning of "cheap" can be
different between debug and release builds - so it sometimes makes sense
to have something like assert/NDEBUG. In a lot of code it makes sense to
have the check on even in release builds (this is true of a lot of the
"assert" checks we have in initialization code).

But detecting and reporting precondition failures is not something that
code is responsible for, so I don't think we should have tests for it.

Thoughts?

~~~~

Extracts from:
https://code.launchpad.net/~vanvugt/mir/saucy-build/+merge/169108

> Instead of disabling the test I would propose replacing the assert() in the MirTestFramework contstructor with "if (...) abort()" as a temporary measure
....
> /1/ A test represents a requirement on the behavior of code.
>
> /2/ A precondition is something that code is allowed to assume (as it is the responsibility of the caller to ensure it holds).
>
> /3/ The code has no responsibility to behave in a particular way if preconditions are violated. Its behavior is unspecified (or even undefined).
>
> /4/ Tests that "preconditions violations" are reported correctly imply that they are not really preconditions - just conditions.
>
> The point: *if* we want some preconditions to be conditions in debug builds, then it makes sense for the corresponding tests to be conditionally compiled.
>
> Personally, I don't think these checks should ever be conditions *with tests to validate the behavior*. But unless there are performance reasons I feel that is is as important to aid problem diagnosis in release builds as in debug ones.
...
> If our error handling strategy depends on assert never being eliminated then that's obviously bad, but a discussion for elsewhere.

---

From: Daniel van Vugt <[email-masked]>
Date: Fri, 14 Jun 2013 09:38:47 +0800

I agree that we shouldn't require precondition checking. And we 
certainly shouldn't depend on it in the way our death-tests expect 
assertion.

Assertions are fine. So long as you always understand they won't always 
be there in release builds.

On 13/06/13 22:46, Alan Griffiths wrote:
>  From some review comments (reproduced below) it seems we're not
> entirely in agreement on the policy of handling precondition failures.
>
>> The point:*if*  we want some preconditions to be conditions in debug builds, then it makes sense for the corresponding tests to be conditionally compiled.
>>
>> Personally, I don't think these checks should ever be conditions*with tests to validate the behavior*. But unless there are performance reasons I feel that is is as important to aid problem diagnosis in release builds as in debug ones.
> ...