fix: run event receivers on injected properties

[thomhurst]

Summary

Fixes #6554 and #6557 — two related gaps in event receiver registration:

1. [Bug]: Injected properties don't get their event receivers executed #6554: receivers implemented by property-injected objects (e.g. [ClassDataSource<T>] properties) were never invoked.
2. First-in-session/assembly event receivers registered mid-run can miss the one-shot event #6557 (found in review): one-shot first-in-session/assembly events could miss receivers registered mid-run.

Root causes

#6554 — stale per-context caches. TestFilterService.RegisterTest calls GetTestRegisteredReceivers(), building all per-context receiver caches (EventReceiversBuilt = true) — deliberately before property resolution so SkipAttribute can short-circuit expensive data sources. ObjectLifecycleService.RegisterTestAsync then resolves injected property values, but the caches were never invalidated, so the eligible-event-object set consumed later was stale and missing the injected values. The registry-level fast-path gates (HasTestStartReceivers() etc.) then reported no receivers and the events were skipped entirely.

#6557 — one-shot events vs lazy registration. First-in-session/assembly/class invocations memoize per scope key and enumerate the EventReceiverRegistry when the first gated test reaches them. Receivers were registered lazily when their own test entered TestCoordinator, so an object-scoped receiver on a later-scheduled test missed the one-shot whenever an earlier same-interface receiver had already triggered the memoized invocation. Pre-existing and source-agnostic (ctor/method args had the same exposure); surfaced by Codex review on this PR.

Fixes

• ObjectLifecycleService.RegisterTestAsync: invalidate the per-context receiver caches after property resolution (only when injected properties exist) — cache coherence at the single mutation point. O(1), off the hot path, both source-gen and reflection modes.
• EventReceiverOrchestrator.InitializeTestCounts → renamed PrepareForExecution: registers every test's eligible receivers in its existing pre-execution loop over all executable tests. Runs after argument/property resolution and deferred placeholder expansion (TestSessionCoordinator ordering), so eligible-object sets are complete and the registry is fully populated before any one-shot event can fire.
• TestCoordinator's per-test RegisterReceivers stays as a fallback for dynamically queued tests that bypass the eager pass; for scheduled tests it's a cheap dedup no-op.
• Class instances still don't exist at the eager pass; they keep registering per test via RegisterClassInstanceReceiver (unchanged).

Note: ITestRegisteredEventReceiver on injected properties remains unsupported by design — OnTestRegistered intentionally fires before data sources are materialized so skip attributes can prevent their creation.

Testing

• Bugs/_6554: injected PerTestSession property implementing start/end receivers. Fails without the invalidation fix, passes with it — both source-gen and reflection modes.
• Bugs/_6557: injected property implementing first-in-session/assembly receivers, deterministic under the eager pass (every test awaits the memoized one-shot invocations before its body; the pre-fix behavior was scheduling-dependent, which is exactly the bug).
• *EventReceiver* and *PropertyInjection* suites: failure sets identical with and without these changes in both parallel and sequential (--maximum-parallel-tests 1) runs — the handful of filtered-run failures pre-exist on main.

[greptile-apps]

Greptile Summary

The PR makes property-injected event receivers visible after property resolution and eagerly registers scheduled receivers before one-shot lifecycle events can run.

• Invalidates per-test receiver caches after injected properties are resolved.
• Registers eligible receiver objects during pre-execution preparation, retaining per-test registration as a dynamic-test fallback.
• Adds regression coverage for injected start/end receivers, partial injection failures, and first-in-session/assembly receivers.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/TUnit.Core/TestContext.cs	Adds a per-context marker allowing scheduled tests to avoid redundant receiver registration while preserving separate class-instance handling.
src/TUnit.Engine/Services/EventReceiverOrchestrator.cs	Extends pre-execution preparation to populate the receiver registry before one-shot events begin.
src/TUnit.Engine/Services/ObjectLifecycleService.cs	Invalidates receiver caches after injected-property resolution, including exceptional partial-resolution paths.
src/TUnit.Engine/Services/TestExecution/TestCoordinator.cs	Retains receiver registration as a fallback for tests that bypass the eager preparation pass.
src/TUnit.Engine/TestSessionCoordinator.cs	Invokes receiver preparation after deferred test expansion and before scheduling.
tests/TUnit.Engine.Tests/PartialInjectionFailure6554Tests.cs	Adds integration coverage ensuring a resolved sibling receiver observes test end after another injected property fails.
tests/TUnit.TestProject/Bugs/_6554/PartialInjectionFailureTests.cs	Defines the partial property-injection failure regression scenario.
tests/TUnit.TestProject/Bugs/_6554/Tests.cs	Covers start and end callbacks on a session-shared injected receiver.
tests/TUnit.TestProject/Bugs/_6557/Tests.cs	Covers eagerly registered first-in-session and first-in-assembly receivers.

Sequence Diagram

sequenceDiagram
  participant Registration as Test registration
  participant Injection as Property injection
  participant Context as TestContext caches
  participant Orchestrator as EventReceiverOrchestrator
  participant Execution as Test execution
  Registration->>Context: Build registration receiver caches
  Registration->>Injection: Resolve injected properties
  Injection->>Context: Cache injected values
  Injection->>Context: Invalidate receiver caches
  Orchestrator->>Context: Rebuild eligible receiver objects
  Orchestrator->>Orchestrator: Register all scheduled receivers
  Execution->>Orchestrator: Invoke lifecycle events

Loading

_{Reviews (5): Last reviewed commit: "perf: gate per-test receiver registratio..." | Re-trigger Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cef18832f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bda1ca8da0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Code review

Summary: Well-scoped fix for #6554/#6557. I traced the full call chain (TestFilterService.RegisterTest → TestArgumentRegistrationService → ObjectLifecycleService.RegisterTestAsync → cache invalidation, and TestDiscoveryService/TestRegistry → TestSessionCoordinator.ExecuteTests → PrepareForExecution) to confirm the two fixes are correctly ordered: discovery-time property injection + cache invalidation always completes before the eager registration pass runs, and GetTestRegisteredReceivers() is never re-invoked after invalidation, so OnTestRegistered can't double-fire. The dedup via _initializedObjects/_registeredFirstEventReceiverTypes also correctly prevents the retained TestCoordinator fallback call from double-invoking receivers already registered eagerly. Regression tests for both bugs look solid.

One architectural concern worth addressing:

TestSessionCoordinator.ExecuteTests calls InitializeEventReceivers (→ PrepareForExecution) before the try/finally that wraps execution and does cleanup (disposal sweep, TestDataContainer.Reset(), artifact reporting).

Before this PR that call site was safe because InitializeTestCounts only touched Interlocked/ConcurrentDictionary counters — essentially unthrowable. This PR adds a RegisterReceivers(context) call into that same loop (EventReceiverOrchestrator.cs#L606-L627), which for every scheduled test builds the event-receiver caches and sorts receivers by their (user-overridable) Order property (TestContextExtensions.cs#L151-L163) and filters by IScopedAttribute.ScopeType. Both are user-implemented properties that can throw.

Previously, this same cache-building work only ran per-test inside TestCoordinator.ExecuteTestAsync's own try/catch, so one misbehaving receiver failed only that test. Now it runs synchronously for all tests, single-threaded, ahead of the session's try/finally. If any one test's Order/ScopeType getter throws, the exception now aborts the entire run and skips the finally cleanup (tracked-object disposal, static-tracking reset, artifact reporting) instead of failing just that one test — a real increase in blast radius from a single bad receiver implementation.

Suggested fix: either move the InitializeEventReceivers call inside the try block (so finally still runs on failure), or wrap the per-test iteration inside PrepareForExecution in a try/catch that fails just that test and continues the loop (mirroring how failures are isolated elsewhere in this pipeline, e.g. TestFilterService.RegisterTest's argument-registration failure handling). Given this touches control flow in two files, I haven't proposed a one-line suggestion — happy to help draft the fix if useful.

Secondary, non-blocking note: PrepareForExecution now does this work single-threaded for every scheduled test before any test starts executing, whereas previously it was implicitly parallelized (each test built its own cache lazily on its own execution slot). For very large suites this could add measurable serial startup latency before the first test runs. Not necessarily worth blocking on, but worth knowing if suite-size regressions show up in CI timing.

[claude]

Code review (re-check)

I independently re-traced the ordering guarantees for both fixes and they hold: HandleRunRequestAsync fully awaits DiscoverTests(isForExecution: true) (which runs TestFilterService.RegisterTestsAsync → ObjectLifecycleService.RegisterTestAsync) before TestSessionCoordinator.ExecuteTests is even called, and DeferredTestExpander.ExpandAsync awaits RegisterTestsAsync for its children before ExpandDeferredPlaceholdersAsync returns — so by the time PrepareForExecution's eager loop runs, every test's injected-property caches are already invalidated/rebuilt-on-demand. ParallelTaskHelper.ForEachAsync uses Task.WhenAll (not Parallel.ForEachAsync), so siblings really do finish caching before a faulting property's exception propagates — confirms the finally-based invalidation (06c863e) is deterministic, not racy. _initializedObjects/_registeredFirstEventReceiverTypes dedup correctly prevents the retained TestCoordinator.RegisterReceivers fallback from double-invoking anything the eager pass already registered. Both #6554 and #6557 repros look correct and the codex-flagged partial-injection-failure gap is properly closed.

Previously-raised concern is still unresolved: my earlier review on this PR (2026-08-07T09:35) flagged that TestSessionCoordinator.ExecuteTests calls InitializeEventReceivers → PrepareForExecution before the try/finally that wraps execution and cleanup. That's still true at the current head (06c863e). PrepareForExecution now runs RegisterReceivers synchronously for every scheduled test, which builds caches via List<T>.Sort on the user-overridable Order property and ScopedAttributeFilter.FilterScopedAttributes on IScopedAttribute.ScopeType — both can throw on a bad user implementation. Pre-PR, that same work only ran inside TestCoordinator.ExecuteTestAsync's own try/catch(Exception) { MarkFailed }, so one bad receiver failed only its test. Now it runs ahead of and outside the session try/finally, so one bad receiver aborts the whole run and skips the finally cleanup (tracked-object disposal sweep, TestDataContainer.Reset(), artifact reporting).

Suggested fix (unchanged from before): move the InitializeEventReceivers(testList, cancellationToken); call inside the try block, ahead of PrepareTestOrchestrator, so finally still runs if a receiver's Order/ScopeType getter throws. This is a minimal, low-risk change and doesn't affect the correctness fixes in this PR.

Non-blocking: PrepareForExecution also moved cache-building from implicitly-parallel (each test built its cache lazily on its own execution slot) to single-threaded, up-front, for every scheduled test. Probably fine, but worth watching in CI timing for very large suites.

[claude]

Code review (re-check #3)

Re-verified against current head (db042cbd3). Since my last review (06c863e27), the only changes are test-only additions (PartialInjectionFailure6554Tests.cs and a tweak to PartialInjectionFailureTests.cs to keep the deliberately-failing test out of the EngineTest=Pass bucket) — both look correct and don't affect production code.

Previously-raised concern is still unresolved. TestSessionCoordinator.ExecuteTests calls InitializeEventReceivers → PrepareForExecution on line 64, before the try block starts on line 66:

InitializeEventReceivers(testList, cancellationToken);   // line 64 — outside try

try
{
    await PrepareTestOrchestrator(testList, cancellationToken);
    await ExecuteTestsCore(testList, cancellationToken);
    ...
}
finally
{
    // disposal sweep, TestDataContainer.Reset(), artifact reporting
}

PrepareForExecution now runs RegisterReceivers(context) synchronously for every scheduled test (EventReceiverOrchestrator.cs#L606-L627), which calls GetEligibleEventObjects() → BuildEventReceiverCaches, and that method:

• sorts receivers via receivers.Sort(static (a, b) => a.Order.CompareTo(b.Order)) — Order is a user-overridable property getter (TestContextExtensions.cs#L159)
• filters via ScopedAttributeFilter.FilterScopedAttributes, which reads the user-overridable IScopedAttribute.ScopeType (TestContextExtensions.cs#L162)

Both can throw on a bad user implementation. Pre-PR, this same cache-building work only ran per-test inside TestCoordinator.ExecuteTestAsync's own try/catch, so a single misbehaving receiver failed only that one test. Now it runs single-threaded, ahead of and outside the session-level try/finally, so one bad Order/ScopeType getter aborts the entire run and skips the finally cleanup — tracked-object disposal, static-tracking reset, and artifact reporting all get bypassed. That's a real increase in blast radius from a single bad receiver implementation, and in IDE/server mode (where the process is reused across run requests) skipping TestDataContainer.Reset()/the disposal sweep could poison subsequent runs in the same process.

Suggested fix (either works, first is simplest): move the InitializeEventReceivers(testList, cancellationToken) call inside the try block so finally still executes on failure. Alternatively, wrap the per-test loop inside PrepareForExecution in a try/catch that marks just that test failed and continues (mirroring TestFilterService.RegisterTest's existing per-test failure isolation).

Everything else holds up on this re-check: the ordering guarantees for both #6554 and #6557 are correct, the ConcurrentHashSet-backed dedup (_initializedObjects/_registeredFirstEventReceiverTypes) safely prevents the retained TestCoordinator fallback from double-invoking eagerly-registered receivers, and the new partial-injection-failure regression test is a solid addition. This is the one remaining blocker before merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db042cbd3a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3f7913d9f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Code review (re-check #4)

Re-verified against current head (b3f7913d9). Since the last review (db042cbd3), the only new commit is b3f7913d9 ("perf: gate per-test receiver registration fallback behind eager-pass flag"), touching TestContext.cs, EventReceiverOrchestrator.cs, and TestCoordinator.cs.

This new commit looks correct. RegisterReceivers now stamps context.EventObjectsRegistered = true unconditionally at the end of the eager PrepareForExecution pass (which runs single-threaded, synchronously, and fully completes — via the await PrepareTestOrchestrator boundary — before ExecuteTestsCore starts any parallel test execution). TestCoordinator's fallback call is gated behind !EventObjectsRegistered, so normally-scheduled tests skip the redundant scan while dynamically-queued tests (which bypass the eager pass) still get registered. The flag is deliberately not cleared by InvalidateEventReceiverCaches() on retry — I checked this holds up: retries get a new ClassInstance registered separately via EventReceiverOrchestrator.RegisterClassInstanceReceiver (called from TestInitializer.cs), independent of this flag, and _initializedObjects dedups by reference so a stale-but-still-true flag can't cause a missed registration for any other eligible object (those instances are unchanged across retries). No new correctness issue here.

Previously-raised concern is still unresolved (4th round). TestSessionCoordinator.ExecuteTests still calls InitializeEventReceivers(testList, cancellationToken) → PrepareForExecution on line 64, before the try block that wraps ExecuteTestsCore and the finally cleanup (disposal sweep, TestDataContainer.Reset(), artifact reporting). This commit didn't touch TestSessionCoordinator.cs at all, so the concern raised on 06c863e and re-confirmed on db042cbd3 carries forward unchanged:

PrepareForExecution runs RegisterReceivers(context) synchronously for every scheduled test, which sorts receivers via the user-overridable Order property and filters via the user-overridable IScopedAttribute.ScopeType — both can throw on a bad user implementation. Pre-PR, that same work only ran inside TestCoordinator.ExecuteTestAsync's own try/catch, so a single misbehaving receiver failed only that test. Now it runs single-threaded, ahead of and outside the session-level try/finally, so one bad Order/ScopeType getter aborts the entire run and skips the finally cleanup — in IDE/server mode (process reused across run requests) that could poison subsequent runs in the same process.

Suggested fix (unchanged, still minimal and low-risk): move the InitializeEventReceivers(testList, cancellationToken) call inside the try block, ahead of PrepareTestOrchestrator, so finally still executes if a receiver's Order/ScopeType getter throws. This doesn't affect the correctness fixes in this PR and is a one-line move — happy to draft it if useful.

Everything else continues to hold up: ordering guarantees for both #6554 and #6557 are correct, and the new perf-gating change doesn't introduce a regression. This remains the one outstanding item before merge.