Skip to content
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

Skip tests even if SkipException is coming from within Assert.Throws #12

Merged
merged 5 commits into from Oct 3, 2018

Conversation

fredrikhr
Copy link
Contributor

@fredrikhr fredrikhr commented Sep 26, 2018

Updated in #12 (comment)


As described in xunit/xunit#1722 the Xunit.Sdk.ThrowsException that is thrown from a failed execution of Assert.Throws (and related) does not capture the exception actually thrown.

The thrown Exception message does however contain the string Actual: typeof(Xunit.SkipException) if a SkipException was thrown inside the action or function passed to Assert.Throws.

Although the solution to search the ThrowsException message for that string is far from elegant, it works and can be replaced when xunit/xunit#1722 has been fixed.

Fixes #10.

@AArnott
Copy link
Owner

AArnott commented Sep 27, 2018

@couven92 Thanks for contributing. It looks to me though that my fix for the xunit bug you mention is already completed and in the xunit 2.4 release available on NuGet. So can we rework this PR with the more elegant fix that you mention?

@fredrikhr
Copy link
Contributor Author

@AArnott thanks for the tip. If have updated the code. It now checks whether the first element is Xunit.Sdk.ThrowsException and checks if the exception that directly follows is skippable.

I hope it's okay that I have used newer language features (i.e. a pattern-matching switch block)

I ran dotnet outdated to update the package references to newer versions.

@AArnott
Copy link
Owner

AArnott commented Oct 3, 2018

Thanks. Yes, it's fine to use newer C# features. I personally find this switch statement harder to read than a single if/else pair (I reserve switch statements to more than two cases), but it's not a big deal.
Thanks for your contribution.

@AArnott AArnott merged commit c7f20ea into AArnott:master Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skip tests even when skipping exception is wrapped by ThrowsException
2 participants