tearDown() not called for an ignored test

[throwtheswitch]

tearDown() not called for an ignored test

[mssimo]

Agreed with above.

And of course you should be able to use asserts in tearDown but in my opinion those asserts should just check that things have been cleaned nicely for the next setUp / test cases to be run. Thus, tearDown cleans the things that have been setup in setUp, nothing more. If a test does some more initialization, then it is the respobsibility of a test to do the cleanup, not the tearDown.

What about inroducing a new macro TEST_IGNORE_NO_TEARDOWN() if current functionality needs to be supported.

[emidttun]

There are a number of users who put assertions in their tearDown functions. This creates a
challenging flow:
...

There might be a good reason to do this, but personally I don't see it. Also, if an assertion fails in teardown because the test was not run I think there is something wrong with the test design.

I think the correct thing would be to not run setup() if the test is ignored. But it seems difficult with the current design. Otherwise I think the user should be responsible for designing the test cases correctly. Maybe there is a way to improve the output so that the mistake is obvious.

[mvandervoord]

:) Yeah, this is a topic that comes up from time to time. We've reversed the way this works a number of times. Here's the problem: There are a number of users who put assertions in their tearDown functions. This creates a challenging flow:

setup is run
the test is run: encountering an IGNORE statement, it immediately leaves
tearDown is run: encountering a failing assertion, it now flags the test as a failure and aborts the tearDown.

The solution would be to have every assertion double-check if there is an ignore in effect, but this is a lot of additional overhead for a weird corner case.

We've talked about expanding the options for functions... something like:

setUp - for memory initialization, etc... no assertions here
preTest - run before each test. assertions allowed here
(test)
postTest - run after each test. assertions allowed here. only run when test passed.
tearDown - run after each test. no assertions here. always run

There are a few downsides:

(1) on many platforms, these are just more functions that someone has to define in order to use the tools... they have no intention of actually using them. (yuck, boilerplate!)
(2) there's not really a way to enforce the no-assertion rules... or at least not without additional overhead in each assertion call.
(3) I threw out some names above... but adding two more functions to an already confusing naming scheme isn't going to clear things up, most likely. ;)

Anyway, that's what we're up against here. Thoughts? I'd love to get more opinions.

Mark

[jsalling]

@mvandervoord commented

The solution would be to have every assertion double-check if there is an ignore in effect

It appears this check happens already. UNITY_SKIP_EXECUTION bypasses all asserts when ignored.
(BTW, could we change the name of that macro to be more descriptive? I can open another issue. #239 Name changed to RETURN_IF_FAIL_OR_IGNORE)

The flow:
If Unity ignore didn't skip teardown, all tearDown() code would be run except assertions' internals, which would not fail.

This feels like the best choice to me. I tried it out, and all tests still passed. This includes one test that calls a failing TEST_ASSERT in teardown.
I can provide a PR.

[jsalling]

I move to close - issue resolved in above PR.