Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions TUnit.Engine.Tests/DerivedSkipWithClassDataSourceTests.cs
Original file line number Diff line number Diff line change
@@ -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)
]);
}
}
30 changes: 29 additions & 1 deletion TUnit.Engine/Services/TestExecution/TestCoordinator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
{
Expand Down
36 changes: 24 additions & 12 deletions TUnit.Engine/Services/TestFilterService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
73 changes: 73 additions & 0 deletions TUnit.TestProject/Bugs/4737/DerivedSkipWithClassDataSourceTests.cs
Original file line number Diff line number Diff line change
@@ -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!");
}
}
Loading