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

Too easy to accidentally use Should().NotThrow() against async void lambda #711

Closed
zarenner opened this issue Dec 21, 2017 · 2 comments
Closed

Comments

@zarenner
Copy link
Contributor

With the current async support in FA, it's too easy in my opinion to accidentally use .Should().NotThrow() against an async void lambda. This will result in a test that looks valid but doesn't actually test the continuation.

Example:

Action asyncFunc = async () => { await Task.Delay(5000); throw new Exception("fail"); };
asyncFunc.Should().NotThrow();
// Test (incorrectly) passes

...instead of the correct test implementation:

Func<Task> asyncFunc = async () => { await Task.Delay(5000); throw new Exception("fail"); };
asyncFunc.Should().NotThrow();
// Test (correctly) fails

I see a couple of separate but complementary possible improvements:

  • Detect that the Action is an async method (e.g. action.Method.GetCustomAttribute(typeof(AsyncStateMachineAttribute)) != null), and throw. Would break anyone intentionally sending in an async void, but that seems unlikely. I don't believe it's possible to "fix" the async void method; if it was that would be another alternative.

  • Add actual async [Not]ThrowAsync methods that only work on Func<Task>. Wouldn't directly prevent this mistake if NotThrow was still used, but the added Async (and await) would make it more obvious when the async assertions aren't being used. I don't know if there was a reason AsyncFunctionAssertions was designed to block instead of using async/await (possibly because it would be a bit less fluent), but personally I'd much prefer actual async assertions.

@zarenner zarenner changed the title Too easy to accidentally use Should().NotThrow() against async void lamda Too easy to accidentally use Should().NotThrow() against async void lambda Dec 21, 2017
@dennisdoomen
Copy link
Member

Option 1 sounds like a decent solution.

@jnyrup
Copy link
Member

jnyrup commented Jan 27, 2018

Can this be closed with #737 merged?

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

No branches or pull requests

3 participants