From 4c01bc69d075576dba26b36dc75436fc65d36182 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 7 Jun 2026 21:41:04 +0100 Subject: [PATCH] fix: invoke inner Func for TestDataRow> data sources (#6161) A [MethodDataSource] returning IEnumerable> invokes the Func and spreads a tuple result across the test method parameters. The same source wrapped as IEnumerable>> did not: the inner Func was passed through as a single argument, so it could not bind to the test parameters and parameterized DisplayName placeholders never resolved (causing rows with the same template to collapse into duplicates). - TestDataRowUnwrapper.UnwrapArray now invokes the inner Func after unwrapping the TestDataRow, so TestDataRow> behaves identically to a bare Func source. This is the single convergence point shared by both source-generated and reflection modes, so no codegen changes are needed. - TestDataAnalyzer strips Func<> and TestDataRow<> layers in a loop, so TestDataRow> (as well as Func>) reduces to the inner type and no longer reports a false parameter-count diagnostic. - DisplayNameSubstitutor now performs word-boundary-aware placeholder replacement and resolves positional $argN before $paramName. This fixes a pre-existing prefix-collision bug where a parameter named "a" was matched inside "$arg1" (rendering "1rg1"), also covered by the existing CustomDisplayNameTests.TestParameterNamePrefixBug. Adds Bugs/6161 runtime tests (source-gen + reflection) and an analyzer no-diagnostic test, and documents the tuple + parameterized DisplayName case. --- .../MethodDataSourceAnalyzerTests.cs | 31 ++++++++ TUnit.Analyzers/TestDataAnalyzer.cs | 36 +++++---- TUnit.Core/Helpers/DisplayNameSubstitutor.cs | 74 +++++++++++++++---- TUnit.Core/Helpers/TestDataRowUnwrapper.cs | 8 +- TUnit.TestProject/Bugs/6161/Tests.cs | 55 ++++++++++++++ docs/docs/writing-tests/test-data-row.md | 21 ++++++ 6 files changed, 195 insertions(+), 30 deletions(-) create mode 100644 TUnit.TestProject/Bugs/6161/Tests.cs diff --git a/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs b/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs index 29a3ca8258..653e2ab52b 100644 --- a/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs +++ b/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs @@ -928,6 +928,37 @@ public static IEnumerable>> GetData() ); } + [Test] + public async Task Method_Data_Source_With_TestDataRow_Func_Tuple_No_Error() + { + // TestDataRow> should unwrap the Func and spread the tuple across the + // three parameters (issue #6161). The Func is nested *inside* TestDataRow, so this exercises + // the opposite nesting order from Method_Data_Source_With_Func_TestDataRow_No_Error. + await Verifier + .VerifyAnalyzerAsync( + """ + using TUnit.Core; + using System; + using System.Collections.Generic; + + public class MyClass + { + [MethodDataSource(nameof(GetData))] + [Test] + public void MyTest(int a, int b, int c) + { + } + + public static IEnumerable>> GetData() + { + yield return new(() => (1, 1, 2), DisplayName: "$arg1 + $arg2 = $arg3"); + yield return new(() => (1, 2, 3), DisplayName: "$arg1 + $arg2 = $arg3"); + } + } + """ + ); + } + [Test] public async Task No_Warning_For_Immutable_Record() { diff --git a/TUnit.Analyzers/TestDataAnalyzer.cs b/TUnit.Analyzers/TestDataAnalyzer.cs index fd520ac28d..1d346a4721 100644 --- a/TUnit.Analyzers/TestDataAnalyzer.cs +++ b/TUnit.Analyzers/TestDataAnalyzer.cs @@ -770,22 +770,32 @@ private ImmutableArray UnwrapTypes(SymbolAnalysisContext context, return ImmutableArray.Create(type); } - if (type is INamedTypeSymbol { IsGenericType: true, TypeArguments.Length: 1 } genericType - && genericType.ToDisplayString().StartsWith("System.Func<")) - { - isFunc = true; - type = genericType.TypeArguments[0]; - } - - // Check for TestDataRow wrapper - unwrap to get the inner data type - // This must come after Func unwrapping so that Func> → TestDataRow → T + // Unwrap Func and TestDataRow wrappers in any nesting order, so that Func, + // TestDataRow, Func>, and TestDataRow> all reduce to the inner T + // before the tuple check below. Looping handles either order without assuming which wraps which. var testDataRowTypeSymbol = context.Compilation.GetTypeByMetadataName("TUnit.Core.TestDataRow`1"); - if (testDataRowTypeSymbol != null - && type is INamedTypeSymbol { IsGenericType: true } testDataRowType - && SymbolEqualityComparer.Default.Equals(testDataRowType.OriginalDefinition, testDataRowTypeSymbol)) + bool unwrappedLayer; + do { - type = testDataRowType.TypeArguments[0]; + unwrappedLayer = false; + + if (type is INamedTypeSymbol { IsGenericType: true, TypeArguments.Length: 1 } genericType + && genericType.ToDisplayString().StartsWith("System.Func<")) + { + isFunc = true; + type = genericType.TypeArguments[0]; + unwrappedLayer = true; + } + + if (testDataRowTypeSymbol != null + && type is INamedTypeSymbol { IsGenericType: true } testDataRowType + && SymbolEqualityComparer.Default.Equals(testDataRowType.OriginalDefinition, testDataRowTypeSymbol)) + { + type = testDataRowType.TypeArguments[0]; + unwrappedLayer = true; + } } + while (unwrappedLayer); if (type is INamedTypeSymbol namedType && namedType.IsTupleType) { diff --git a/TUnit.Core/Helpers/DisplayNameSubstitutor.cs b/TUnit.Core/Helpers/DisplayNameSubstitutor.cs index 39170c117e..9d1c0c72fc 100644 --- a/TUnit.Core/Helpers/DisplayNameSubstitutor.cs +++ b/TUnit.Core/Helpers/DisplayNameSubstitutor.cs @@ -1,3 +1,5 @@ +using System.Text; + namespace TUnit.Core.Helpers; /// @@ -28,33 +30,73 @@ public static string Substitute( var result = displayName; var effectiveFormatters = formatters ?? []; + // Substitute by position ($arg1, $arg2, etc.) first. Positional placeholders are more + // specific than single-letter parameter names (e.g. a parameter named "a" would otherwise + // be matched inside "$arg1"), so resolving them up front avoids that collision. + for (var i = 0; i < arguments.Length; i++) + { + var placeholder = $"$arg{i + 1}"; + if (!result.Contains(placeholder)) + { + continue; + } + + var parameterType = i < parameters.Length ? parameters[i].Type : null; + result = ReplacePlaceholder(result, placeholder, ArgumentFormatter.Format(arguments[i], parameterType, effectiveFormatters)); + } + // Substitute by parameter name ($paramName) for (var i = 0; i < parameters.Length && i < arguments.Length; i++) { var paramName = parameters[i].Name; - if (!string.IsNullOrEmpty(paramName)) + if (string.IsNullOrEmpty(paramName)) { - var placeholder = $"${paramName}"; - if (result.Contains(placeholder)) - { - var formatted = ArgumentFormatter.Format(arguments[i], parameters[i].Type, effectiveFormatters); - result = result.Replace(placeholder, formatted); - } + continue; } - } - // Substitute by position ($arg1, $arg2, etc.) - for (var i = 0; i < arguments.Length; i++) - { - var placeholder = $"$arg{i + 1}"; - if (result.Contains(placeholder)) + var placeholder = $"${paramName}"; + if (!result.Contains(placeholder)) { - var parameterType = i < parameters.Length ? parameters[i].Type : null; - var formatted = ArgumentFormatter.Format(arguments[i], parameterType, effectiveFormatters); - result = result.Replace(placeholder, formatted); + continue; } + + result = ReplacePlaceholder(result, placeholder, ArgumentFormatter.Format(arguments[i], parameters[i].Type, effectiveFormatters)); } return result; } + + /// + /// Replaces every occurrence of that is followed by a + /// non-identifier character (or the end of the string) with , so that + /// "$a" is not matched inside "$arg1" or "$abc". + /// + private static string ReplacePlaceholder(string input, string placeholder, string? value) + { + var index = input.IndexOf(placeholder, StringComparison.Ordinal); + if (index < 0) + { + return input; + } + + var builder = new StringBuilder(input.Length); + var searchStart = 0; + + while (index >= 0) + { + var afterIndex = index + placeholder.Length; + var isBoundary = afterIndex >= input.Length || !IsIdentifierChar(input[afterIndex]); + + builder.Append(input, searchStart, index - searchStart); + builder.Append(isBoundary ? value ?? string.Empty : placeholder); + + searchStart = afterIndex; + index = input.IndexOf(placeholder, searchStart, StringComparison.Ordinal); + } + + builder.Append(input, searchStart, input.Length - searchStart); + return builder.ToString(); + } + + private static bool IsIdentifierChar(char c) => char.IsLetterOrDigit(c) || c == '_'; } diff --git a/TUnit.Core/Helpers/TestDataRowUnwrapper.cs b/TUnit.Core/Helpers/TestDataRowUnwrapper.cs index bb9af7d368..e5c9a6c060 100644 --- a/TUnit.Core/Helpers/TestDataRowUnwrapper.cs +++ b/TUnit.Core/Helpers/TestDataRowUnwrapper.cs @@ -68,7 +68,13 @@ public static (object?[] Data, TestDataRowMetadata? Metadata) UnwrapArray(object { if (values.Length == 1 && TryUnwrap(values[0], out var data, out var metadata)) { - // Single TestDataRow - unwrap it + // Single TestDataRow - unwrap it. + // If the inner data is a Func, invoke it so TestDataRow> behaves the same + // as a bare Func data source (the Func is invoked, then tuples are spread into + // individual parameters). Bare Func values are invoked in DataSourceHelpers.ToObjectArray, + // but a Func wrapped in a TestDataRow is opaque there and only surfaces here. + data = DataSourceHelpers.InvokeIfFunc(data); + // If the inner data is already an array, use it directly if (data is object?[] dataArray) { diff --git a/TUnit.TestProject/Bugs/6161/Tests.cs b/TUnit.TestProject/Bugs/6161/Tests.cs new file mode 100644 index 0000000000..a1f4c6249a --- /dev/null +++ b/TUnit.TestProject/Bugs/6161/Tests.cs @@ -0,0 +1,55 @@ +using System.Collections.Concurrent; +using TUnit.TestProject.Attributes; + +namespace TUnit.TestProject.Bugs._6161; + +// Issue #6161: a [MethodDataSource] returning IEnumerable>> must invoke the +// inner Func and spread the resulting tuple into the test method parameters - matching the behaviour +// of a bare IEnumerable> data source. Before the fix the Func was passed through as a +// single argument, so the tuple never bound to (int a, int b, int c) and the parameterized +// DisplayName placeholders could not be substituted (causing rows with the same template to collapse). +[EngineTest(ExpectedResult.Pass)] +public class Tests +{ + private static readonly ConcurrentBag<(int, int, int)> ExecutedTuples = []; + + [Test] + [MethodDataSource(nameof(TupleData))] + public async Task TupleFuncIsUnwrapped(int a, int b, int c) + { + ExecutedTuples.Add((a, b, c)); + await Assert.That(a + b).IsEqualTo(c); + } + + [Test] + [MethodDataSource(nameof(RecordData))] + public async Task ReferenceFuncIsUnwrapped(Calculation calculation) + { + await Assert.That(calculation.First + calculation.Second).IsEqualTo(calculation.Expected); + } + + // Two rows deliberately share the same DisplayName template. Once the Func is invoked the + // placeholders resolve to distinct values, so both rows run as separate test cases. + public static IEnumerable>> TupleData() + { + yield return new(static () => (1, 1, 2), DisplayName: "$arg1 + $arg2 = $arg3"); + yield return new(static () => (1, 2, 3), DisplayName: "$arg1 + $arg2 = $arg3"); + yield return new(static () => (2, 3, 5), DisplayName: "$arg1 + $arg2 = $arg3"); + } + + public static IEnumerable>> RecordData() + { + yield return new(static () => new Calculation(1, 1, 2), DisplayName: "Calc one"); + yield return new(static () => new Calculation(4, 5, 9), DisplayName: "Calc two"); + } + + // Regression for the dedup symptom: if rows had collapsed, fewer than the three distinct tuples + // would have executed. + [After(Class)] + public static async Task AllDistinctRowsExecuted() + { + await Assert.That(ExecutedTuples.Distinct().Count()).IsEqualTo(3); + } + + public record Calculation(int First, int Second, int Expected); +} diff --git a/docs/docs/writing-tests/test-data-row.md b/docs/docs/writing-tests/test-data-row.md index baa5e4c8c5..4346155145 100644 --- a/docs/docs/writing-tests/test-data-row.md +++ b/docs/docs/writing-tests/test-data-row.md @@ -108,6 +108,27 @@ public static IEnumerable>> GetHttpClients() } ``` +The inner `Func` is invoked before the test runs, exactly like a bare `IEnumerable>` +data source. This means a `Func` that returns a tuple is spread across the test method's +parameters, and the resolved values are available to the parameterized `DisplayName`: + +```csharp +public static IEnumerable>> GetSums() +{ + yield return new(() => (1, 1, 2), DisplayName: "$arg1 + $arg2 = $arg3"); + yield return new(() => (1, 2, 3), DisplayName: "$arg1 + $arg2 = $arg3"); +} + +[Test] +[MethodDataSource(nameof(GetSums))] +public async Task Adds(int a, int b, int expected) +{ + await Assert.That(a + b).IsEqualTo(expected); +} +``` + +The two cases display as `1 + 1 = 2` and `1 + 2 = 3`. + ## Skipping Individual Test Cases Use the `Skip` property to skip specific test cases while keeping others active: