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

NUnit1032 on MsSqlContainer.DisposeAsync() #781

Open
perlun opened this issue Sep 9, 2024 · 5 comments
Open

NUnit1032 on MsSqlContainer.DisposeAsync() #781

perlun opened this issue Sep 9, 2024 · 5 comments

Comments

@perlun
Copy link

perlun commented Sep 9, 2024

Hi,

With the following test class:

public class MsSqlContainerTest {
    private MsSqlContainer _msSqlContainer;

    [OneTimeSetUp]
    public void OneTimeSetup() {
        _msSqlContainer = new MsSqlBuilder().Build();
    }

    [OneTimeTearDown]
    public void OneTimeTearDown() {
        _msSqlContainer.DisposeAsync().AsTask().Wait();
    }
}

...I get a warning/error saying 17>SomeControllerTests.cs(84,28): Error NUnit1032 : The field _msSqlContainer should be Disposed in a method annotated with [OneTimeTearDownAttribute] (https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1032.md)

I realize why this happens; the MsSqlContainer (from https://dotnet.testcontainers.org/modules/mssql/) doesn't implement the IDisposable interface but only IAsyncDisposable, which is why we have to do it this way in the OneTimeTearDown method.

Should we consider this a shortcoming/deficiency in NUnit(.Analyzers) or should this be reported upstream to TestContainers.NET, WDYT? 🤔

@perlun
Copy link
Author

perlun commented Sep 9, 2024

(Hmm, I realize that making the OneTimeTearDown method be public async Task OneTimeTearDown() and just calling _msSqlContainer.DisposeAsync() works fine. 🙈 Perhaps this is just to be considered as user error on my part? 😁

I saw an exception message about async void methods not working as setup/teardown methods in NUnit but incorrectly thought it meant that all async was forbidden... which is not the case, only async void, which is very understandable)

@manfred-brands
Copy link
Member

I thought the rule recognized DisposeAsync?
Can you try

public Task OneTimeTearDown() {
       await _msSqlContainer.DisposeAsync();
    }

@manfred-brands
Copy link
Member

I should have read your second email before responding.

@perlun
Copy link
Author

perlun commented Sep 10, 2024

Yeah, but no worries. 👍 If we really wanted to make it perfect, I guess we could make the rule (or another rule...) warn about DisposeAsync() being used in a non-async OneTimeTearDown-method or something... but perhaps that's overly ambitious. 🙂

@manfred-brands
Copy link
Member

warn about DisposeAsync() being used in a non-async OneTimeTearDown-method

That is more generic than NUnit and hold for all Async inside non-Async method.

The Microsoft.VisualStudio.Threading.Analyzers does raise a warning for this:

image

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

No branches or pull requests

2 participants