Skip to content

Commit eb926bc

Browse files
committed
Attempt to use CollectionFixture instead of ClassFixture
I think there may be race conditions if we reuse containers.
1 parent 57dadbd commit eb926bc

File tree

8 files changed

+44
-247
lines changed

8 files changed

+44
-247
lines changed

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AdoNet/DapperTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace Datadog.Trace.ClrProfiler.IntegrationTests.AdoNet
1414
[Trait("RequiresDockerDependency", "true")]
1515
[Trait("DockerGroup", "1")]
1616
[Collection(ContainersCollection.Name)]
17-
public class DapperTests : TracingIntegrationTest, IClassFixture<PostgresFixture>
17+
public class DapperTests : TracingIntegrationTest
1818
{
1919
public DapperTests(ITestOutputHelper output, PostgresFixture postgresFixture)
2020
: base("Dapper", output)

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AdoNet/NpgsqlCommandTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace Datadog.Trace.ClrProfiler.IntegrationTests.AdoNet
2323
[Trait("DockerGroup", "1")]
2424
[Collection(ContainersCollection.Name)]
2525
[UsesVerify]
26-
public class NpgsqlCommandTests : TracingIntegrationTest, IClassFixture<PostgresFixture>
26+
public class NpgsqlCommandTests : TracingIntegrationTest
2727
{
2828
public NpgsqlCommandTests(ITestOutputHelper output, PostgresFixture postgresFixture)
2929
: base("Npgsql", output)

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/AerospikeTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace Datadog.Trace.ClrProfiler.IntegrationTests
2222
[Trait("DockerGroup", "2")]
2323
[Collection(ContainersCollection.Name)]
2424
[UsesVerify]
25-
public class AerospikeTests : TracingIntegrationTest, IClassFixture<AerospikeFixture>
25+
public class AerospikeTests : TracingIntegrationTest
2626
{
2727
public AerospikeTests(ITestOutputHelper output, AerospikeFixture aerospikeFixture)
2828
: base("Aerospike", output)

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/ContainersCollection.cs

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,46 +3,21 @@
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
55

6-
using System;
7-
using System.Threading.Tasks;
86
using Datadog.Trace.TestHelpers.AutoInstrumentation.Containers;
97
using Xunit;
108

119
namespace Datadog.Trace.ClrProfiler.IntegrationTests;
1210

1311
/// <summary>
1412
/// Collection definition for TestContainers.
15-
/// This ensures that all containers are properly disposed when all tests complete.
13+
/// Using ICollectionFixture ensures that ONE instance of each fixture is shared across all test classes
14+
/// in the collection, and disposed when all tests complete. This prevents race conditions and
15+
/// eliminates the need for reference counting.
1616
/// </summary>
1717
[CollectionDefinition(Name)]
18-
public class ContainersCollection : ICollectionFixture<ContainersCleanup>
18+
public class ContainersCollection :
19+
ICollectionFixture<PostgresFixture>,
20+
ICollectionFixture<AerospikeFixture>
1921
{
2022
public const string Name = "TestContainers";
2123
}
22-
23-
/// <summary>
24-
/// Cleanup fixture that disposes all TestContainers when the test collection completes.
25-
/// This implements the optimization mentioned in PR #5031: "stop the docker images as soon as they're not needed anymore"
26-
/// </summary>
27-
#pragma warning disable SA1402 // File may only contain a single type
28-
public class ContainersCleanup : IAsyncLifetime
29-
#pragma warning restore SA1402 // File may only contain a single type
30-
{
31-
public Task InitializeAsync()
32-
{
33-
// No initialization needed
34-
return Task.CompletedTask;
35-
}
36-
37-
public async Task DisposeAsync()
38-
{
39-
try
40-
{
41-
await ContainersRegistry.DisposeAll();
42-
}
43-
catch
44-
{
45-
// Don't throw - we don't want to fail tests due to cleanup errors
46-
}
47-
}
48-
}

tracer/test/Datadog.Trace.Security.IntegrationTests/ContainersCollection.cs

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,37 +12,12 @@ namespace Datadog.Trace.Security.IntegrationTests;
1212

1313
/// <summary>
1414
/// Collection definition for TestContainers.
15-
/// This ensures that all containers are properly disposed when all tests complete.
15+
/// Using ICollectionFixture ensures that ONE instance of each fixture is shared across all test classes
16+
/// in the collection, and disposed when all tests complete. This prevents race conditions and
17+
/// eliminates the need for reference counting.
1618
/// </summary>
1719
[CollectionDefinition(Name)]
18-
public class ContainersCollection : ICollectionFixture<ContainersCleanup>
20+
public class ContainersCollection : ICollectionFixture<PostgresFixture>
1921
{
2022
public const string Name = "TestContainers";
2123
}
22-
23-
/// <summary>
24-
/// Cleanup fixture that disposes all TestContainers when the test collection completes.
25-
/// This implements the optimization mentioned in PR #5031: "stop the docker images as soon as they're not needed anymore"
26-
/// </summary>
27-
#pragma warning disable SA1402 // File may only contain a single type
28-
public class ContainersCleanup : IAsyncLifetime
29-
#pragma warning restore SA1402 // File may only contain a single type
30-
{
31-
public Task InitializeAsync()
32-
{
33-
// No initialization needed
34-
return Task.CompletedTask;
35-
}
36-
37-
public async Task DisposeAsync()
38-
{
39-
try
40-
{
41-
await ContainersRegistry.DisposeAll();
42-
}
43-
catch
44-
{
45-
// Don't throw - we don't want to fail tests due to cleanup errors
46-
}
47-
}
48-
}

tracer/test/Datadog.Trace.Security.IntegrationTests/IAST/AspNetCore5IastDbTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Datadog.Trace.Security.IntegrationTests.IAST;
1717

1818
[Trait("RequiresDockerDependency", "true")]
1919
[Collection(ContainersCollection.Name)]
20-
public class AspNetCore5IastDbTests : AspNetCore5IastTests, IClassFixture<PostgresFixture>
20+
public class AspNetCore5IastDbTests : AspNetCore5IastTests
2121
{
2222
public AspNetCore5IastDbTests(AspNetCoreTestFixture fixture, PostgresFixture postgresFixture, ITestOutputHelper outputHelper)
2323
: base(fixture, outputHelper, enableIast: true, testName: "AspNetCore5IastDbTestsIastEnabled", samplingRate: 100, vulnerabilitiesPerRequest: 200, isIastDeduplicationEnabled: false, sampleName: "AspNetCore5")

tracer/test/Datadog.Trace.TestHelpers.AutoInstrumentation/Containers/ContainerFixture.cs

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,32 +15,48 @@ namespace Datadog.Trace.TestHelpers.AutoInstrumentation.Containers;
1515

1616
public abstract class ContainerFixture : IAsyncLifetime
1717
{
18-
private IReadOnlyDictionary<string, object>? _resources;
18+
private Dictionary<string, object>? _resources;
1919

2020
public async Task InitializeAsync()
2121
{
22-
_resources = await ContainersRegistry.GetOrAdd(GetType(), InitializeResources);
22+
_resources = new Dictionary<string, object>();
23+
await InitializeResources(_resources.Add).ConfigureAwait(false);
2324
}
2425

2526
public async Task DisposeAsync()
2627
{
27-
// Release the reference to this fixture type
28-
// When the reference count reaches zero, the containers will be disposed
29-
await ContainersRegistry.Release(GetType());
28+
if (_resources == null)
29+
{
30+
return;
31+
}
32+
33+
foreach (var resourceKvp in _resources)
34+
{
35+
var resource = resourceKvp.Value;
36+
37+
try
38+
{
39+
if (resource is IAsyncDisposable asyncDisposable)
40+
{
41+
await asyncDisposable.DisposeAsync().ConfigureAwait(false);
42+
}
43+
else if (resource is IDisposable disposable)
44+
{
45+
disposable.Dispose();
46+
}
47+
}
48+
catch
49+
{
50+
// Continue disposing other resources even if one fails
51+
}
52+
}
53+
54+
_resources.Clear();
3055
}
3156

3257
public virtual IEnumerable<KeyValuePair<string, string>> GetEnvironmentVariables() => Enumerable.Empty<KeyValuePair<string, string>>();
3358

3459
protected abstract Task InitializeResources(Action<string, object> registerResource);
3560

3661
protected T GetResource<T>(string key) => (T)_resources![key];
37-
38-
private async Task<IReadOnlyDictionary<string, object>> InitializeResources()
39-
{
40-
var resources = new Dictionary<string, object>();
41-
42-
await InitializeResources(resources.Add).ConfigureAwait(false);
43-
44-
return resources;
45-
}
4662
}

tracer/test/Datadog.Trace.TestHelpers.AutoInstrumentation/Containers/ContainersRegistry.cs

Lines changed: 0 additions & 169 deletions
This file was deleted.

0 commit comments

Comments
 (0)