Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
fix(engine): tighten property injection guard
Share the already-populated check between source-generated and reflection paths, and only skip reference-type properties.

Remove the unrelated SDK roll-forward change from this PR.
  • Loading branch information
thomhurst committed Apr 26, 2026
commit 1bf13da7f7d6fdf7a63bc26efe7105b517b2cbd5
14 changes: 14 additions & 0 deletions TUnit.Engine.Tests/Issue5753Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,18 @@ await RunTestsWithFilter(
result => result.ResultSummary.Counters.NotExecuted.ShouldBe(0)
]);
}

[Test]
public async Task ValueTypePropertyInjection_DoesNotTreatDefaultValueAsAlreadyPopulated()
{
await RunTestsWithFilter(
"/*/*/Issue5753ValueTypePropertyInjectionTests/*",
[
result => result.ResultSummary.Outcome.ShouldBe("Completed"),
result => result.ResultSummary.Counters.Total.ShouldBe(1),
result => result.ResultSummary.Counters.Passed.ShouldBe(1),
result => result.ResultSummary.Counters.Failed.ShouldBe(0),
result => result.ResultSummary.Counters.NotExecuted.ShouldBe(0)
]);
}
}
23 changes: 13 additions & 10 deletions TUnit.Engine/Services/PropertyInjector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,17 +252,10 @@ private async Task InjectSourceGeneratedPropertyAsync(
ConcurrentDictionary<object, byte> visitedObjects,
CancellationToken cancellationToken)
{
// First check if the property already has a value - skip if it does
// This handles nested objects that were already constructed with their properties set
var property = GetCachedPropertyInfo(metadata.ContainingType, metadata.PropertyName);
if (property != null && property.CanRead)
if (IsPropertyAlreadyPopulated(property, instance))
{
var existingValue = property.GetValue(instance);
if (existingValue != null)
{
// Property already has a value, don't overwrite it
return;
}
return;
}

var testContext = TestContext.Current;
Expand Down Expand Up @@ -329,7 +322,7 @@ private async Task InjectReflectionPropertyAsync(
ConcurrentDictionary<object, byte> visitedObjects,
CancellationToken cancellationToken)
{
if (property.CanRead && property.GetValue(instance) != null)
if (IsPropertyAlreadyPopulated(property, instance))
{
return;
}
Expand All @@ -355,6 +348,16 @@ private async Task InjectReflectionPropertyAsync(
propertySetter(instance, resolvedValue);
}

private static bool IsPropertyAlreadyPopulated(PropertyInfo? property, object instance)
{
if (property is null || !property.CanRead || property.PropertyType.IsValueType)
{
return false;
}

return property.GetValue(instance) is not null;
}

private Task RecurseIntoNestedPropertiesAsync(
object instance,
PropertyInjectionPlan plan,
Expand Down
40 changes: 31 additions & 9 deletions TUnit.TestProject/Bugs/5753/Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,39 @@ public class Issue5753ReflectionPropertyInjectionTests
public async Task InjectedProperty_IsNotResolvedAgain_WhenAlreadySet(Issue5753Container container)
{
await Assert.That(container.Service).IsNotNull();
await Assert.That(container.Service.InstanceNumber).IsEqualTo(1);
await Assert.That(Issue5753InjectedService.InstancesCreated).IsEqualTo(1);
await Assert.That(container.Service).IsSameReferenceAs(container.ServiceCapturedDuringInitialization);
await Assert.That(container.Service.IsInitialized).IsTrue();
}
}

public class Issue5753Container
public class Issue5753ValueTypePropertyInjectionTests
{
[ClassDataSource<Issue5753InjectedService>]
public Issue5753InjectedService Service { get; set; } = null!;
[Issue5753IntDataSource]
public int Value { get; set; }

[Test]
public async Task ValueTypeProperty_IsInjected_WhenDefaultValueIsBoxed()
{
await Assert.That(Value).IsEqualTo(42);
}
}

public class Issue5753InjectedService : IAsyncInitializer
public class Issue5753Container : IAsyncInitializer
{
private static int _instancesCreated;
[ClassDataSource<Issue5753InjectedService>]
public Issue5753InjectedService Service { get; set; } = null!;

public static int InstancesCreated => Volatile.Read(ref _instancesCreated);
public Issue5753InjectedService? ServiceCapturedDuringInitialization { get; private set; }

public int InstanceNumber { get; } = Interlocked.Increment(ref _instancesCreated);
public Task InitializeAsync()
{
ServiceCapturedDuringInitialization = Service;
return Task.CompletedTask;
}
}

public class Issue5753InjectedService : IAsyncInitializer
{
public bool IsInitialized { get; private set; }

public Task InitializeAsync()
Expand All @@ -39,3 +52,12 @@ public Task InitializeAsync()
return Task.CompletedTask;
}
}

public class Issue5753IntDataSourceAttribute : TypedDataSourceAttribute<int>
{
public override async IAsyncEnumerable<Func<Task<int>>> GetTypedDataRowsAsync(DataGeneratorMetadata dataGeneratorMetadata)
{
yield return () => Task.FromResult(42);
await Task.CompletedTask;
}
}
2 changes: 1 addition & 1 deletion global.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"sdk": {
"version": "10.0.203",
"rollForward": "latestFeature",
"rollForward": "latestMajor",
"allowPrerelease": true
},
"test": {
Expand Down
Loading