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

Add support for Assert.EnterMultipleScope #816

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

manfred-brands
Copy link
Member

Fixes #769

Detects that code inside a using (Assert.EnterMultipleScope()) is inside a multiple context.
Adds a codefix for to use Assert.EnterMultipleScope.

@manfred-brands
Copy link
Member Author

When referencing NUnit >= 4.2.0

{11E56E81-99D5-405D-A9BB-F312078DFAE7}

And
{BE7F9BE2-6B67-4F8C-A87A-F909AF1127E2}

When referencing NUnit 3.14.0:
{6E962561-C638-4082-8F16-DCD0B05B85AA}

@manfred-brands manfred-brands force-pushed the Issue769_EnterMultipleScope branch from 23a6195 to 9fef008 Compare December 23, 2024 08:47
Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I'm happy to merge this when you are. Thanks for the improvement @manfred-brands

@@ -45,6 +45,38 @@ await Assert.MultipleAsync(async () =>
}");
RoslynAssert.Valid(this.analyzer, testCode);
}

#if WOULD_SOMEONE_ACTUALLY_USE_THIS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for why we wouldn't want to run this test :) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it won't work as we don't have support for it.
It would require tracking the declared variable and check if it is manually disposed somewhere within the scope.

Unless someone has a good reason for using this pattern, I don't see any, I won't spend my time on it.

@manfred-brands manfred-brands force-pushed the Issue769_EnterMultipleScope branch from 9fef008 to cf7eacb Compare January 8, 2025 06:10
@manfred-brands manfred-brands merged commit 18f6a70 into master Jan 8, 2025
6 checks passed
@manfred-brands manfred-brands deleted the Issue769_EnterMultipleScope branch January 8, 2025 07:47
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.

Recognized the new Assert.EnterMultipleScope() concept.
2 participants