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

NUnit should support async setup, teardown, fixture setup and fixture teardown #60

Closed
angularsen opened this issue Nov 23, 2013 · 15 comments

Comments

@angularsen
Copy link

This does not work as expected. Tests are still run as if fixture setup was OK.

[TestFixtureSetUp]
public async void FixtureSetUp()
{
    throw new Exception("Ex in FixtureSetUp");
}

This works as expected. Tests are not run and exception is written to test output.

[TestFixtureSetUp]
public void FixtureSetUp()
{
    throw new Exception("Ex in FixtureSetUp");
}

Only relevant for .NET 4.5 and later, which supports await/async C# identifiers.

@CharliePoole
Copy link
Contributor

There is currently no support for async setup, teardown, fixture setup or fixture teardown. In the example you give, the call returns before any code is even executed, so NUnit has no awareness of the exception being thrown.

Obviously, one would normally avoid use of async on any of these methods, but it is conceivable that they need to call other async methods and you might be forced to use it in the signature. It seems to me that the desired behavior on NUnit's part is to wait for the methods to complete, rather than to continue to fire off other methods.

I'm changing the title of the bug accordingly.

@angularsen
Copy link
Author

The following signature gives me "Ignored: Invalid signature for SetUp or TearDown method", but I had hoped returning Task would help NUnit understand it was awaitable as explained in http://simoneb.github.io/blog/2013/01/19/async-support-in-nunit/ .

[TestFixtureSetUp]
public async Task FixtureSetUp() {}

@CharliePoole
Copy link
Contributor

Nope. There's no support. If you read Simone's post closely, you'll see he only talks about test methods. Support for async test methods is also present in NUnit 3.0, where it's a bit more complex, since we may start other test methods while waiting. SetUp, TearDown, etc. are simpler in fact but still have to be implemented at the point where those calls are made.

Charlie

@CharliePoole
Copy link
Contributor

Marking this review so we can decide what action to take.

@CharliePoole CharliePoole added this to the 3.0 milestone Mar 2, 2014
@asbjornu
Copy link
Contributor

asbjornu commented Mar 4, 2014

I just read about the support for async in NUnit 2.6.2 and plowed ahead only to have this exact issue bite me because I tried to use an async void SetUp() to create a local variable that was later used in an async test. The local variable was of course null when the test was executed, causing it to fail.

Proper async support in SetUp and TearDown methods would be highly appreciated. I agree with @CharliePoole in that "the desired behavior on NUnit's part is to wait for the methods to complete, rather than to continue to fire off other methods".

@CharliePoole
Copy link
Contributor

This is still under review - which means we are deciding whether to do it at all. It would help if you had a motivating example for us, showing why a SetUp needs to be async.

@nzbart
Copy link

nzbart commented Mar 5, 2014

My thoughts would not be so much what can't be done without async (we would all code in assembly language by that rationale), but more that users, including myself, intuitively expected that this feature would be available and were confused and wasted time when SetUp didn't work the same way as the rest of the API, which does support async.

I think that support for async SetUp and TearDown should be provided for similar reasons to why async tests are supported - it allows us to write clean, idiomatic, async code without having to sprinkle .Wait() calls throughout tests to keep the test runner happy.

Less WTF moments and cleaner code leads to happier users.

@asbjornu
Copy link
Contributor

asbjornu commented Mar 5, 2014

What @nzbart said. The use case is simple: Creating a User object in a REST API with an async call through System.Net.HttpClient.SendAsync(). The User object will be used in all test methods, but is not what the tests are actually testing (these are integration tests). I now have to do this with .Wait() and would rather just decorate the setup method with async and use await.

It wouldn't be so bad if Wait() returned Task.Result, but it returns void, so it breaks up the code and makes it much less clear what's going on, which hurts maintenance and much of the point about testing being self-descriptive documentation of how the code works.

@CharliePoole
Copy link
Contributor

Thanks, that's helpful.
On Mar 5, 2014 12:02 AM, "Asbjørn Ulsberg" notifications@github.com wrote:

What @nzbart https://github.com/nzbart said. The use case is simple:
Creating a User object in a REST API with an async call through
System.Net.HttpClient.SendAsync(). I now have to do this with .Wait() and
would rather just decorate the setup method with async and use await.

It wouldn't be so bad if Wait() returned Task.Result, but it returns void,
so it breaks up the code and makes it much less clear what's going on,
which hurts maintenance and much of the point about testing being
self-descriptive documentation of how the code works.

Reply to this email directly or view it on GitHubhttps://github.com//issues/60#issuecomment-36718366
.

@CharliePoole
Copy link
Contributor

OK, based on the comments and our review, we'll accept this change as a new enhancement.

We will at least allow SetUp, TearDown, OneTimeSetUp and OneTimeTearDown to be async. We'll have to do a little analysis to figure out whether other user code that we call can also be async without adverse side effects.

Other examples of user code would inclued a method that provides data for a test or the code executed by an action attribute. Where possible, we should allow these to be async. If it turns out to not be possible, we should give some kind of error or warning message.

@asbjornu
Copy link
Contributor

asbjornu commented Mar 6, 2014

👍

@simoneb simoneb self-assigned this Mar 19, 2014
@davidpeden3
Copy link

Are there any updates on this issue?

@rprouse
Copy link
Member

rprouse commented Jun 22, 2014

@davidpeden3, This issue has been assigned to a developer and assigned to the 3.0 release, so we have committed to it, but AFAIK, it hasn't been worked on yet.

@davidpeden3
Copy link

@rprouse Great, thanks for the quick update.

@CharliePoole CharliePoole modified the milestones: 3.0Beta1, 3.0 Oct 7, 2014
@kobynet
Copy link

kobynet commented Oct 22, 2014

Would also like to see this feature.

johnmwright pushed a commit to johnmwright/nunit that referenced this issue Oct 28, 2019
Update supported framework runtime attribute to exclude maxversion, so that it is not restricted to current  .net Version. Also work around  a current VS CTP5 bug which prevents loading of vsix now.
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

8 participants