Skip to content

Use enum names instead of numeric values in test display names#5178

Merged
thomhurst merged 4 commits intomainfrom
copilot/feature-use-enum-name-for-test-display
Mar 17, 2026
Merged

Use enum names instead of numeric values in test display names#5178
thomhurst merged 4 commits intomainfrom
copilot/feature-use-enum-name-for-test-display

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 17, 2026

MatrixDataSourceAttribute uses Enum.GetValuesAsUnderlyingType() to auto-generate enum parameter values, which returns boxed numeric types (e.g., int 0, 1, 2). These bypass the o is Enum check in ArgumentFormatter.FormatDefault(), causing test names like RoundingWorks(1) instead of RoundingWorks(AwayFromZero).

Changes

  • Add parameter-type-aware Format overload to ArgumentFormatter that accepts a Type? parameterType — when the parameter type is an enum and the value is numeric, it converts to the enum name for display via Enum.ToObject()
  • Update display name callers (TestContext.GetDisplayName(), DisplayNameAttribute, DisplayNameSubstitutor) to pass parameter type information to the new formatter overload
  • No changes to parameter injection or exclusion logicMatrixDataSourceAttribute is untouched; only display formatting is affected
  • Update PublicAPI snapshots for the new public method

Before / After

ExcludingAutoGeneratedMatrixValues(1, True)   →  ExcludingAutoGeneratedMatrixValues(One, True)
MatrixTest_Enum(2, One, 2)                    →  MatrixTest_Enum(2, One, Two)
RoundingWorks(1)                              →  RoundingWorks(AwayFromZero)
Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature]: Use enum name for test display name by default</issue_title>
<issue_description>### Problem Statement

When using enums as matrix parameters for tests the test name is using the numerical value. E.g.

internal class Test
{
	[Test]
	[MatrixDataSource]
	public async Task RoundingWorks([Matrix]MidpointRounding mode)
	{
		var result = Math.Round(1.5, mode);
		var expected = mode switch
		{
			MidpointRounding.ToEven => 2.0,
			MidpointRounding.AwayFromZero => 2.0,
			MidpointRounding.ToZero => 1.0,
			MidpointRounding.ToNegativeInfinity => 1.0,
			MidpointRounding.ToPositiveInfinity => 2.0,
			_ => throw new InvalidOperationException($"Unexpected mode: {mode}")
		};

		await Assert.That(result).IsEqualTo(expected);
	}
}

Results in test names of: RoundingWorks(1), RoundingWorks(2) etc...

Proposed Solution

I would like TUnit to use the enum name for display purposes so the above code produces:

RoundingWorks(ToEven), RoundingWorks(AwayFromZero) and so on

Alternatives Considered

This could probably also be done via a custom DisplayNameFormatterAttribute.

Feature Category

Test Discovery / Attributes

How important is this feature to you?

Important - significantly impacts my workflow

Additional Context

No response

Contribution

  • I'm willing to submit a pull request for this feature</issue_description>

Comments on the Issue (you are @copilot in this section)


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…trixDataSource

Change MatrixDataSourceAttribute to convert underlying type values back to enum
instances using Enum.ToObject(), so ArgumentFormatter.FormatDefault() correctly
identifies them as enums and displays their names (e.g., "ToEven") instead of
numeric values (e.g., "0").

Fixes: #4059

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
Copilot AI changed the title [WIP] [Feature] Use enum name for test display name by default Use enum names instead of numeric values in MatrixDataSource test display names Mar 17, 2026
Copilot AI requested a review from thomhurst March 17, 2026 10:35
@thomhurst
Copy link
Copy Markdown
Owner

@copilot I only want to affect the display name logic, not the actual parameter injection/exclusion logic

Copilot AI and others added 2 commits March 17, 2026 11:30
… parameter injection

Revert MatrixDataSourceAttribute.cs to original (no changes to parameter
injection/exclusion logic). Instead, add a parameter-type-aware Format overload
to ArgumentFormatter that converts numeric values to enum names for display only.

Updated callers: TestContext.GetDisplayName(), DisplayNameAttribute,
DisplayNameSubstitutor. Updated PublicAPI snapshots for the new public method.

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
…ption type

Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 17, 2026

@copilot I only want to affect the display name logic, not the actual parameter injection/exclusion logic

Reverted MatrixDataSourceAttribute.cs completely — parameter injection/exclusion logic is untouched. The fix now only affects display name formatting by adding a parameter-type-aware Format overload to ArgumentFormatter that converts numeric values to enum names when the parameter type is known to be an enum. Updated callers: TestContext.GetDisplayName(), DisplayNameAttribute, and DisplayNameSubstitutor. See d467e11.

Copilot AI changed the title Use enum names instead of numeric values in MatrixDataSource test display names Use enum names instead of numeric values in test display names Mar 17, 2026
Copilot AI temporarily deployed to Pull Requests March 17, 2026 11:46 Inactive
Copilot AI temporarily deployed to Pull Requests March 17, 2026 11:46 Inactive
@thomhurst thomhurst marked this pull request as ready for review March 17, 2026 12:45
@thomhurst thomhurst merged commit 4295577 into main Mar 17, 2026
13 of 15 checks passed
@thomhurst thomhurst deleted the copilot/feature-use-enum-name-for-test-display branch March 17, 2026 12:45
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 17, 2026

Code Review

This PR correctly fixes the root issue: MatrixDataSourceAttribute uses Enum.GetValuesAsUnderlyingType() which returns boxed numeric types (e.g., int 0, 1, 2) that bypass the o is Enum check in FormatDefault(). The fix is well-scoped — it only touches display formatting and deliberately leaves parameter injection/exclusion logic untouched (as requested by the repo owner).

What works well

  • The approach is correctly layered: the fix is applied at the formatting layer rather than the data generation layer, which avoids any risk of breaking exclusion logic or parameter injection.
  • Nullable enum handling (Nullable.GetUnderlyingType(parameterType) ?? parameterType) is correct.
  • The try/catch ArgumentException guard around Enum.ToObject() is appropriate defensive coding.
  • The DisplayNameSubstitutor correctly handles the $arg{N} positional placeholder path with the i < parameters.Length bounds check.

Concerns

1. Code duplication in the two Format overloads

The new public overload repeats the formatter loop verbatim from the original:

public static string Format(object? o, List<Func<object?, string?>> formatters)
public static string Format(object? o, Type? parameterType, List<Func<object?, string?>> formatters)

The original overload could simply delegate to the new one:

public static string Format(object? o, List<Func<object?, string?>> formatters)
    => Format(o, parameterType: null, formatters);

This removes the duplicated loop and makes the relationship between the two overloads explicit. Currently a future change to the formatter loop (e.g., short-circuiting, logging) would need to be applied in two places.

2. TestDataFormatter.FormatArguments has not been updated

TestDataFormatter.FormatArguments(object?[] arguments, List<Func<object?, string?>> formatters) still calls ArgumentFormatter.Format(arguments[i], formatters) (the type-unaware overload). This path is used when displaying test data from TestDataRow and other data sources. If someone uses [MatrixDataSource] and a data-source display name goes through TestDataFormatter, the enum names would still show as numbers. Consider whether this path also needs the type-aware overload, or at minimum add a comment explaining why it is intentionally excluded.

3. GetConstantValue still uses the type-unaware overload

ArgumentFormatter.GetConstantValue(TestContext, object?) calls Format(o, testContext.ArgumentDisplayFormatters) without passing parameter type. Depending on where constant values are displayed, matrix-generated enum arguments could still be shown as numbers through this path.

4. Enum.ToObject with an already-enum value

The guard !type.IsEnum correctly prevents a double-conversion when the value is already an enum. However, if the runtime ever passes a value whose GetType() is an enum of a different type than resolvedParameterType (e.g., a flag combination stored as a long), the Enum.ToObject call will succeed but may produce misleading output. This is a very edge-case scenario and the current behaviour (silently producing a valid-looking name) is acceptable, but worth noting.

Summary

The fix correctly solves the reported issue and respects the constraint of not touching injection/exclusion logic. The main actionable improvements are: (1) eliminating the duplicated formatter loop by delegating from the old overload to the new one, and (2) reviewing whether TestDataFormatter.FormatArguments needs the same treatment for consistency. Everything else is minor.

This was referenced Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Use enum name for test display name by default

2 participants