Skip to content

fix: prevent After(TestSession) hook from firing prematurely on test timeout#4720

Merged
thomhurst merged 2 commits intomainfrom
fix/after-session-hook-timeout-double-fire
Feb 10, 2026
Merged

fix: prevent After(TestSession) hook from firing prematurely on test timeout#4720
thomhurst merged 2 commits intomainfrom
fix/after-session-hook-timeout-double-fire

Conversation

@thomhurst
Copy link
Owner

Summary

  • Adds a first-call-wins guard to AfterHookPairTracker.RegisterAfterTestSessionHook so only the initial registration (using the session-level cancellation token) takes effect — subsequent calls from per-test timeout tokens are ignored
  • Routes TestScheduler's post-execution session hook call through AfterHookPairTracker.GetOrCreateAfterTestSessionTask instead of calling HookExecutor directly, ensuring hooks execute exactly once even if a cancellation callback already triggered them
  • Wires up the AfterHookPairTracker dependency in TUnitServiceProvider

Test plan

Closes #4712

🤖 Generated with Claude Code

…timeout

When a test timed out, the cancellation callback registered by
AfterHookPairTracker would fire the session-level After hooks
immediately — even while other tests were still running. Additionally,
TestScheduler called HookExecutor directly, bypassing the tracker's
deduplication, causing hooks to run a second time.

Fix by: (1) adding a first-call-wins guard so only the first
RegisterAfterTestSessionHook call registers a cancellation callback
(the one from TestCoordinator with the session-level token, not
per-test timeout tokens), and (2) routing TestScheduler through
AfterHookPairTracker.GetOrCreateAfterTestSessionTask instead of
calling HookExecutor directly.

Closes #4712

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Contributor

claude bot commented Feb 10, 2026

Performance optimization opportunity in TestScheduler.cs line 143: The lambda creates an unnecessary ValueTask to Task to ValueTask conversion. Since GetOrCreateAfterTestSessionTask internally calls .AsTask() on the returned ValueTask, you can simplify the lambda to just: () => _hookExecutor.ExecuteAfterTestSessionHooksAsync(cancellationToken). This eliminates one allocation cycle and aligns with CLAUDE.md Rule 4 about minimizing allocations and using ValueTask appropriately. See https://github.com/thomhurst/TUnit/blob/0075a0134d8c7c5ed58d9d5820df1e20f7441293/CLAUDE.md for the rule and

{
_afterTestSessionTask = taskFactory().AsTask();
}
for the .AsTask() call.

…duler

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thomhurst thomhurst enabled auto-merge (squash) February 10, 2026 16:08
@thomhurst thomhurst disabled auto-merge February 10, 2026 16:17
@thomhurst thomhurst merged commit f7a6217 into main Feb 10, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the fix/after-session-hook-timeout-double-fire branch February 10, 2026 16:17
This was referenced Feb 11, 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]: [After(TestSession)] hook incorrectly triggered on test timeout

1 participant