Add analyzer to detect Thread.Sleep and Task.Wait in test methods

src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md

@@ -6,3 +6,4 @@
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
 MSTEST0058 | Usage | Info | AvoidAssertsInCatchBlocksAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0058)
+MSTEST0059 | Design | Warning | AvoidBlockingCallsInTestsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0059)

src/Analyzers/MSTest.Analyzers/AvoidBlockingCallsInTestsAnalyzer.cs

@@ -0,0 +1,142 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Immutable;
+
+using Analyzer.Utilities.Extensions;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+using MSTest.Analyzers.Helpers;
+
+namespace MSTest.Analyzers;
+
+/// <summary>
+/// MSTEST0059: <inheritdoc cref="Resources.AvoidBlockingCallsInTestsTitle"/>.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public sealed class AvoidBlockingCallsInTestsAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly LocalizableResourceString Title = new(nameof(Resources.AvoidBlockingCallsInTestsTitle), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString Description = new(nameof(Resources.AvoidBlockingCallsInTestsDescription), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.AvoidBlockingCallsInTestsMessageFormat), Resources.ResourceManager, typeof(Resources));
+
+    internal static readonly DiagnosticDescriptor AvoidBlockingCallsInTestsRule = DiagnosticDescriptorHelper.Create(
+        DiagnosticIds.AvoidBlockingCallsInTestsRuleId,
+        Title,
+        MessageFormat,
+        Description,
+        Category.Design,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    /// <inheritdoc />
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+        = ImmutableArray.Create(AvoidBlockingCallsInTestsRule);
+
+    /// <inheritdoc />
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+
+        context.RegisterCompilationStartAction(context =>
+        {
+            // Get the required symbols
+            if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingThread, out INamedTypeSymbol? threadSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask, out INamedTypeSymbol? taskSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestMethodAttribute, out INamedTypeSymbol? testMethodAttributeSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestInitializeAttribute, out INamedTypeSymbol? testInitializeAttributeSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestCleanupAttribute, out INamedTypeSymbol? testCleanupAttributeSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingClassInitializeAttribute, out INamedTypeSymbol? classInitializeAttributeSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingClassCleanupAttribute, out INamedTypeSymbol? classCleanupAttributeSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssemblyInitializeAttribute, out INamedTypeSymbol? assemblyInitializeAttributeSymbol) ||
+                !context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssemblyCleanupAttribute, out INamedTypeSymbol? assemblyCleanupAttributeSymbol))
+            {
+                return;
+            }
+
+            context.RegisterOperationAction(
+                context => AnalyzeInvocation(context, threadSymbol, taskSymbol, testMethodAttributeSymbol, testInitializeAttributeSymbol, testCleanupAttributeSymbol, classInitializeAttributeSymbol, classCleanupAttributeSymbol, assemblyInitializeAttributeSymbol, assemblyCleanupAttributeSymbol),
+                OperationKind.Invocation);
+        });
+    }
+
+    private static void AnalyzeInvocation(
+        OperationAnalysisContext context,
+        INamedTypeSymbol threadSymbol,
+        INamedTypeSymbol taskSymbol,
+        INamedTypeSymbol testMethodAttributeSymbol,
+        INamedTypeSymbol testInitializeAttributeSymbol,
+        INamedTypeSymbol testCleanupAttributeSymbol,
+        INamedTypeSymbol classInitializeAttributeSymbol,
+        INamedTypeSymbol classCleanupAttributeSymbol,
+        INamedTypeSymbol assemblyInitializeAttributeSymbol,
+        INamedTypeSymbol assemblyCleanupAttributeSymbol)
+    {
+        var invocationOperation = (IInvocationOperation)context.Operation;
+        IMethodSymbol method = invocationOperation.TargetMethod;
+
+        // Check if we're inside a test-related method
+        if (context.ContainingSymbol is not IMethodSymbol containingMethod)
+        {
+            return;
+        }
+
+        // Check if the containing method is a test method or test fixture method
+        if (!IsTestRelatedMethod(containingMethod, testMethodAttributeSymbol, testInitializeAttributeSymbol, testCleanupAttributeSymbol, classInitializeAttributeSymbol, classCleanupAttributeSymbol, assemblyInitializeAttributeSymbol, assemblyCleanupAttributeSymbol))
+        {
+            return;
+        }
+
+        // Check if the invocation is Thread.Sleep
+        if (SymbolEqualityComparer.Default.Equals(method.ContainingType, threadSymbol) && method.Name == "Sleep")
+        {
+            context.ReportDiagnostic(invocationOperation.Syntax.CreateDiagnostic(AvoidBlockingCallsInTestsRule, "Thread.Sleep"));
+            return;
+        }
+
+        // Check if the invocation is Task.Wait
+        if (SymbolEqualityComparer.Default.Equals(method.ContainingType, taskSymbol) && method.Name == "Wait")
+        {
+            context.ReportDiagnostic(invocationOperation.Syntax.CreateDiagnostic(AvoidBlockingCallsInTestsRule, "Task.Wait"));
+            return;
+        }
+    }
+
+    private static bool IsTestRelatedMethod(
+        IMethodSymbol method,
+        INamedTypeSymbol testMethodAttributeSymbol,
+        INamedTypeSymbol testInitializeAttributeSymbol,
+        INamedTypeSymbol testCleanupAttributeSymbol,
+        INamedTypeSymbol classInitializeAttributeSymbol,
+        INamedTypeSymbol classCleanupAttributeSymbol,
+        INamedTypeSymbol assemblyInitializeAttributeSymbol,
+        INamedTypeSymbol assemblyCleanupAttributeSymbol)
+    {
+        ImmutableArray<AttributeData> attributes = method.GetAttributes();
+        foreach (AttributeData attribute in attributes)
+        {
+            if (attribute.AttributeClass is null)
+            {
+                continue;
+            }
+
+            // Check if the method has any test-related attribute
+            if (attribute.AttributeClass.Inherits(testMethodAttributeSymbol) ||
+                SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, testInitializeAttributeSymbol) ||
+                SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, testCleanupAttributeSymbol) ||
+                SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, classInitializeAttributeSymbol) ||
+                SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, classCleanupAttributeSymbol) ||
+                SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, assemblyInitializeAttributeSymbol) ||
+                SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, assemblyCleanupAttributeSymbol))
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs

@@ -63,4 +63,5 @@ internal static class DiagnosticIds
     public const string TestMethodAttributeShouldSetDisplayNameCorrectlyRuleId = "MSTEST0056";
     public const string TestMethodAttributeShouldPropagateSourceInformationRuleId = "MSTEST0057";
     public const string AvoidAssertsInCatchBlocksRuleId = "MSTEST0058";
+    public const string AvoidBlockingCallsInTestsRuleId = "MSTEST0059";
 }

src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs

@@ -56,4 +56,5 @@ internal static class WellKnownTypeNames
     public const string SystemThreadingTasksTask1 = "System.Threading.Tasks.Task`1";
     public const string SystemThreadingTasksValueTask = "System.Threading.Tasks.ValueTask";
     public const string SystemThreadingTasksValueTask1 = "System.Threading.Tasks.ValueTask`1";
+    public const string SystemThreadingThread = "System.Threading.Thread";
 }

src/Analyzers/MSTest.Analyzers/Resources.resx

@@ -693,4 +693,13 @@ The type declaring these methods should also respect the following rules:
   <data name="AvoidAssertsInCatchBlocksDescription" xml:space="preserve">
     <value>Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block.</value>
   </data>
+  <data name="AvoidBlockingCallsInTestsTitle" xml:space="preserve">
+    <value>Avoid using Thread.Sleep or Task.Wait in test methods</value>
+  </data>
+  <data name="AvoidBlockingCallsInTestsMessageFormat" xml:space="preserve">
+    <value>Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</value>
+  </data>
+  <data name="AvoidBlockingCallsInTestsDescription" xml:space="preserve">
+    <value>Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</value>
+  </data>
 </root>

src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf

@@ -1004,6 +1004,18 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
         <target state="translated">Používání kontrolních výrazů v blocích catch je problematické, protože test projde, i když se nevyvolá žádná výjimka a blok catch se nikdy nespustí. K ověření, že je vyvolána výjimka, použijte metody Assert.Throws, Assert.ThrowsExactly, Assert.ThrowsAsync nebo Assert.ThrowsExactlyAsync a poté proveďte další kontrolní výrazy nad zachycenou výjimkou bez použití bloku try-catch.</target>
         <note />
       </trans-unit>
+          <trans-unit id="AvoidBlockingCallsInTestsTitle">
+        <source>Avoid using Thread.Sleep or Task.Wait in test methods</source>
+        <target state="new">Avoid using Thread.Sleep or Task.Wait in test methods</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsMessageFormat">
+        <source>Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</source>
+        <target state="new">Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsDescription">
+        <source>Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</source>
+        <target state="new">Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</target>
+        <note /></trans-unit>
     </body>
   </file>
 </xliff>

src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf

@@ -1005,6 +1005,18 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
         <target state="translated">Die Verwendung von Asserts in Catch-Blöcken ist problematisch, da der Test auch dann erfolgreich ist, wenn keine Ausnahme ausgelöst wird und der Catch-Block nie ausgeführt wird. Verwenden Sie „Assert.Throws“, „Assert.ThrowsExactly“, „Assert.ThrowsAsync“ oder „Assert.ThrowsExactlyAsync“, um zu überprüfen, ob eine Ausnahme ausgelöst wird, und erstellen Sie dann zusätzliche Assertionen für die abgefangene Ausnahme, ohne den „try-catch“-Block zu verwenden.</target>
         <note />
       </trans-unit>
+          <trans-unit id="AvoidBlockingCallsInTestsTitle">
+        <source>Avoid using Thread.Sleep or Task.Wait in test methods</source>
+        <target state="new">Avoid using Thread.Sleep or Task.Wait in test methods</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsMessageFormat">
+        <source>Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</source>
+        <target state="new">Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsDescription">
+        <source>Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</source>
+        <target state="new">Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</target>
+        <note /></trans-unit>
     </body>
   </file>
 </xliff>

src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf

@@ -1004,6 +1004,18 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
         <target state="translated">El uso de aserciones en bloques catch es problemático porque la prueba se superará incluso aunque no se produzca ninguna excepción y el bloque catch nunca se ejecuta. Use "Assert.Throws", "Assert.ThrowsExactly", "Assert.ThrowsAsync" o "Assert.ThrowsExactlyAsync" para comprobar que se produzca una excepción y, a continuación, realice aserciones adicionales en la excepción detectada sin usar el bloque try-catch.</target>
         <note />
       </trans-unit>
+          <trans-unit id="AvoidBlockingCallsInTestsTitle">
+        <source>Avoid using Thread.Sleep or Task.Wait in test methods</source>
+        <target state="new">Avoid using Thread.Sleep or Task.Wait in test methods</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsMessageFormat">
+        <source>Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</source>
+        <target state="new">Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsDescription">
+        <source>Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</source>
+        <target state="new">Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</target>
+        <note /></trans-unit>
     </body>
   </file>
 </xliff>

src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf

@@ -1004,6 +1004,18 @@ Le type doit être une classe
         <target state="translated">L’utilisation d’assertions dans les blocs catch pose problème, car le test réussit même si aucune exception n’est levée et que le bloc catch n’est jamais exécuté. Utilisez « Assert.Throws », « Assert.ThrowsExactly », « Assert.ThrowsAsync » ou « Assert.ThrowsExactlyAsync » pour vérifier qu’une exception est levée, puis effectuez des assertions supplémentaires sur l’exception capturée sans utiliser le bloc try-catch.</target>
         <note />
       </trans-unit>
+          <trans-unit id="AvoidBlockingCallsInTestsTitle">
+        <source>Avoid using Thread.Sleep or Task.Wait in test methods</source>
+        <target state="new">Avoid using Thread.Sleep or Task.Wait in test methods</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsMessageFormat">
+        <source>Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</source>
+        <target state="new">Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsDescription">
+        <source>Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</source>
+        <target state="new">Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</target>
+        <note /></trans-unit>
     </body>
   </file>
 </xliff>

src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf

@@ -1004,6 +1004,18 @@ Anche il tipo che dichiara questi metodi deve rispettare le regole seguenti:
         <target state="translated">L'uso di asserzioni nei blocchi catch è problematico perché il test risulta superato anche se non viene generata alcuna eccezione e il blocco catch non viene mai eseguito. Utilizzare 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' o 'Assert.ThrowsExactlyAsync' per verificare che venga generata un'eccezione, quindi effettuare ulteriori asserzioni sull'eccezione rilevata senza usare il blocco try-catch.</target>
         <note />
       </trans-unit>
+          <trans-unit id="AvoidBlockingCallsInTestsTitle">
+        <source>Avoid using Thread.Sleep or Task.Wait in test methods</source>
+        <target state="new">Avoid using Thread.Sleep or Task.Wait in test methods</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsMessageFormat">
+        <source>Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</source>
+        <target state="new">Avoid using '{0}' in test methods as it can cause flakiness. Consider using asynchronous alternatives.</target>
+        <note /></trans-unit>
+      <trans-unit id="AvoidBlockingCallsInTestsDescription">
+        <source>Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</source>
+        <target state="new">Using 'Thread.Sleep' or 'Task.Wait' in test methods can lead to flaky tests when operations don't complete within the specified time frame. Consider using asynchronous alternatives like 'await Task.Delay' or 'await task' instead.</target>
+        <note /></trans-unit>
     </body>
   </file>
 </xliff>