Add MSTEST0067 analyzer to flag Thread.Sleep/Task.Wait/Task<T>.Result in tests#8646
Merged
Merged
Conversation
… in tests Fixes #6950. Adds a new MSTEST0066 'AvoidThreadSleepAndTaskWaitInTests' analyzer that reports an Info-level diagnostic when one of the following blocking APIs is invoked inside a test method or fixture method (TestInitialize/TestCleanup/ClassInitialize/ClassCleanup/AssemblyInitialize/AssemblyCleanup/GlobalTestInitialize/GlobalTestCleanup): - Thread.Sleep(...) - Task.Wait(...) (including Task<T>.Wait) - Task<T>.Result The walk also looks through local functions and lambda bodies declared inside such methods so a Thread.Sleep buried in a callback still surfaces. The issue text mentioned 'Thread.Wait' which does not exist on System.Threading.Thread; this is interpreted as Task.Wait, which is the actual flakiness/deadlock source. Scope was extended to Task<T>.Result for the same reason. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds MSTEST0066, a new MSTest analyzer that flags blocking waits in test and fixture methods to reduce test flakiness and sync-over-async deadlock risk.
Changes:
- Adds analyzer logic for
Thread.Sleep,Task.Wait, andTask<TResult>.Resultin MSTest test/fixture methods, including lambdas/local functions. - Registers MSTEST0066 in analyzer IDs and release tracking.
- Adds analyzer resources, localized XLF placeholders, and unit coverage for key C# scenarios.
Show a summary per file
| File | Description |
|---|---|
src/Analyzers/MSTest.Analyzers/AvoidThreadSleepAndTaskWaitInTestsAnalyzer.cs |
Implements MSTEST0066 analyzer logic. |
src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md |
Registers MSTEST0066 as an unshipped rule. |
src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs |
Adds MSTEST0066 diagnostic ID constant. |
src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs |
Adds System.Threading.Thread metadata name. |
src/Analyzers/MSTest.Analyzers/Resources.resx |
Adds title/message/description resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf |
Adds localization units for new resources. |
src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf |
Adds localization units for new resources. |
test/UnitTests/MSTest.Analyzers.UnitTests/AvoidThreadSleepAndTaskWaitInTestsAnalyzerTests.cs |
Adds unit tests for diagnostics and non-diagnostic cases. |
Copilot's findings
- Files reviewed: 19/19 changed files
- Comments generated: 2
- Fix XML doc rule ID (MSTEST0064 -> MSTEST0066) on the analyzer summary. - Expand rule title to also mention Task<TResult>.Result, matching the analyzer's reported scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Addressed the two Copilot review comments in e712ed4:
Not adopting the github-code-quality '.Select(...)' suggestion: it would force adding 'using System.Linq' for a single-line projection that doesn't improve readability, and the current foreach is consistent with how other analyzers in this codebase iterate attribute data. |
…verage - Detect static Task.WaitAll / Task.WaitAny as blocking calls. - Update title/description resources and regenerate xlf files. - Add tests for ClassCleanup, AssemblyCleanup, GlobalTestInitialize, GlobalTestCleanup. - Add test for custom attribute deriving from TestMethodAttribute. - Add expression-bodied test method case. - Add VB smoke tests (Thread.Sleep and Task(Of T).Result). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md # src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6950.
Adds a new
MSTEST0067AvoidThreadSleepAndTaskWaitInTestsanalyzer that reports anInfolevel diagnostic when one of the following blocking APIs is invoked inside a test method or fixture method:Thread.Sleep(...)Task.Wait(...)(includingTask<T>.Wait)Task.WaitAll(...)/Task.WaitAny(...)Task<T>.ResultThe analyzer covers methods marked with any of:
[TestMethod](and types deriving fromTestMethodAttributesuch as[DataTestMethod]),[TestInitialize],[TestCleanup],[ClassInitialize],[ClassCleanup],[AssemblyInitialize],[AssemblyCleanup],[GlobalTestInitialize],[GlobalTestCleanup]. The walk also looks through local functions and lambda bodies declared inside such methods so aThread.Sleepburied in a callback still surfaces.Notes on scope
Thread.Waitwhich does not exist onSystem.Threading.Thread. I interpreted that asTask.Wait, which is the actual flakiness/deadlock source.Task<T>.Resultfor the same reason — it is a sync-over-async blocking call that is just as problematic asTask.Waitin tests.SemaphoreSlim.Waitis intentionally not flagged — it is a synchronization primitive, not a sync-over-async pattern.Info(the issue suggestedInfoorWarning). Conservative choice to avoid breaking existing test suites; users can elevate via.editorconfig.Note on rule ID
Originally authored as
MSTEST0066, this analyzer was renumbered toMSTEST0067after rebasing onmainbecauseMSTEST0066was concurrently assigned toIgnoreShouldHaveJustificationAnalyzer. The branch name still uses the oldmstest0066slug for historical reasons.Files
src/Analyzers/MSTest.Analyzers/AvoidThreadSleepAndTaskWaitInTestsAnalyzer.cs— new analyzer (C# + VB).src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs— newMSTEST0067constant.src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs— addedSystem.Threading.Thread.src/Analyzers/MSTest.Analyzers/Resources.resx+ auto-generated xlf files.src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md— registers the new rule.test/UnitTests/MSTest.Analyzers.UnitTests/AvoidThreadSleepAndTaskWaitInTestsAnalyzerTests.cs— unit tests covering positive and negative cases (Thread.Sleep variants, Task.Wait overloads, Task.WaitAll/WaitAny, Task.Result, await, helper methods, non-test classes, local functions, lambdas, SemaphoreSlim).All analyzer unit tests pass locally.