Throw exception if ExpectedExceptionBaseAttribute is applied more tha…

…n once (#4358)
microsoft · Dec 16, 2024 · d6d6bbe · d6d6bbe
1 parent b10bda3
commit d6d6bbe
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 4 deletions.
diff --git a/src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs b/src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs
@@ -104,11 +104,11 @@ public virtual bool IsNonDerivedAttributeDefined<TAttribute>(MemberInfo memberIn
     {
         DebugEx.Assert(methodInfo != null, "MethodInfo should be non-null");
 
-        // Get the expected exception attribute
-        ExpectedExceptionBaseAttribute? expectedException;
+        IEnumerable<ExpectedExceptionBaseAttribute> expectedExceptions;
+
         try
         {
-            expectedException = GetFirstDerivedAttributeOrDefault<ExpectedExceptionBaseAttribute>(methodInfo, inherit: true);
+            expectedExceptions = GetDerivedAttributes<ExpectedExceptionBaseAttribute>(methodInfo, inherit: true);
         }
         catch (Exception ex)
         {
@@ -123,7 +123,21 @@ public virtual bool IsNonDerivedAttributeDefined<TAttribute>(MemberInfo memberIn
             throw new TypeInspectionException(errorMessage);
         }
 
-        return expectedException ?? null;
+        // Verify that there is only one attribute (multiple attributes derived from
+        // ExpectedExceptionBaseAttribute are not allowed on a test method)
+        // This is needed EVEN IF the attribute doesn't allow multiple.
+        // See https://github.com/microsoft/testfx/issues/4331
+        if (expectedExceptions.Count() > 1)
+        {
+            string errorMessage = string.Format(
+                CultureInfo.CurrentCulture,
+                Resource.UTA_MultipleExpectedExceptionsOnTestMethod,
+                testMethod.FullClassName,
+                testMethod.Name);
+            throw new TypeInspectionException(errorMessage);
+        }
+
+        return expectedExceptions.FirstOrDefault();
     }
 
     /// <summary>

diff --git a/test/UnitTests/MSTestAdapter.UnitTests/Helpers/ReflectHelperTests.cs b/test/UnitTests/MSTestAdapter.UnitTests/Helpers/ReflectHelperTests.cs
@@ -5,6 +5,7 @@
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
+using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
 using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.TestableImplementations;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -284,6 +285,30 @@ public void GettingAttributesShouldNotReturnInheritedAttributesWhenAskingForNonI
         Verify(nonInheritedAttributes.Length == 1);
     }
 
+    public void ResolveExpectedExceptionShouldThrowWhenAttributeIsDefinedTwice_DifferentConcreteType()
+    {
+        MethodInfo testMethodInfo = typeof(DummyTestClass).GetMethod(nameof(DummyTestClass.DummyTestMethod1));
+
+        // Don't mock. Use the real ReflectionOperations2.
+        _testablePlatformServiceProvider.MockReflectionOperations = null;
+
+        TypeInspectionException ex = Assert.ThrowsException<TypeInspectionException>(
+            () => ReflectHelper.Instance.ResolveExpectedExceptionHelper(testMethodInfo, new("DummyName", "DummyFullClassName", "DummyAssemblyName", isAsync: false)));
+        Assert.AreEqual("The test method DummyFullClassName.DummyName has multiple attributes derived from ExpectedExceptionBaseAttribute defined on it. Only one such attribute is allowed.", ex.Message);
+    }
+
+    public void ResolveExpectedExceptionShouldThrowWhenAttributeIsDefinedTwice_SameConcreteType()
+    {
+        MethodInfo testMethodInfo = typeof(DummyTestClass).GetMethod(nameof(DummyTestClass.DummyTestMethod2));
+
+        // Don't mock. Use the real ReflectionOperations2.
+        _testablePlatformServiceProvider.MockReflectionOperations = null;
+
+        TypeInspectionException ex = Assert.ThrowsException<TypeInspectionException>(
+            () => ReflectHelper.Instance.ResolveExpectedExceptionHelper(testMethodInfo, new("DummyName", "DummyFullClassName", "DummyAssemblyName", isAsync: false)));
+        Assert.AreEqual("The test method DummyFullClassName.DummyName has multiple attributes derived from ExpectedExceptionBaseAttribute defined on it. Only one such attribute is allowed.", ex.Message);
+    }
+
     internal class AttributeMockingHelper
     {
         public AttributeMockingHelper(Mock<IReflectionOperations2> mockReflectionOperations) => _mockReflectionOperations = mockReflectionOperations;
@@ -338,4 +363,30 @@ public object[] GetCustomAttributesNotCached(ICustomAttributeProvider attributeP
 
 public class TestableExtendedTestMethod : TestMethodAttribute;
 
+public class DummyTestClass
+{
+    private class MyExpectedException1Attribute : ExpectedExceptionBaseAttribute
+    {
+        protected internal override void Verify(Exception exception) => throw new NotImplementedException();
+    }
+
+    [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
+    public class MyExpectedException2Attribute : ExpectedExceptionBaseAttribute
+    {
+        protected internal override void Verify(Exception exception) => throw new NotImplementedException();
+    }
+
+    [ExpectedException(typeof(Exception))]
+    [MyExpectedException1]
+
+    public void DummyTestMethod1()
+    {
+    }
+
+    [MyExpectedException2]
+    [MyExpectedException2]
+    public void DummyTestMethod2()
+    {
+    }
+}
 #endregion