Fix #123 #141

pengweiqhca · Feb 9, 2025 · 57a27ed · 57a27ed
1 parent bf31b50
commit 57a27ed
Show file tree

Hide file tree

Showing 23 changed files with 285 additions and 106 deletions.
diff --git a/Xunit.DependencyInjection.sln b/Xunit.DependencyInjection.sln
@@ -58,6 +58,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "MinimalApiSample", "test\Mi
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Xunit.DependencyInjection.Test.Parallelization", "test\Xunit.DependencyInjection.Test.Parallelization\Xunit.DependencyInjection.Test.Parallelization.csproj", "{4C426CCA-AEA3-4AC1-A89B-F0AC9FD5A816}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Xunit.DependencyInjection.Test.Parallelization2", "test\Xunit.DependencyInjection.Test.Parallelization2\Xunit.DependencyInjection.Test.Parallelization2.csproj", "{7CAECA8D-F29A-47C6-9167-074CE49F3C6F}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -128,6 +130,10 @@ Global
 		{4C426CCA-AEA3-4AC1-A89B-F0AC9FD5A816}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{4C426CCA-AEA3-4AC1-A89B-F0AC9FD5A816}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{4C426CCA-AEA3-4AC1-A89B-F0AC9FD5A816}.Release|Any CPU.Build.0 = Release|Any CPU
+		{7CAECA8D-F29A-47C6-9167-074CE49F3C6F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{7CAECA8D-F29A-47C6-9167-074CE49F3C6F}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{7CAECA8D-F29A-47C6-9167-074CE49F3C6F}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{7CAECA8D-F29A-47C6-9167-074CE49F3C6F}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -149,6 +155,7 @@ Global
 		{8E57DE30-9A8B-4501-9EB7-9DAC3A267FF2} = {A77B7EA7-84FB-4D74-A3AB-2DC8F3281CFA}
 		{57B47C72-4F79-4422-BFC7-E37F44899C9F} = {25119119-1FAE-46DF-9722-9E1AA38586D7}
 		{4C426CCA-AEA3-4AC1-A89B-F0AC9FD5A816} = {25119119-1FAE-46DF-9722-9E1AA38586D7}
+		{7CAECA8D-F29A-47C6-9167-074CE49F3C6F} = {25119119-1FAE-46DF-9722-9E1AA38586D7}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {63FC91EA-4C82-472D-AD93-5A073622A8AD}

diff --git a/src/Xunit.DependencyInjection/DependencyInjectionContext.cs b/src/Xunit.DependencyInjection/DependencyInjectionContext.cs
@@ -13,11 +13,14 @@ public class DependencyInjectionTestContext(
     IHost host,
     bool disableParallelization,
     bool force,
+    int maxParallelThreads,
     SemaphoreSlim? parallelSemaphore)
     : DependencyInjectionContext(host, disableParallelization)
 {
     public bool ForcedParallelization { get; } = force;
 
+    public int MaxParallelThreads { get; } = maxParallelThreads;
+
     public SemaphoreSlim? ParallelSemaphore { get; } = parallelSemaphore;
 }
 
@@ -33,6 +36,8 @@ public class DependencyInjectionStartupContext(
     public IReadOnlyDictionary<ITestClass, DependencyInjectionContext?> ContextMap { get; } = contextMap;
 
     public SemaphoreSlim? ParallelSemaphore { get; internal set; }
+
+    public int MaxParallelThreads { get; internal set; }
 }
 
 public enum ParallelizationMode

diff --git a/src/Xunit.DependencyInjection/DependencyInjectionTestAssemblyRunner.cs b/src/Xunit.DependencyInjection/DependencyInjectionTestAssemblyRunner.cs
@@ -48,25 +48,25 @@ protected override async Task<RunSummary> RunTestCollectionsAsync(IMessageBus me
         if (type.GetField("disableParallelization", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(this) is true)
             return await base.RunTestCollectionsAsync(messageBus, cancellationTokenSource);
 
-        var maxParallelThreads =
+        _context.MaxParallelThreads =
             (int)type.GetField("maxParallelThreads", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(this);
 
-        if (maxParallelThreads > 0)
+        if (_context.MaxParallelThreads > 0)
         {
             var parallelAlgorithm = type.GetField("parallelAlgorithm", BindingFlags.Instance | BindingFlags.NonPublic);
             if (parallelAlgorithm != null && (int)parallelAlgorithm.GetValue(this) == 0)
             {
-                _context.ParallelSemaphore = new(maxParallelThreads);
+                _context.ParallelSemaphore = new(_context.MaxParallelThreads);
 
                 type.GetField("parallelSemaphore", BindingFlags.Instance | BindingFlags.NonPublic)?
                     .SetValue(this, _context.ParallelSemaphore);
 
                 ThreadPool.GetMinThreads(out var minThreads, out var minIOPorts);
-                if (minThreads < maxParallelThreads)
-                    ThreadPool.SetMinThreads(maxParallelThreads, minIOPorts);
+                if (minThreads < _context.MaxParallelThreads)
+                    ThreadPool.SetMinThreads(_context.MaxParallelThreads, minIOPorts);
             }
             else
-                SetupSyncContext(maxParallelThreads);
+                SetupSyncContext(_context.MaxParallelThreads);
         }
 
         Func<Func<Task<RunSummary>>, Task<RunSummary>> taskRunner = SynchronizationContext.Current != null
@@ -79,6 +79,8 @@ protected override async Task<RunSummary> RunTestCollectionsAsync(IMessageBus me
         List<Func<Task<RunSummary>>>? nonParallel = null;
         var summaries = new List<RunSummary>();
 
+        var previous = new SemaphoreSlim(1, 1);
+
         foreach (var collection in OrderTestCollections())
         {
             var task = () =>
@@ -92,7 +94,27 @@ protected override async Task<RunSummary> RunTestCollectionsAsync(IMessageBus me
             if (attr?.GetNamedArgument<bool>(nameof(CollectionDefinitionAttribute.DisableParallelization)) == true)
                 (nonParallel ??= []).Add(task);
             else
-                (parallel ??= []).Add(taskRunner(task));
+            {
+                var current = previous;
+                var next = new SemaphoreSlim(0, 1);
+                previous = next;
+
+                (parallel ??= []).Add(taskRunner(async () =>
+                {
+                    // Keep TestCollection order
+                    await current.WaitAsync();
+
+                    try
+                    {
+                        return await task();
+                    }
+                    finally
+                    {
+                        next.Release();
+                        current.Dispose();
+                    }
+                }));
+            }
         }
 
         if (parallel?.Count > 0)

diff --git a/src/Xunit.DependencyInjection/DependencyInjectionTestCollectionRunner.cs b/src/Xunit.DependencyInjection/DependencyInjectionTestCollectionRunner.cs
@@ -40,7 +40,9 @@ protected override async Task BeforeTestCollectionFinishedAsync()
     {
         await base.BeforeTestCollectionFinishedAsync();
 
-        foreach (var fixture in CollectionFixtureMappings.Values.OfType<IAsyncDisposable>())
+        foreach (var fixture in CollectionFixtureMappings.Values
+                     .Where(x => x is not IDisposable and not IAsyncLifetime)
+                     .OfType<IAsyncDisposable>())
             await Aggregator.RunAsync(() => fixture.DisposeAsync().AsTask());
 
         if (_serviceScope is { } disposable) await disposable.DisposeAsync();
@@ -90,16 +92,33 @@ protected override async Task<RunSummary> RunTestClassesAsync()
     }
 
     /// <inheritdoc />
-    protected override Task<RunSummary> RunTestClassAsync(ITestClass testClass,
-        IReflectionTypeInfo @class, IEnumerable<IXunitTestCase> testCases) =>
-        context.ContextMap.TryGetValue(testClass, out var value) && value != null
-            ? new DependencyInjectionTestClassRunner(
+    protected override async Task<RunSummary> RunTestClassAsync(ITestClass testClass,
+        IReflectionTypeInfo @class, IEnumerable<IXunitTestCase> testCases)
+    {
+        if (context.ContextMap.TryGetValue(testClass, out var value) && value != null)
+            return await new DependencyInjectionTestClassRunner(
                 new(value.Host, value.DisableParallelization ||
                     !(context.ParallelizationMode == ParallelizationMode.Force ||
                         context.ParallelizationMode == ParallelizationMode.Enhance &&
-                        (SynchronizationContext.Current is MaxConcurrencySyncContext || context.ParallelSemaphore != null)),
-                    context.ParallelizationMode == ParallelizationMode.Force, context.ParallelSemaphore), testClass, @class, testCases,
+                        (SynchronizationContext.Current is MaxConcurrencySyncContext ||
+                            context.ParallelSemaphore != null)),
+                    context.ParallelizationMode == ParallelizationMode.Force,
+                    context.MaxParallelThreads,
+                    context.ParallelSemaphore), testClass,
+                @class, testCases,
                 DiagnosticMessageSink, MessageBus, TestCaseOrderer, new(Aggregator), CancellationTokenSource,
-                CollectionFixtureMappings).RunAsync()
-            : base.RunTestClassAsync(testClass, @class, testCases);
+                CollectionFixtureMappings).RunAsync();
+
+        if (context.ParallelSemaphore is not null)
+            await context.ParallelSemaphore.WaitAsync(CancellationTokenSource.Token);
+
+        try
+        {
+            return await base.RunTestClassAsync(testClass, @class, testCases);
+        }
+        finally
+        {
+            context.ParallelSemaphore?.Release();
+        }
+    }
 }
diff --git a/src/Xunit.DependencyInjection/DependencyInjectionTestMethodRunner.cs b/src/Xunit.DependencyInjection/DependencyInjectionTestMethodRunner.cs
@@ -20,17 +20,30 @@ protected override async Task<RunSummary> RunTestCasesAsync()
     {
         if (context.DisableParallelization ||
             TestCases.Count() < 2 ||
-            TestMethod.TestClass.Class.GetCustomAttributes(typeof(CollectionDefinitionAttribute)).FirstOrDefault() is { } attr &&
+            TestMethod.TestClass.Class.GetCustomAttributes(typeof(CollectionDefinitionAttribute)).FirstOrDefault() is
+                { } attr &&
             attr.GetNamedArgument<bool>(nameof(CollectionDefinitionAttribute.DisableParallelization)) ||
             TestMethod.TestClass.Class.GetCustomAttributes(typeof(DisableParallelizationAttribute)).Any() ||
-            TestMethod.TestClass.Class.GetCustomAttributes(typeof(CollectionAttribute)).Any() && !context.ForcedParallelization ||
+            TestMethod.TestClass.Class.GetCustomAttributes(typeof(CollectionAttribute)).Any() &&
+            !context.ForcedParallelization ||
             TestMethod.Method.GetCustomAttributes(typeof(DisableParallelizationAttribute)).Any() ||
-            TestMethod.Method.GetCustomAttributes(typeof(MemberDataAttribute)).Any(a =>
-                a.GetNamedArgument<bool>(nameof(MemberDataAttribute.DisableDiscoveryEnumeration))))
-            return await base.RunTestCasesAsync();
+            context is { ParallelSemaphore: not null, MaxParallelThreads: 1 })
+        {
+            if (context.ParallelSemaphore is not null)
+                await context.ParallelSemaphore.WaitAsync(CancellationTokenSource.Token);
+
+            try
+            {
+                return await base.RunTestCasesAsync();
+            }
+            finally
+            {
+                context.ParallelSemaphore?.Release();
+            }
+        }
 
         // Respect MaxParallelThreads by using the MaxConcurrencySyncContext if it exists, mimicking how collections are run
-        // https://github.com/xunit/xunit/blob/2.4.2/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestAssemblyRunner.cs#L169-L176
+        // https://github.com/xunit/xunit/blob/v2-2.4.2/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestAssemblyRunner.cs#L169-L176
         var scheduler = SynchronizationContext.Current == null
             ? TaskScheduler.Default
             : TaskScheduler.FromCurrentSynchronizationContext();
@@ -45,7 +58,7 @@ protected override async Task<RunSummary> RunTestCasesAsync()
 
                 try
                 {
-                    return await RunTestCaseAsync(testCase).ConfigureAwait(false);
+                    return await Task.Run(() => RunTestCaseAsync(testCase)).ConfigureAwait(false);
                 }
                 finally
                 {

diff --git a/src/Xunit.DependencyInjection/Xunit.DependencyInjection.csproj b/src/Xunit.DependencyInjection/Xunit.DependencyInjection.csproj
@@ -6,6 +6,7 @@
 
 Release notes:
 
+9.9: Fix some parallelization problem.
 9.8: Allow the default startup to be missing anywhere.
 9.7: Fix #133 #134, IHost.StopAsync might not have been invoked.
 9.6: Support required property on test class.
@@ -26,7 +27,7 @@ Release notes:
 8.1: Startup allow static method or class (like Asp.Net Core startup).
 8.0: New feature: Support multiple startup.</Description>
     <PackageTags>xunit ioc di DependencyInjection test</PackageTags>
-    <Version>9.8.0</Version>
+    <Version>9.9.0</Version>
     <PackageReleaseNotes>$(Description)</PackageReleaseNotes>
     <PolySharpExcludeGeneratedTypes>System.Runtime.CompilerServices.RequiresLocationAttribute;System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute</PolySharpExcludeGeneratedTypes>
   </PropertyGroup>

diff --git a/test/Xunit.DependencyInjection.Test.Parallelization/BlockingCollectionTests.cs b/test/Xunit.DependencyInjection.Test.Parallelization/BlockingCollectionTests.cs
diff --git a/test/Xunit.DependencyInjection.Test.Parallelization/BlockingTheoryTests.cs b/test/Xunit.DependencyInjection.Test.Parallelization/BlockingTheoryTests.cs
diff --git a/...unit.DependencyInjection.Test.Parallelization/CollectionAttributeSequentialTheoryTests.cs b/...unit.DependencyInjection.Test.Parallelization/CollectionAttributeSequentialTheoryTests.cs
@@ -6,5 +6,12 @@ public class CollectionAttributeSequentialTheoryTests(ConcurrencyFixture fixture
     [Theory]
     [InlineData(1)]
     [InlineData(2)]
-    public async Task Theory(int _) => Assert.Equal(1, await fixture.CheckConcurrencyAsync());
+    [InlineData(3)]
+    [InlineData(4)]
+    [InlineData(5)]
+    [InlineData(6)]
+    [InlineData(7)]
+    [InlineData(8)]
+    [InlineData(9)]
+    public Task Theory(int _) => fixture.CheckConcurrencyAsync();
 }
diff --git a/test/Xunit.DependencyInjection.Test.Parallelization/CollectionAttributeTests.cs b/test/Xunit.DependencyInjection.Test.Parallelization/CollectionAttributeTests.cs
@@ -4,8 +4,29 @@ namespace Xunit.DependencyInjection.Test.Parallelization;
 public class CollectionAttributeTests(ConcurrencyFixture fixture) : IClassFixture<ConcurrencyFixture>
 {
     [Fact]
-    public async Task Fact1() => Assert.Equal(1, await fixture.CheckConcurrencyAsync());
+    public Task Fact1() => fixture.CheckConcurrencyAsync();
 
     [Fact]
-    public async Task Fact2() => Assert.Equal(1, await fixture.CheckConcurrencyAsync());
+    public Task Fact2() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact3() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact4() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact5() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact6() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact7() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact8() => fixture.CheckConcurrencyAsync();
+
+    [Fact]
+    public Task Fact9() => fixture.CheckConcurrencyAsync();
 }
diff --git a/test/Xunit.DependencyInjection.Test.Parallelization/ConcurrencyFixture.cs b/test/Xunit.DependencyInjection.Test.Parallelization/ConcurrencyFixture.cs
@@ -1,12 +1,19 @@
-using Xunit.Sdk;
+using System.Collections.Concurrent;
+using Xunit.Sdk;
 
 namespace Xunit.DependencyInjection.Test.Parallelization;
 
 public class ConcurrencyFixture
 {
+    private readonly bool _enableParallelization;
+    private readonly ConcurrentBag<int> _results = new();
     private int _concurrency;
 
-    public async Task<int> CheckConcurrencyAsync()
+    public ConcurrencyFixture() => _enableParallelization = true;
+
+    protected ConcurrencyFixture(bool enableParallelization) => _enableParallelization = enableParallelization;
+
+    public async Task CheckConcurrencyAsync()
     {
         Interlocked.Increment(ref _concurrency);
 
@@ -18,7 +25,12 @@ public async Task<int> CheckConcurrencyAsync()
 
         Interlocked.Decrement(ref _concurrency);
 
-        return overlap;
+        _results.Add(overlap);
+
+        if (_enableParallelization) Assert.InRange(overlap, 1, 2);
+        else Assert.Equal(1, overlap);
+
+        CheckConcurrency(overlap);
 
         static ValueTask Delay(int millisecondsDelay)
         {
@@ -33,4 +45,16 @@ static ValueTask Delay(int millisecondsDelay)
             return default;
         }
     }
+
+    private void CheckConcurrency(int overlap)
+    {
+        var dictionary = _results.GroupBy(x => x).ToDictionary(g => g.Key, g => g.Count());
+
+        if (_enableParallelization) Assert.InRange(dictionary.Count, 1, 2);
+        else Assert.Single(dictionary);
+
+        Assert.Contains(overlap, dictionary);
+    }
 }
+
+public class ConcurrencyDisableFixture() : ConcurrencyFixture(false);