Skip to content

fix: evaluate SkipAttribute before data source initialization#4743

Merged
thomhurst merged 1 commit intomainfrom
fix/4737-derived-skip-with-class-data-source
Feb 11, 2026
Merged

fix: evaluate SkipAttribute before data source initialization#4743
thomhurst merged 1 commit intomainfrom
fix/4737-derived-skip-with-class-data-source

Conversation

@thomhurst
Copy link
Owner

Summary

Fixes #4737

  • Reorder registration phase: Fire OnTestRegistered event receivers (including SkipAttribute) before RegisterTestArgumentsAsync in TestFilterService.RegisterTest, and skip argument registration entirely when SkipReason is set. This prevents ClassDataSource initialization for tests that should be skipped.
  • Early skip check in execution: Check SkipReason in ExecuteTestInternalAsync before entering retry/timeout logic and instance creation, avoiding unnecessary work for already-skipped tests.
  • Pre-instance skip check: Check SkipReason in ExecuteTestLifecycleAsync before CreateInstanceAsync() to prevent data source IAsyncInitializer calls that would fail or waste resources for skipped tests.

Root cause

Derived SkipAttribute subclasses (e.g., CI-environment skip attributes) evaluate ShouldSkip() at runtime during OnTestRegistered. Previously, RegisterTestArgumentsAsync ran before event receivers, and CreateInstanceAsync() (which triggers IAsyncInitializer.InitializeAsync() on data sources) ran before the SkipReason check. If initialization threw (e.g., database unavailable in CI), the exception escaped before the skip check could run, marking the test as failed instead of skipped.

Test plan

  • New regression test: derived SkipAttribute + ClassDataSource with failing IAsyncInitializer → test is skipped, not failed
  • New regression test: derived SkipAttribute + ClassDataSource with succeeding initializer → test is skipped
  • Both tests run in all 3 modes (SourceGenerated, Reflection, AOT)
  • Existing skip tests pass: SkipTests, DynamicSkipReasonTests, SkipInHooksTests, SkipTestExceptionTests
  • Existing data source tests pass: DataSourceCountTests, DataSourceExceptionPropagationTests, GenericMethodWithDataSourceTests, PropertyInjectionInitFailureTests

🤖 Generated with Claude Code

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>
@thomhurst thomhurst enabled auto-merge (squash) February 11, 2026 23:14
@thomhurst thomhurst merged commit c0000c1 into main Feb 11, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the fix/4737-derived-skip-with-class-data-source branch February 11, 2026 23:54
This was referenced Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Version 1.11.45 breaks integration tests with migrations in Github Workflow

1 participant