fix: initialize IAsyncInitializer on data sources before constructor injection

[thomhurst]

Summary

• Fixes data sources implementing IAsyncInitializer not being initialized before being passed to the test class constructor
• Adds InitializeClassDataForExecutionAsync() to call execution-time initialization on class data before instance creation
• Adds regression test to verify the fix

Fixes #4432

Test plan

• New regression test ConstructorInjectionAsyncInitializerTests verifies fixture is initialized before constructor
• All [EngineTest(ExpectedResult.Pass)] tests pass

🤖 Generated with Claude Code

[thomhurst]

Summary

Fixes initialization of IAsyncInitializer data sources before constructor injection by properly calling InitializeAsync during execution phase.

Critical Issues

None found ✅

Analysis

This PR correctly addresses issue #4432 by fixing a phase mismatch bug:

The Bug: In TestBuilder.CreateInstance() (line 70), the code was calling InitializeClassDataAsync(), which internally calls ObjectInitializer.InitializeForDiscoveryAsync(). However, InitializeForDiscoveryAsync() only initializes IAsyncDiscoveryInitializer objects, NOT regular IAsyncInitializer objects. This meant that data sources implementing IAsyncInitializer were never initialized before being passed to the test constructor.

The Fix:

• Introduces InitializeClassDataForExecutionAsync() that calls ObjectInitializer.InitializeAsync() to initialize ALL IAsyncInitializer objects
• Updates CreateInstance() to use the new execution-phase method
• Keeps InitializeClassDataAsync() unchanged for its legitimate discovery-phase uses (lines 212, 332, 1480, 1633)

TUnit Rules Compliance:
✅ Dual-Mode: Both source-gen and reflection modes use the same TestBuilder.CreateInstance() method, so the fix applies to both equally (TestBuilder.cs:425)
✅ No Snapshot Changes: Source generator output unchanged, no .verified.txt updates needed
✅ No Public API Changes: Internal method addition only
✅ AOT Compatible: No reflection changes
✅ Performance: Minimal impact - same initialization pattern, just correct phase

Test Coverage:
The regression test ConstructorInjectionAsyncInitializerTests properly verifies:

• Fixture implementing IAsyncInitializer is initialized before constructor
• Constructor receives the initialized fixture with Value = 123
• Test marked with [EngineTest(ExpectedResult.Pass)] for validation

Verdict

✅ APPROVE - No critical issues. This is a clean, focused bug fix that correctly addresses the initialization phase mismatch.

[thomhurst]

Summary

Fixes data sources implementing IAsyncInitializer not being initialized before being passed to test class constructors.

Critical Issues

None found ✅

Suggestions

1. Missing null check in new method (minor)

In ObjectLifecycleService.cs:158, the new InitializeObjectForExecutionAsync method has a null check:

if (obj is null)
{
    return;
}

However, in TestBuilder.cs:69-72, this method is called within a loop over classData:

foreach (var data in classData)
{
    await _objectLifecycleService.InitializeObjectForExecutionAsync(data);
}

This is correct, but you might consider whether the loop should skip null entries to avoid unnecessary async method calls, similar to how it's done elsewhere in the codebase (e.g., RegisterArgumentsAsync in ObjectLifecycleService.cs:106-110). However, this is a micro-optimization and the current approach is fine.

2. Documentation clarity

The new method InitializeObjectForExecutionAsync has good documentation, but it could be clearer about when this should be called vs. the existing InitializeTestObjectsAsync. Specifically:

• InitializeObjectForExecutionAsync: Called for data source objects before test instance creation
• InitializeTestObjectsAsync: Called for test objects after BeforeClass hooks

This distinction is important for understanding the fix.

3. Consistency check

The fix correctly initializes class data before constructor injection at TestBuilder.cs:69-72. This matches the execution-time initialization pattern used elsewhere. The key insight is that class data objects need execution-time initialization (IAsyncInitializer) not just discovery-time initialization (IAsyncDiscoveryInitializer), because they're being used during test execution (passed to constructor).

The old code at line 70 called InitializeClassDataAsync which only called ObjectInitializer.InitializeForDiscoveryAsync, meaning regular IAsyncInitializer objects were never initialized before being passed to constructors. The new approach correctly calls ObjectInitializer.InitializeAsync to initialize all IAsyncInitializer objects.

Verdict

✅ APPROVE - No critical issues

The fix is sound and follows TUnit's dual-mode principle correctly. The new InitializeObjectForExecutionAsync method properly initializes both nested objects and the object itself using execution-time initialization, which is exactly what's needed for objects being passed to test constructors.

[thomhurst]

Summary

Fixes data sources implementing IAsyncInitializer not being initialized before being passed to the test class constructor.

Critical Issues

None found ✅

Analysis

Changes Review

1. TestBuilder.cs (TUnit.Engine/Building/TestBuilder.cs:69-70)

• Before: Called static InitializeClassDataAsync which only initialized IAsyncDiscoveryInitializer objects
• After: Replaced with loop calling _objectLifecycleService.InitializeObjectForExecutionAsync(data) for each classData item
• ✅ Correct: Now properly initializes all nested IAsyncInitializer objects before constructor injection

2. ObjectLifecycleService.cs (TUnit.Engine/Services/ObjectLifecycleService.cs:154-167)

• New method: InitializeObjectForExecutionAsync
• Initializes nested objects first via InitializeNestedObjectsForExecutionAsync
• Then calls ObjectInitializer.InitializeAsync on the object itself
• ✅ Correct: Follows depth-first initialization pattern consistent with the rest of the codebase

3. Test Coverage (TUnit.TestProject/Bugs/4432/ConstructorInjectionAsyncInitializerTests.cs)

• Comprehensive regression tests covering:
  • Basic constructor injection
  • Nested IAsyncInitializer properties (2 and 3 levels deep)
  • Multiple constructor parameters
  • Combined constructor + property injection
  • Shared fixtures (PerClass and None)
• ✅ All tests marked with [EngineTest(ExpectedResult.Pass)]

TUnit Rules Compliance

✅ Dual-Mode: Not applicable - changes are in reflection-only Engine code, not metadata collection
✅ Snapshot Testing: Not applicable - no source generator or public API changes
✅ No VSTest: No violations detected
✅ Performance: Minimal impact - only affects constructor-injected data sources with IAsyncInitializer
✅ AOT Compatible: No reflection added; uses existing ObjectInitializer infrastructure

Design Quality

The fix correctly addresses the issue by:

1. Proper initialization timing: Class data is now initialized at execution time (not discovery time) before being passed to constructors
2. Reuses existing infrastructure: Leverages InitializeNestedObjectsForExecutionAsync and ObjectInitializer.InitializeAsync
3. Maintains phase separation: Discovery still only initializes IAsyncDiscoveryInitializer; execution initializes all IAsyncInitializer
4. Consistent with architecture: Follows the same depth-first pattern used elsewhere in ObjectLifecycleService

The comprehensive test coverage ensures this specific bug (fixtures not initialized before constructor) won't regress.

Verdict

✅ APPROVE - No critical issues

Well-structured fix that properly handles the execution-time initialization of constructor-injected data sources implementing IAsyncInitializer. The change is minimal, follows existing patterns, and includes excellent test coverage.

[thomhurst]

Summary

Fixes IAsyncInitializer data sources not being initialized before constructor injection by replacing discovery-phase initialization with execution-phase initialization.

Critical Issues

None found ✅

Analysis

This PR correctly fixes issue #4432 where ClassDataSource objects implementing IAsyncInitializer were not initialized before being passed to test class constructors.

Root Cause:

• The old code at TestBuilder.cs:70 called InitializeClassDataAsync() which internally uses ObjectInitializer.InitializeForDiscoveryAsync()
• InitializeForDiscoveryAsync() only initializes IAsyncDiscoveryInitializer objects, NOT regular IAsyncInitializer objects
• Constructor injection happens during execution phase, so it needs execution-time initialization

The Fix:

1. TestBuilder.cs:69-72: Replaces static discovery-phase initialization with execution-phase initialization via new InitializeObjectForExecutionAsync() method
2. ObjectLifecycleService.cs:154-167: Adds InitializeObjectForExecutionAsync() that properly initializes nested objects and the object itself using ObjectInitializer.InitializeAsync()
3. PropertyInjectionSourceGenerator.cs:362-377: Adds array handling for constructor arguments containing generic types (fixes source-gen mode support)

TUnit Rules Compliance:
✅ Dual-Mode: Changes apply to both source-gen (PropertyInjectionSourceGenerator.cs) and reflection (TestBuilder.cs) modes
✅ Snapshot Testing: No source generator output changes, no .verified.txt updates needed
✅ No VSTest: No violations
✅ Performance: Minimal impact - only affects constructor-injected data sources with IAsyncInitializer
✅ AOT Compatible: No new reflection, uses existing infrastructure

Test Coverage:
Comprehensive regression tests in ConstructorInjectionAsyncInitializerTests.cs covering:

• Basic constructor injection
• Nested IAsyncInitializer (2-3 levels deep)
• Multiple constructor parameters
• Combined constructor + property injection
• Shared vs non-shared fixtures

All tests marked with [EngineTest(ExpectedResult.Pass)] for validation.

Note on Existing Reviews

I see you've already posted three thorough self-reviews on this PR. While self-reviews can be helpful for documentation, typically PR reviews come from other team members. Your analysis is solid and comprehensive.

Verdict

✅ APPROVE - Clean, focused bug fix with excellent test coverage that properly addresses the initialization phase mismatch.

[thomhurst]

Summary

Fixes data sources implementing IAsyncInitializer not being initialized before constructor injection.

Critical Issues

None found

Suggestions

1. Source Generator: Array handling

The new array handling code looks good and addresses the bug for non-generic ClassDataSource syntax. However, consider adding a clarifying comment that both array and single-value paths are needed for different attribute syntaxes.

2. Method naming consistency

The new InitializeObjectForExecutionAsync method is well-documented and follows the execution-phase pattern. Nice work maintaining the discovery/execution phase separation.

3. Dual-mode verification

Changes properly handle both source-gen and reflection modes:

• Source generator: Added array handling for constructor arguments
• Reflection mode: InitializeObjectForExecutionAsync works on runtime objects regardless of discovery mode
• Both paths correctly initialize nested objects depth-first before the parent

4. Test coverage

The regression test suite is exceptionally thorough:

• 15 test classes covering: basic injection, nested initializers (3 levels deep), multiple parameters, property+constructor injection, shared fixtures, non-initializer fixtures, and method-level injection
• Tests both generic and non-generic ClassDataSource syntax
• 445 lines of comprehensive test coverage

5. Performance consideration (minor)

In TestBuilder.cs:69-72, consider using a ValueListBuilder pattern to avoid Task allocations in hot paths. Not critical for this PR.

Verdict

APPROVE - No critical issues

Excellent fix with thorough testing. The dual-mode implementation is correct, the phase separation is maintained, and the regression tests are comprehensive.