#2719: Class with custom Fact with throwing Skip should fail appropriately

bradwilson · bradwilson · commit e1e4c2e23648 · 2024-05-06T11:36:11.000-07:00
diff --git a/src/xunit.execution/Sdk/Frameworks/XunitTestCase.cs b/src/xunit.execution/Sdk/Frameworks/XunitTestCase.cs
@@ -3,6 +3,7 @@
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Diagnostics;
+using System.Globalization;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -118,25 +119,32 @@ protected override void Initialize()
         {
             base.Initialize();
 
-            var factAttribute = TestMethod.Method.GetCustomAttributes(typeof(FactAttribute)).First();
-            var baseDisplayName = factAttribute.GetNamedArgument<string>("DisplayName") ?? BaseDisplayName;
+            try
+            {
+                var factAttribute = TestMethod.Method.GetCustomAttributes(typeof(FactAttribute)).First();
+                var baseDisplayName = factAttribute.GetNamedArgument<string>("DisplayName") ?? BaseDisplayName;
 
-            DisplayName = GetDisplayName(factAttribute, baseDisplayName);
-            SkipReason = GetSkipReason(factAttribute);
-            Timeout = GetTimeout(factAttribute);
+                DisplayName = GetDisplayName(factAttribute, baseDisplayName);
+                SkipReason = GetSkipReason(factAttribute);
+                Timeout = GetTimeout(factAttribute);
 
-            foreach (var traitAttribute in GetTraitAttributesData(TestMethod))
-            {
-                var discovererAttribute = traitAttribute.GetCustomAttributes(typeof(TraitDiscovererAttribute)).FirstOrDefault();
-                if (discovererAttribute != null)
+                foreach (var traitAttribute in GetTraitAttributesData(TestMethod))
                 {
-                    var discoverer = ExtensibilityPointFactory.GetTraitDiscoverer(DiagnosticMessageSink, discovererAttribute);
-                    if (discoverer != null)
-                        foreach (var keyValuePair in discoverer.GetTraits(traitAttribute))
-                            Traits.Add(keyValuePair.Key, keyValuePair.Value);
+                    var discovererAttribute = traitAttribute.GetCustomAttributes(typeof(TraitDiscovererAttribute)).FirstOrDefault();
+                    if (discovererAttribute != null)
+                    {
+                        var discoverer = ExtensibilityPointFactory.GetTraitDiscoverer(DiagnosticMessageSink, discovererAttribute);
+                        if (discoverer != null)
+                            foreach (var keyValuePair in discoverer.GetTraits(traitAttribute))
+                                Traits.Add(keyValuePair.Key, keyValuePair.Value);
+                    }
+                    else
+                        DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Trait attribute on '{0}' did not have [TraitDiscoverer]", DisplayName));
                 }
-                else
-                    DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Trait attribute on '{0}' did not have [TraitDiscoverer]", DisplayName));
+            }
+            catch (Exception ex)
+            {
+                InitializationException = new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, "Exception during initialization:{0}{1}", Environment.NewLine, ex.Unwrap()));
             }
         }
 
diff --git a/test/test.xunit.execution/Acceptance/Xunit2AcceptanceTests.cs b/test/test.xunit.execution/Acceptance/Xunit2AcceptanceTests.cs
@@ -592,6 +592,33 @@ public class ReadOnlySkipFact : FactAttribute
             // Property setter here is missing, so trying to use it with the overridden skip message will fail at runtime
             public override string Skip => "Skipped";
         }
+
+        // https://github.com/xunit/xunit/issues/2719
+        [Fact]
+        public void ClassWithThrowingSkipGetterShouldReportThatAsFailure()
+        {
+            var msgs = Run<ITestResultMessage>(new[] { typeof(ClassWithThrowingSkip) }).OrderBy(x => x.Test.DisplayName).ToList();
+
+            var msg = Assert.Single(msgs);
+            Assert.Equal("Xunit2AcceptanceTests+CustomFacts+ClassWithThrowingSkip.TestMethod", msg.Test.DisplayName);
+            var failed = Assert.IsAssignableFrom<ITestFailed>(msg);
+            var message = Assert.Single(failed.Messages);
+            Assert.StartsWith("Exception during initialization:" + Environment.NewLine + "System.DivideByZeroException: Attempted to divide by zero.", message);
+        }
+
+        class ClassWithThrowingSkip
+        {
+            [ThrowingSkipFact]
+            public void TestMethod()
+            {
+                Assert.True(false);
+            }
+        }
+
+        public class ThrowingSkipFactAttribute : FactAttribute
+        {
+            public override string Skip { get => throw new DivideByZeroException(); set => base.Skip = value; }
+        }
     }
 
     public class TestOutput : AcceptanceTestV2
