Skip to content

Commit a6ad16c

Browse files
authored
fix: isolate method data source instance (#6379)
1 parent e023677 commit a6ad16c

7 files changed

Lines changed: 418 additions & 305 deletions

File tree

TUnit.Core/TestContext.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,6 @@ internal override void SetAsyncLocalContext()
341341

342342
internal bool RunOnTestDiscovery { get; set; }
343343

344-
/// <summary>
345-
/// Indicates whether this test is reusing the discovery-time instance instead of creating a new instance.
346-
/// When true, property resolution and initialization should be skipped since the instance is already prepared.
347-
/// </summary>
348-
internal bool IsDiscoveryInstanceReused { get; set; }
349-
350344
/// <summary>
351345
/// Gets a synchronization object that can be used for thread-safe operations within this test context.
352346
/// </summary>
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using Shouldly;
2+
using TUnit.Engine.Tests.Enums;
3+
4+
namespace TUnit.Engine.Tests;
5+
6+
public class Issue6361Tests(TestMode testMode) : InvokableTestBase(testMode)
7+
{
8+
[Test]
9+
public async Task Deferred_Instance_Method_Data_Source_Does_Not_Reuse_Enumeration_Instance()
10+
{
11+
await RunTestsWithFilter(
12+
"/*/TUnit.TestProject.Bugs._6361/Issue6361InstanceMethodDataSourceIsolationTests/*",
13+
[
14+
result => result.ResultSummary.Outcome.ShouldBe("Completed"),
15+
result => result.ResultSummary.Counters.Total.ShouldBe(4),
16+
result => result.ResultSummary.Counters.Passed.ShouldBe(4),
17+
result => result.ResultSummary.Counters.Failed.ShouldBe(0)
18+
]);
19+
}
20+
}

TUnit.Engine/Building/Interfaces/ITestBuilder.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ internal interface ITestBuilder
1414
/// <param name="metadata">The test metadata</param>
1515
/// <param name="testData">The test data</param>
1616
/// <param name="testBuilderContext"></param>
17-
/// <param name="isReusingDiscoveryInstance">Whether this test is reusing the discovery instance</param>
1817
/// <returns>An executable test ready for execution</returns>
19-
Task<AbstractExecutableTest> BuildTestAsync(TestMetadata metadata, TestBuilder.TestData testData, TestBuilderContext testBuilderContext, bool isReusingDiscoveryInstance = false, CancellationToken cancellationToken = default);
18+
Task<AbstractExecutableTest> BuildTestAsync(TestMetadata metadata, TestBuilder.TestData testData, TestBuilderContext testBuilderContext, CancellationToken cancellationToken = default);
2019

2120
/// <summary>
2221
/// Builds all executable tests from a single TestMetadata using its DataCombinationGenerator delegate.

TUnit.Engine/Building/TestBuilder.cs

Lines changed: 341 additions & 293 deletions
Large diffs are not rendered by default.

TUnit.Engine/Services/PropertyInjector.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ public Task ResolveAndCachePropertiesAsync(
4848
TestContext testContext,
4949
CancellationToken cancellationToken = default)
5050
{
51-
// Even when the first data row reuses a discovery instance, this test still
52-
// needs its own cached property values so shared fixtures get ref-counted.
51+
// Resolve cached property values here so shared fixtures get ref-counted before execution.
5352
var plan = PropertyInjectionCache.GetOrCreatePlan(testClassType);
5453

5554
if (!plan.HasProperties)
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
using System.Collections.Concurrent;
2+
using TUnit.TestProject.Attributes;
3+
4+
namespace TUnit.TestProject.Bugs._6361;
5+
6+
[EngineTest(ExpectedResult.Pass)]
7+
public sealed class Issue6361InstanceMethodDataSourceIsolationTests
8+
{
9+
private static readonly ConcurrentBag<int> TestInstanceIds = [];
10+
11+
private static int _nextInstanceId;
12+
private static int _dataSourceInstanceId;
13+
14+
private readonly int _instanceId = Interlocked.Increment(ref _nextInstanceId);
15+
16+
[Test]
17+
[MethodDataSource(nameof(GetCases), DeferEnumeration = true)]
18+
public async Task InstanceMethodDataSource_DoesNotReuseEnumerationInstance(string value)
19+
{
20+
TestInstanceIds.Add(_instanceId);
21+
22+
await Assert.That(value).IsNotNullOrEmpty();
23+
}
24+
25+
public IEnumerable<string> GetCases()
26+
{
27+
Interlocked.Exchange(ref _dataSourceInstanceId, _instanceId);
28+
29+
yield return "Case1";
30+
yield return "Case2";
31+
yield return "Case3";
32+
}
33+
34+
[After(Class)]
35+
public static async Task AssertEnumerationInstanceWasIsolated()
36+
{
37+
try
38+
{
39+
var testInstanceIds = TestInstanceIds.ToArray();
40+
var dataSourceInstanceId = Volatile.Read(ref _dataSourceInstanceId);
41+
42+
await Assert.That(dataSourceInstanceId).IsNotEqualTo(0);
43+
await Assert.That(testInstanceIds).Count().IsEqualTo(3);
44+
await Assert.That(testInstanceIds.Distinct()).Count().IsEqualTo(3);
45+
await Assert.That(testInstanceIds).DoesNotContain(dataSourceInstanceId);
46+
}
47+
finally
48+
{
49+
TestInstanceIds.Clear();
50+
_nextInstanceId = 0;
51+
_dataSourceInstanceId = 0;
52+
}
53+
}
54+
}

TUnit.UnitTests/PropertyInjectorTests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@ namespace TUnit.UnitTests;
77
public class PropertyInjectorTests
88
{
99
[Test]
10-
public async Task ReusedDiscoveryInstanceStillCachesInjectedProperties()
10+
public async Task ResolveAndCacheProperties_CachesInjectedProperties()
1111
{
1212
var context = CreateContext<ReusedDiscoveryInstanceTestClass>();
13-
context.IsDiscoveryInstanceReused = true;
1413

1514
var injector = new PropertyInjector(new Lazy<IInitializationCallback>(() => new PassthroughInitializationCallback()), "session");
1615

0 commit comments

Comments
 (0)