fix: evaluate SkipAttribute before data source initialization (#4737) (#4743)

Derived SkipAttribute subclasses (e.g., skip-in-CI attributes) set
SkipReason during OnTestRegistered, but the execution path called
CreateInstanceAsync() — triggering expensive IAsyncInitializer calls —
before checking SkipReason. If initialization threw (e.g., no database
in CI), the test was marked as failed instead of skipped.

Three layered fixes:

1. TestFilterService.RegisterTest: fire OnTestRegistered event receivers
   BEFORE RegisterTestArgumentsAsync, and skip argument registration
   entirely when SkipReason is set. This prevents ClassDataSource
   initialization for skipped tests during the registration phase.

2. TestCoordinator.ExecuteTestInternalAsync: check SkipReason before
   entering retry/timeout logic and instance creation.

3. TestCoordinator.ExecuteTestLifecycleAsync: check SkipReason before
   CreateInstanceAsync() to prevent data source initialization that
   would fail or waste resources for skipped tests.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: thomhurst

TUnit.Engine.Tests/DerivedSkipWithClassDataSourceTests.cs

@@ -0,0 +1,41 @@
+using Shouldly;
+using TUnit.Engine.Tests.Enums;
+
+namespace TUnit.Engine.Tests;
+
+/// <summary>
+/// Regression tests for https://github.com/thomhurst/TUnit/issues/4737
+/// Verifies that derived SkipAttribute subclasses properly skip tests
+/// when combined with ClassDataSource, even when the data source's
+/// IAsyncInitializer would throw.
+/// </summary>
+public class DerivedSkipWithClassDataSourceTests(TestMode testMode) : InvokableTestBase(testMode)
+{
+    [Test]
+    public async Task DerivedSkipWithFailingDataSource_ShouldBeSkipped()
+    {
+        await RunTestsWithFilter(
+            "/*/*/DerivedSkipWithFailingClassDataSourceTests/*",
+            [
+                result => result.ResultSummary.Outcome.ShouldBe("Failed"),
+                result => result.ResultSummary.Counters.Total.ShouldBe(1),
+                result => result.ResultSummary.Counters.Passed.ShouldBe(0),
+                result => result.ResultSummary.Counters.Failed.ShouldBe(0),
+                result => result.ResultSummary.Counters.NotExecuted.ShouldBe(1)
+            ]);
+    }
+
+    [Test]
+    public async Task DerivedSkipWithSucceedingDataSource_ShouldBeSkipped()
+    {
+        await RunTestsWithFilter(
+            "/*/*/DerivedSkipWithSucceedingClassDataSourceTests/*",
+            [
+                result => result.ResultSummary.Outcome.ShouldBe("Failed"),
+                result => result.ResultSummary.Counters.Total.ShouldBe(1),
+                result => result.ResultSummary.Counters.Passed.ShouldBe(0),
+                result => result.ResultSummary.Counters.Failed.ShouldBe(0),
+                result => result.ResultSummary.Counters.NotExecuted.ShouldBe(1)
+            ]);
+    }
+}

TUnit.Engine/Services/TestExecution/TestCoordinator.cs

@@ -63,6 +63,17 @@ private async ValueTask ExecuteTestInternalAsync(AbstractExecutableTest test, Ca
 
             _contextRestorer.RestoreContext(test);
 
+            // Check if test was already marked as skipped during registration
+            // (e.g., by a derived SkipAttribute evaluated in OnTestRegistered).
+            // This must be checked before any instance creation or retry/timeout logic.
+            if (!string.IsNullOrEmpty(test.Context.SkipReason))
+            {
+                _stateManager.MarkSkipped(test, test.Context.SkipReason!);
+                await _eventReceiverOrchestrator.InvokeTestSkippedEventReceiversAsync(test.Context, cancellationToken).ConfigureAwait(false);
+                await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(test.Context, cancellationToken).ConfigureAwait(false);
+                return;
+            }
+
             // Check if test was already marked as failed during registration (e.g., property injection failure)
             // If so, skip execution and report the failure immediately
             var existingResult = test.Context.Execution.Result;
@@ -249,12 +260,29 @@ await _eventReceiverOrchestrator.InvokeLastTestInSessionEventReceiversAsync(
     /// </summary>
     private async ValueTask ExecuteTestLifecycleAsync(AbstractExecutableTest test, CancellationToken cancellationToken)
     {
+        // Check if this test should be skipped before creating the class instance.
+        // Derived SkipAttribute subclasses set SkipReason during OnTestRegistered (registration phase),
+        // and creating the instance can trigger expensive data source initialization (e.g., starting a
+        // WebApplicationFactory) that would fail or waste resources for tests that should be skipped.
+        if (!string.IsNullOrEmpty(test.Context.SkipReason))
+        {
+            _stateManager.MarkSkipped(test, test.Context.SkipReason!);
+
+            await _eventReceiverOrchestrator.InvokeTestSkippedEventReceiversAsync(test.Context, cancellationToken).ConfigureAwait(false);
+
+            await _eventReceiverOrchestrator.InvokeTestEndEventReceiversAsync(test.Context, cancellationToken).ConfigureAwait(false);
+
+            return;
+        }
+
         test.Context.Metadata.TestDetails.ClassInstance = await test.CreateInstanceAsync().ConfigureAwait(false);
 
         // Invalidate cached eligible event objects since ClassInstance changed
         test.Context.CachedEligibleEventObjects = null;
 
-        // Check if this test should be skipped (after creating instance)
+        // Check if this test should be skipped (after creating instance).
+        // This handles basic [Skip] attributes that use SkippedTestInstance as a sentinel,
+        // and any SkipReason set during instance creation.
         if (test.Context.Metadata.TestDetails.ClassInstance is SkippedTestInstance ||
             !string.IsNullOrEmpty(test.Context.SkipReason))
         {

TUnit.Engine/Services/TestFilterService.cs

@@ -72,18 +72,11 @@ private async Task RegisterTest(AbstractExecutableTest test)
 
         test.Context.InternalDiscoveredTest = discoveredTest;
 
-        try
-        {
-            await testArgumentRegistrationService.RegisterTestArgumentsAsync(test.Context);
-        }
-        catch (Exception ex)
-        {
-            // Mark the test as failed and skip further event receiver processing
-            test.SetResult(TestState.Failed, ex);
-            return;
-        }
-
-        // Use pre-computed receivers (already filtered, sorted, and scoped-attribute filtered)
+        // Invoke event receivers BEFORE argument registration so that SkipAttribute
+        // and other ITestRegisteredEventReceiver implementations can set SkipReason
+        // before any potentially expensive data source initialization occurs.
+        // This is critical for derived SkipAttribute subclasses (e.g., skip-in-CI attributes)
+        // that need to prevent ClassDataSource initialization when the test should be skipped.
         foreach (var receiver in test.Context.GetTestRegisteredReceivers())
         {
             try
@@ -97,6 +90,25 @@ private async Task RegisterTest(AbstractExecutableTest test)
             }
         }
 
+        // If the test was marked as skipped by an event receiver, skip argument registration entirely.
+        // This avoids initializing expensive shared data sources (e.g., WebApplicationFactory)
+        // for tests that will never execute.
+        if (!string.IsNullOrEmpty(test.Context.SkipReason))
+        {
+            test.Context.InvalidateDisplayNameCache();
+            return;
+        }
+
+        try
+        {
+            await testArgumentRegistrationService.RegisterTestArgumentsAsync(test.Context);
+        }
+        catch (Exception ex)
+        {
+            // Mark the test as failed - event receivers have already run above
+            test.SetResult(TestState.Failed, ex);
+        }
+
         // Clear the cached display name after registration events
         // This ensures that ArgumentDisplayFormatterAttribute and similar attributes
         // have a chance to register their formatters before the display name is finalized

TUnit.TestProject/Bugs/4737/DerivedSkipWithClassDataSourceTests.cs

@@ -0,0 +1,73 @@
+using TUnit.Core.Interfaces;
+
+namespace TUnit.TestProject.Bugs._4737;
+
+/// <summary>
+/// Derived SkipAttribute that always skips - simulates a CI-environment skip attribute
+/// like the user's IntegrationTestAttribute in issue #4737.
+/// </summary>
+public class AlwaysSkipAttribute() : SkipAttribute("Skip in CI environment")
+{
+    public override Task<bool> ShouldSkip(TestRegisteredContext context)
+    {
+        return Task.FromResult(true);
+    }
+}
+
+/// <summary>
+/// A data source that throws during InitializeAsync - simulates a WebApplicationFactory
+/// that fails to connect to a database that doesn't exist in CI.
+/// </summary>
+public class FailingDataSource : IAsyncInitializer
+{
+    public Task InitializeAsync()
+    {
+        throw new InvalidOperationException("Simulated infrastructure failure: cannot connect to database");
+    }
+}
+
+/// <summary>
+/// Regression test for https://github.com/thomhurst/TUnit/issues/4737
+/// A class-level derived SkipAttribute combined with a ClassDataSource whose
+/// IAsyncInitializer would throw. The test must be reported as skipped, not failed.
+/// </summary>
+[ClassDataSource<FailingDataSource>(Shared = SharedType.PerTestSession)]
+[AlwaysSkip]
+public class DerivedSkipWithFailingClassDataSourceTests(FailingDataSource dataSource)
+{
+    [Test]
+    public void Test()
+    {
+        throw new Exception("This test should have been skipped, not executed!");
+    }
+}
+
+/// <summary>
+/// A data source that succeeds initialization.
+/// </summary>
+public class SucceedingDataSource : IAsyncInitializer
+{
+    public bool IsInitialized { get; private set; }
+
+    public Task InitializeAsync()
+    {
+        IsInitialized = true;
+        return Task.CompletedTask;
+    }
+}
+
+/// <summary>
+/// Verifies that a derived SkipAttribute at the class level correctly skips tests
+/// even when a ClassDataSource is present and would succeed. The test body would
+/// fail if executed, proving the skip is effective.
+/// </summary>
+[ClassDataSource<SucceedingDataSource>(Shared = SharedType.PerTestSession)]
+[AlwaysSkip]
+public class DerivedSkipWithSucceedingClassDataSourceTests(SucceedingDataSource dataSource)
+{
+    [Test]
+    public void Test()
+    {
+        throw new Exception("This test should have been skipped, not executed!");
+    }
+}