1

Resolved

Exceptions from Dispose method should be aggregated with exceptions from test method

description

On occasion I like to use the Dispose() method on a test class to perform general asserts that should hold for every test case:

public class Tests : IDisposable {
public void Dispose() {
    // general asserts
}

[Fact]
public void Test() {
    // specific asserts
}
}

However, the current behavior is that an assert failure in the Dispose method will blow away any assert failures in the test method. This makes it tricky to debug regressions in tests because the UI presents incomplete information.
It would be nice if exceptions from the test method were aggregated with exceptions from the Dispose method, such that if something fails we see a full picture of what's going on (and make sure to preserver the order: test method first, Dispose second()).

comments

bartelink wrote Sep 6, 2012 at 11:13 PM

Could a composite assertion help ? (http://xunit.codeplex.com/discussions/390946)

However even with such a facility, I prefer to have each assertion be a separable thing - i.e. the last thing you want is something that can fail for 2 reasons at the same time. (Just explaining not +1ing)

marcind wrote Sep 7, 2012 at 1:29 AM

I don't really have an opinion about composite assertions.

The thing that bothers me about the current xUnit behavior is that exceptions in a test's Dispose method mask any exceptions in the test itself.

So I guess what I should have said originally is that I would prefer a change where exceptions from the Dispose method only get reported if the test method finished successfully. If the test threw an exception, then the Dispose method should still run (for cleanup purposes, etc) but exceptions thrown from it should not overwrite the exception from the test. Whether they get aggregated or the exception from the test method "wins" doesn't really matter to me that much (though if they did get aggregated, that would potentially save me another test rerun once the main exception in the test method is fixed).

bartelink wrote Sep 7, 2012 at 9:19 AM

Thanks for expanding; right, that makes sense - you'd get a +1 from me if you logged that.

In the meantime the extra info in here should allow the guys to triage what you're after here better too.

LOBOMINATOR wrote Sep 7, 2012 at 1:23 PM

For me asserting stuff in the Dispose() method is clearly a unit testing smell. What is the intent of the asserts? Putting it into a dispose method hides the intent completely. Rather write another test which describes what is being asserted or even another fixture. So I vote against this issue!

BradWilson wrote Jun 30, 2013 at 7:10 PM

This is fixed in v2.