Simplify HasCorrectTimeout (#4042)

Co-authored-by: Jakub Jareš <[email protected]> Co-authored-by: Youssef Victor <[email protected]> Co-authored-by: Amaury Levé <[email protected]>
microsoft · Nov 20, 2024 · d53e1e3 · d53e1e3
1 parent b91332a
commit d53e1e3
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 33 deletions.
diff --git a/src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs b/src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs
@@ -437,7 +437,7 @@ private TestAssemblyInfo GetAssemblyInfo(Type type)
                     TimeoutAttribute? timeoutAttribute = _reflectionHelper.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(methodInfo, inherit: false);
                     if (timeoutAttribute != null)
                     {
-                        if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+                        if (!timeoutAttribute.HasCorrectTimeout)
                         {
                             string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, methodInfo.DeclaringType!.FullName, methodInfo.Name);
                             throw new TypeInspectionException(message);
@@ -456,7 +456,7 @@ private TestAssemblyInfo GetAssemblyInfo(Type type)
                     TimeoutAttribute? timeoutAttribute = _reflectionHelper.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(methodInfo, inherit: false);
                     if (timeoutAttribute != null)
                     {
-                        if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+                        if (!timeoutAttribute.HasCorrectTimeout)
                         {
                             string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, methodInfo.DeclaringType!.FullName, methodInfo.Name);
                             throw new TypeInspectionException(message);
@@ -597,7 +597,7 @@ private void UpdateInfoIfClassInitializeOrCleanupMethod(
             TimeoutAttribute? timeoutAttribute = _reflectionHelper.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(methodInfo, inherit: false);
             if (timeoutAttribute != null)
             {
-                if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+                if (!timeoutAttribute.HasCorrectTimeout)
                 {
                     string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, methodInfo.DeclaringType!.FullName, methodInfo.Name);
                     throw new TypeInspectionException(message);
@@ -630,7 +630,7 @@ private void UpdateInfoIfClassInitializeOrCleanupMethod(
             TimeoutAttribute? timeoutAttribute = _reflectionHelper.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(methodInfo, inherit: false);
             if (timeoutAttribute != null)
             {
-                if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+                if (!timeoutAttribute.HasCorrectTimeout)
                 {
                     string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, methodInfo.DeclaringType!.FullName, methodInfo.Name);
                     throw new TypeInspectionException(message);
@@ -696,7 +696,7 @@ private void UpdateInfoIfTestInitializeOrCleanupMethod(
             TimeoutAttribute? timeoutAttribute = _reflectionHelper.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(methodInfo, inherit: false);
             if (timeoutAttribute != null)
             {
-                if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+                if (!timeoutAttribute.HasCorrectTimeout)
                 {
                     string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, methodInfo.DeclaringType!.FullName, methodInfo.Name);
                     throw new TypeInspectionException(message);
@@ -727,7 +727,7 @@ private void UpdateInfoIfTestInitializeOrCleanupMethod(
             TimeoutAttribute? timeoutAttribute = _reflectionHelper.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(methodInfo, inherit: false);
             if (timeoutAttribute != null)
             {
-                if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+                if (!timeoutAttribute.HasCorrectTimeout)
                 {
                     string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, methodInfo.DeclaringType!.FullName, methodInfo.Name);
                     throw new TypeInspectionException(message);
@@ -894,7 +894,7 @@ private TimeoutInfo GetTestTimeout(MethodInfo methodInfo, TestMethod testMethod)
 
         if (timeoutAttribute != null)
         {
-            if (!methodInfo.HasCorrectTimeout(timeoutAttribute))
+            if (!timeoutAttribute.HasCorrectTimeout)
             {
                 string message = string.Format(CultureInfo.CurrentCulture, Resource.UTA_ErrorInvalidTimeout, testMethod.FullClassName, testMethod.Name);
                 throw new TypeInspectionException(message);

diff --git a/src/Adapter/MSTest.TestAdapter/Extensions/MethodInfoExtensions.cs b/src/Adapter/MSTest.TestAdapter/Extensions/MethodInfoExtensions.cs
@@ -80,22 +80,6 @@ internal static bool HasCorrectTestMethodSignature(this MethodInfo method, bool
             method.IsValidReturnType(); // Match return type Task for async methods only. Else return type void.
     }
 
-    /// <summary>
-    /// Checks whether test method has correct Timeout attribute.
-    /// </summary>
-    /// <param name="method">The method to verify.</param>
-    /// <param name="timeoutAttribute">The timeout attribute when we already have it.</param>
-    /// <returns>True if the method has the right test timeout signature.</returns>
-    internal static bool HasCorrectTimeout(this MethodInfo method, TimeoutAttribute? timeoutAttribute = null)
-    {
-        DebugEx.Assert(method != null, "method should not be null.");
-
-        // TODO: redesign this, probably change this to GetTimeout? so we don't have to do this weird dance?
-        timeoutAttribute ??= ReflectHelper.Instance.GetFirstNonDerivedAttributeOrDefault<TimeoutAttribute>(method, inherit: false);
-
-        return timeoutAttribute?.Timeout > 0;
-    }
-
     /// <summary>
     /// Check is return type is void for non async and Task for async methods.
     /// </summary>

diff --git a/src/TestFramework/TestFramework/Attributes/TestMethod/TimeoutAttribute.cs b/src/TestFramework/TestFramework/Attributes/TestMethod/TimeoutAttribute.cs
@@ -50,5 +50,10 @@ public bool CooperativeCancellation
         }
     }
 
+    /// <summary>
+    /// Gets a value indicating whether the instance has the correct test timeout signature.
+    /// </summary>
+    internal bool HasCorrectTimeout => Timeout > 0;
+
     internal bool IsCooperativeCancellationSet { get; private set; }
 }
diff --git a/test/UnitTests/MSTestAdapter.UnitTests/Extensions/MethodInfoExtensionsTests.cs b/test/UnitTests/MSTestAdapter.UnitTests/Extensions/MethodInfoExtensionsTests.cs
@@ -246,22 +246,16 @@ public void HasCorrectTestMethodSignatureShouldReturnFalseForAsyncTestMethodsWit
 
     #region HasCorrectTimeout tests
 
-    public void HasCorrectTimeoutShouldReturnFalseForMethodsWithoutTimeoutAttribute()
-    {
-        MethodInfo methodInfo = typeof(DummyTestClass).GetMethod("PublicMethod");
-        Verify(!methodInfo.HasCorrectTimeout());
-    }
-
     public void HasCorrectTimeoutShouldReturnFalseForMethodsWithInvalidTimeoutAttribute()
     {
-        MethodInfo methodInfo = typeof(DummyTestClass).GetMethod("PublicMethodWithInvalidTimeout");
-        Verify(!methodInfo.HasCorrectTimeout());
+        var timeoutAttribute = new UTF.TimeoutAttribute(-11);
+        Verify(!timeoutAttribute.HasCorrectTimeout);
     }
 
     public void HasCorrectTimeoutShouldReturnTrueForMethodsWithTimeoutAttribute()
     {
-        MethodInfo methodInfo = typeof(DummyTestClass).GetMethod("PublicMethodWithTimeout");
-        Verify(methodInfo.HasCorrectTimeout());
+        var timeoutAttribute = new UTF.TimeoutAttribute(11);
+        Verify(timeoutAttribute.HasCorrectTimeout);
     }
 
     #endregion