New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IErrorMessage doesn't give context on failure #75

Closed
citizenmatt opened this Issue Apr 28, 2014 · 3 comments

Comments

Projects
None yet
2 participants
@citizenmatt
Contributor

citizenmatt commented Apr 28, 2014

I'm not sure how to handle IErrorMessage. There's no context given with the message, so it's impossible to correlate with the test, case, collection, method or class that caused the error (especially with async and parallelisation). The ReSharper runner doesn't have the ability to report an arbitrary error message and even if it did, I'd still expect an error (such as exceptions in class or fixture ctor/Dispose) to be associated with the test element that caused it, most likely making it look like the test has failed.

Thoughts? Should IErrorMessage also include some kind of context? A correlation ID? Allow it to be cast to ITestMethodMessage or ITestCaseMessage or something?

@bradwilson

This comment has been minimized.

Show comment
Hide comment
@bradwilson

bradwilson Apr 29, 2014

Member

Not every failure has a straight line to a test. There are a LOT of things that happen on the way to getting something to run that can't be directly attributed to any one particular test case. Theoretically, this is a "oh god, the world fell apart" message, which means that having "nice" handling for one probably isn't necessary.

I'd be tempted to toss up some catastrophic failure dialog box in response to this.

Member

bradwilson commented Apr 29, 2014

Not every failure has a straight line to a test. There are a LOT of things that happen on the way to getting something to run that can't be directly attributed to any one particular test case. Theoretically, this is a "oh god, the world fell apart" message, which means that having "nice" handling for one probably isn't necessary.

I'd be tempted to toss up some catastrophic failure dialog box in response to this.

@citizenmatt

This comment has been minimized.

Show comment
Hide comment
@citizenmatt

citizenmatt Apr 29, 2014

Contributor

They're not catastrophic errors - they can be raised multiple times and they don't stop execution. And perhaps more importantly, it's (usually) user code that's causing them to fail. Specifically, test class and collection fixtures throwing in Dispose (and ctor for xunit1). As a user, I'd expect the test framework to gracefully handle exceptions from my code.

Xunit1 had ExceptionThrown and ClassFailed messages. Xunit2's IErrorMessage is the same as ExceptionThrown, but we're missing an equivalent of ClassFailed, and it's lack of these error messages that are causing this issue.

ExceptionThrown was a catastrophic error - catching any exceptions thrown by the test framework, such as ambiguous matches on method names. ClassFailed was called when either the class's fixture's constructor or Dispose threw an exception.

Currently, if a fixture's constructor throws, it's reported as part of the test result (and the test is still run!?) but if a fixture's Dispose throws, it's reported as a catastrophic error. Similarly, the change I made to the xunit1 runner should have been a ClassFailed style message, but has to be IErrorMessage. All of these cases have the context for a ClassFailed (or CollectionFailed) style message.

Surprisingly, the only genuine catastrophic error that should still raise an IErrorMessage (i.e. it's infrastructure could causing the error, not user code) is an error thrown when trying to run xunit1 tests - most likely an ambiguous match exception. Any similar errors caused by xunit2 are thrown as exceptions (e.g. can't find xunit.dll or xunit.execution.dll)

Should there be ClassFailed and CollectionFailed messages?

Contributor

citizenmatt commented Apr 29, 2014

They're not catastrophic errors - they can be raised multiple times and they don't stop execution. And perhaps more importantly, it's (usually) user code that's causing them to fail. Specifically, test class and collection fixtures throwing in Dispose (and ctor for xunit1). As a user, I'd expect the test framework to gracefully handle exceptions from my code.

Xunit1 had ExceptionThrown and ClassFailed messages. Xunit2's IErrorMessage is the same as ExceptionThrown, but we're missing an equivalent of ClassFailed, and it's lack of these error messages that are causing this issue.

ExceptionThrown was a catastrophic error - catching any exceptions thrown by the test framework, such as ambiguous matches on method names. ClassFailed was called when either the class's fixture's constructor or Dispose threw an exception.

Currently, if a fixture's constructor throws, it's reported as part of the test result (and the test is still run!?) but if a fixture's Dispose throws, it's reported as a catastrophic error. Similarly, the change I made to the xunit1 runner should have been a ClassFailed style message, but has to be IErrorMessage. All of these cases have the context for a ClassFailed (or CollectionFailed) style message.

Surprisingly, the only genuine catastrophic error that should still raise an IErrorMessage (i.e. it's infrastructure could causing the error, not user code) is an error thrown when trying to run xunit1 tests - most likely an ambiguous match exception. Any similar errors caused by xunit2 are thrown as exceptions (e.g. can't find xunit.dll or xunit.execution.dll)

Should there be ClassFailed and CollectionFailed messages?

@citizenmatt citizenmatt referenced this issue May 2, 2014

Merged

Xunit2 support #1

4 of 9 tasks complete
@bradwilson

This comment has been minimized.

Show comment
Hide comment
@bradwilson

bradwilson May 2, 2014

Member

We'll look at this for beta 3.

Member

bradwilson commented May 2, 2014

We'll look at this for beta 3.

@bradwilson bradwilson added this to the 2.0-beta-3 milestone May 2, 2014

@bradwilson bradwilson closed this Jun 30, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment