Fix for #2557 (#2877)

Co-authored-by: James Terwilliger <terwillj@moodys.com> Co-authored-by: Brad Wilson <dotnetguy@gmail.com>
xunit · Feb 6, 2024 · d3bc65f · d3bc65f
1 parent 2210888
commit d3bc65f
Show file tree

Hide file tree

Showing 15 changed files with 694 additions and 544 deletions.
diff --git a/src/xunit.v3.core.tests/Acceptance/FixtureAcceptanceTests.cs b/src/xunit.v3.core.tests/Acceptance/FixtureAcceptanceTests.cs
@@ -544,6 +544,35 @@ public ClassWithCountedFixture(CountedFixture fixture)
 			[Fact]
 			public void TheTest() { }
 		}
+
+		[Fact]
+		public async ValueTask CollectionFixtureOnGenericTestClassAcceptsArgument()
+		{
+			var messages = await RunAsync<_TestPassed>(typeof(GenericTests));
+
+			Assert.Equal(2, messages.Count);
+		}
+
+		[CollectionDefinition("generic collection")]
+		public class GenericFixtureCollection<T> : ICollectionFixture<GenericFixture<T>> { }
+
+		[Collection("generic collection")]
+		abstract class GenericTestBase<T>
+		{
+			protected GenericTestBase(GenericFixture<T> fixture) => Fixture = fixture;
+			protected readonly GenericFixture<T> Fixture;
+		}
+
+		public class GenericArgument { }
+
+		class GenericTests : GenericTestBase<GenericArgument>
+		{
+#pragma warning disable xUnit1041 // Fixture arguments to test classes must have fixture sources
+			public GenericTests(GenericFixture<GenericArgument> fixture) : base(fixture) { }
+#pragma warning restore xUnit1041 // Fixture arguments to test classes must have fixture sources
+			[Fact] public void Test1() => Assert.NotNull(Fixture);
+			[Fact] public void Test2() { }
+		}
 	}
 
 	public class AsyncCollectionFixture : AcceptanceTestV3
@@ -874,6 +903,7 @@ public async ValueTask AssemblyFixtureAsyncSetupShouldOnlyRunOnce()
 		{
 			var alphaFixture = Mocks.AssemblyFixtureAttribute(typeof(CountedAsyncFixture<Alpha>));
 			var betaFixture = Mocks.AssemblyFixtureAttribute(typeof(CountedAsyncFixture<Beta>));
+
 			var results = await RunAsync<_TestPassed>(new[] { typeof(TestClass1), typeof(TestClass2) }, additionalAssemblyAttributes: new[] { alphaFixture, betaFixture });
 
 			Assert.Equal(2, results.Count);
@@ -1077,4 +1107,12 @@ class CountedFixture
 
 		public readonly int Identity;
 	}
+
+	sealed class GenericFixture<T> : IAsyncLifetime, IDisposable
+	{
+		public GenericFixture() { }
+		public void Dispose() { }
+		public ValueTask InitializeAsync() => default;
+		public ValueTask DisposeAsync() => default;
+	}
 }
diff --git a/src/xunit.v3.core.tests/Sdk/v3/Runners/TestClassRunnerTests.cs b/src/xunit.v3.core.tests/Sdk/v3/Runners/TestClassRunnerTests.cs
@@ -488,6 +488,13 @@ protected override ValueTask BeforeTestClassFinishedAsync(TestClassRunnerContext
 			return default;
 		}
 
+		protected override ValueTask<object?> GetConstructorArgument(
+			TestClassRunnerContext<_ITestCase> ctxt,
+			ConstructorInfo constructor,
+			int index,
+			ParameterInfo parameter) =>
+				new(availableArguments.FirstOrDefault(arg => parameter.ParameterType.IsAssignableFrom(arg.GetType())));
+
 		public async ValueTask<RunSummary> RunAsync()
 		{
 			await using var ctxt = new TestClassRunnerContext<_ITestCase>(TestClass, @class, testCases, ExplicitOption.Off, messageBus, testCaseOrderer, aggregator, TokenSource);
@@ -516,27 +523,5 @@ public async ValueTask<RunSummary> RunAsync()
 		{
 			return constructor ?? base.SelectTestClassConstructor(ctxt);
 		}
-
-		protected override bool TryGetConstructorArgument(
-			TestClassRunnerContext<_ITestCase> ctxt,
-			ConstructorInfo constructor,
-			int index,
-			ParameterInfo parameter,
-			out object argumentValue)
-		{
-			var resultValue = availableArguments.FirstOrDefault(arg => parameter.ParameterType.IsAssignableFrom(arg.GetType()));
-			if (resultValue is null)
-			{
-				var result = base.TryGetConstructorArgument(ctxt, constructor, index, parameter, out resultValue);
-				if (result == false || resultValue is null)
-				{
-					argumentValue = null!;
-					return false;
-				}
-			}
-
-			argumentValue = resultValue;
-			return true;
-		}
 	}
 }
diff --git a/src/xunit.v3.core.tests/Sdk/v3/Runners/XunitTestClassRunnerTests.cs b/src/xunit.v3.core.tests/Sdk/v3/Runners/XunitTestClassRunnerTests.cs
@@ -405,18 +405,17 @@ public IReadOnlyCollection<TTestCase> OrderTestCases<TTestCase>(IReadOnlyCollect
 	class TestableXunitTestClassRunner : XunitTestClassRunner
 	{
 		readonly ExceptionAggregator aggregator;
-		readonly IReadOnlyDictionary<Type, object> assemblyFixtureMappings;
 		readonly CancellationTokenSource cancellationTokenSource;
 		readonly _IReflectionTypeInfo @class;
-		readonly IReadOnlyDictionary<Type, object> collectionFixtureMappings;
+		readonly FixtureMappingManager collectionFixtureMappings;
 		readonly IMessageBus messageBus;
 		readonly ITestCaseOrderer testCaseOrderer;
 		readonly IReadOnlyCollection<IXunitTestCase> testCases;
 		readonly _ITestClass testClass;
 
 		public List<object?[]> ConstructorArguments = new();
 		public Exception? RunTestMethodAsync_AggregatorResult;
-		public Dictionary<Type, object>? RunTestMethodsAsync_ClassFixtureMappings;
+		public IReadOnlyDictionary<Type, object>? RunTestMethodsAsync_ClassFixtureMappings;
 		public ITestCaseOrderer? RunTestMethodsAsync_TestCaseOrderer;
 
 		TestableXunitTestClassRunner(
@@ -427,8 +426,7 @@ class TestableXunitTestClassRunner : XunitTestClassRunner
 			ITestCaseOrderer testCaseOrderer,
 			ExceptionAggregator aggregator,
 			CancellationTokenSource cancellationTokenSource,
-			IReadOnlyDictionary<Type, object> assemblyFixtureMappings,
-			IReadOnlyDictionary<Type, object> collectionFixtureMappings)
+			FixtureMappingManager collectionFixtureMappings)
 		{
 			this.testClass = testClass;
 			this.@class = @class;
@@ -437,7 +435,6 @@ class TestableXunitTestClassRunner : XunitTestClassRunner
 			this.testCaseOrderer = testCaseOrderer;
 			this.aggregator = aggregator;
 			this.cancellationTokenSource = cancellationTokenSource;
-			this.assemblyFixtureMappings = assemblyFixtureMappings;
 			this.collectionFixtureMappings = collectionFixtureMappings;
 		}
 
@@ -452,22 +449,21 @@ class TestableXunitTestClassRunner : XunitTestClassRunner
 					new MockTestCaseOrderer(),
 					new ExceptionAggregator(),
 					new CancellationTokenSource(),
-					new Dictionary<Type, object>(),
-					collectionFixtures.ToDictionary(fixture => fixture.GetType())
+					new TestableFixtureMappingManager(collectionFixtures)
 				);
 
 		protected override ValueTask<RunSummary> RunTestMethodsAsync(XunitTestClassRunnerContext ctxt)
 		{
 			var result = base.RunTestMethodsAsync(ctxt);
 
-			RunTestMethodsAsync_ClassFixtureMappings = ctxt.ClassFixtureMappings;
+			RunTestMethodsAsync_ClassFixtureMappings = ctxt.ClassFixtureMappings.FixtureCache;
 			RunTestMethodsAsync_TestCaseOrderer = ctxt.TestCaseOrderer;
 
 			return result;
 		}
 
 		public ValueTask<RunSummary> RunAsync() =>
-			RunAsync(testClass, @class, testCases, ExplicitOption.Off, messageBus, testCaseOrderer, aggregator, cancellationTokenSource, assemblyFixtureMappings, collectionFixtureMappings);
+			RunAsync(testClass, @class, testCases, ExplicitOption.Off, messageBus, testCaseOrderer, aggregator, cancellationTokenSource, collectionFixtureMappings);
 
 		protected override ValueTask<RunSummary> RunTestMethodAsync(
 			XunitTestClassRunnerContext ctxt,
@@ -481,5 +477,16 @@ protected override ValueTask<RunSummary> RunTestMethodsAsync(XunitTestClassRunne
 
 			return new(new RunSummary());
 		}
+
+		class TestableFixtureMappingManager : FixtureMappingManager
+		{
+			public TestableFixtureMappingManager(FixtureMappingManager parent) :
+				base("Testable", parent)
+			{ }
+
+			public TestableFixtureMappingManager(params object[] cachedFixtureValues) :
+				base("Testable", cachedFixtureValues)
+			{ }
+		}
 	}
 }
diff --git a/src/xunit.v3.core.tests/Sdk/v3/Runners/XunitTestCollectionRunnerTests.cs b/src/xunit.v3.core.tests/Sdk/v3/Runners/XunitTestCollectionRunnerTests.cs
@@ -274,15 +274,15 @@ public IReadOnlyCollection<TTestCase> OrderTestCases<TTestCase>(IReadOnlyCollect
 	class TestableXunitTestCollectionRunner : XunitTestCollectionRunner
 	{
 		readonly ExceptionAggregator aggregator;
-		readonly IReadOnlyDictionary<Type, object> assemblyFixtureMappings;
+		readonly FixtureMappingManager assemblyFixtureMappings;
 		readonly CancellationTokenSource cancellationTokenSource;
 		readonly IMessageBus messageBus;
 		readonly ITestCaseOrderer testCaseOrderer;
 		readonly IReadOnlyCollection<IXunitTestCase> testCases;
 		readonly _ITestCollection testCollection;
 
 		public Exception? RunTestClassAsync_AggregatorResult;
-		public Dictionary<Type, object>? RunTestClassesAsync_CollectionFixtureMappings;
+		public IReadOnlyDictionary<Type, object>? RunTestClassesAsync_CollectionFixtureMappings;
 		public ITestCaseOrderer? RunTestClassesAsync_TestCaseOrderer;
 
 		TestableXunitTestCollectionRunner(
@@ -292,7 +292,7 @@ class TestableXunitTestCollectionRunner : XunitTestCollectionRunner
 			ITestCaseOrderer testCaseOrderer,
 			ExceptionAggregator aggregator,
 			CancellationTokenSource cancellationTokenSource,
-			IReadOnlyDictionary<Type, object> assemblyFixtureMappings)
+			FixtureMappingManager assemblyFixtureMappings)
 		{
 			this.testCollection = testCollection;
 			this.testCases = testCases;
@@ -313,7 +313,7 @@ class TestableXunitTestCollectionRunner : XunitTestCollectionRunner
 					new MockTestCaseOrderer(),
 					new ExceptionAggregator(),
 					new CancellationTokenSource(),
-					assemblyFixtures.ToDictionary(fixture => fixture.GetType())
+					new TestableFixtureMappingManager(assemblyFixtures)
 				);
 
 		public async ValueTask<RunSummary> RunAsync()
@@ -328,7 +328,7 @@ protected override ValueTask<RunSummary> RunTestClassesAsync(XunitTestCollection
 		{
 			var result = base.RunTestClassesAsync(ctxt);
 
-			RunTestClassesAsync_CollectionFixtureMappings = ctxt.CollectionFixtureMappings;
+			RunTestClassesAsync_CollectionFixtureMappings = ctxt.CollectionFixtureMappings.FixtureCache;
 			RunTestClassesAsync_TestCaseOrderer = ctxt.TestCaseOrderer;
 
 			return result;
@@ -344,5 +344,16 @@ protected override ValueTask<RunSummary> RunTestClassesAsync(XunitTestCollection
 
 			return new(new RunSummary());
 		}
+
+		class TestableFixtureMappingManager : FixtureMappingManager
+		{
+			public TestableFixtureMappingManager(FixtureMappingManager parent) :
+				base("Testable", parent)
+			{ }
+
+			public TestableFixtureMappingManager(params object[] cachedFixtureValues) :
+				base("Testable", cachedFixtureValues)
+			{ }
+		}
 	}
 }