Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestMethod displayName is being ignored if DataRowAttribute is specified #1635

Closed
HenryvanderVegte opened this issue Apr 18, 2023 · 6 comments · Fixed by #3366
Closed

TestMethod displayName is being ignored if DataRowAttribute is specified #1635

HenryvanderVegte opened this issue Apr 18, 2023 · 6 comments · Fixed by #3366
Assignees
Labels
Area: Parameterized tests Help-Wanted The issue is up-for-grabs, and can be claimed by commenting In-PR State: Approved State: In-PR

Comments

@HenryvanderVegte
Copy link

Describe the bug

When specifying a displayName in a TestMethod, it works as expected and the display name is picked up.
However, if a DataRowAttribute is used on a TestMethod, the displayName of the TestMethod is being ignored.

Versions:

    <PackageReference Include="MSTest.TestAdapter" Version="3.0.2" />
    <PackageReference Include="MSTest.TestFramework" Version="3.0.2" />

Steps To Reproduce

Provided 2 tests, both with custom displayNames, one with a DataRowAttribute, and another one without:

namespace UnitTesting
{
    [TestClass]
    public class UnitTests
    {
        [TestMethod("SomeCustomDisplayName")]
        public void TestMethod1()
        {
        }

        [TestMethod("SomeCustomDisplayName2")]
        [DataRow("SomeDataRow")]
        public void TestMethod2(string dataRowObject)
        {
        }
    }
}

Expected behavior

Expected behavior is that the custom display name is picked up for both tests, like:

UnitTests (2)
--SomeCustomDisplayName
--SomeCustomDisplayName2 (SomeDataRow)

Actual behavior

TestMethod2 does not pick up the custom name

Screenshot 2023-04-18 101905

@Evangelink
Copy link
Member

Hi @HenryvanderVegte,

Sorry for the delay in triaging this issue! Thank you for the detailed explanation and working repro case!

I have tested with older versions of MSTest and can conclude this is not a regression so I will proceed and mark this issue as a feature request more than a bug. Because the code base was inherited it's hard to say what was part of the initial design.

There is a DisplayName property on each DataRowAttribute that can be set to change the name of this specific entry (it will then ignore data).

I think that your initial expectation is coherent to me and this seems to be something we should improve.

@Evangelink Evangelink added Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Type: Feature Area: Parameterized tests State: Approved and removed Needs: Triage 🔍 labels Apr 28, 2023
@HenryvanderVegte
Copy link
Author

Hey @Evangelink,

thanks for looking into this!

I saw the DisplayName property on the DataRowAttribute, but there are 2 issues with this for me:
1.) As you already mentioned, it then ignores the Data, so what I'd have to do then is something like:

        [TestMethod("SomeCustomDisplayName2")]
        [DataRow("SomeDataRow1", DisplayName = "SomeCustomDisplayName2 (SomeDataRow1)")]
        [DataRow("SomeDataRow2", DisplayName = "SomeCustomDisplayName2 (SomeDataRow2)")]
        [DataRow("SomeDataRow3", DisplayName = "SomeCustomDisplayName2 (SomeDataRow3)")]
        public void TestMethod2(string dataRowObject)
        {

which is not great..

2.) I have a test class attribute to overwrite to prefix a version to each test in a testclass, like:

    public sealed class VersionedTestClassAttribute : TestClassAttribute
    {
        public VersionedTestClassAttribute(ApiVersion apiVersion)
        {
            this.ApiVersion = apiVersion;
        }

        public ApiVersion ApiVersion { get; }

        public override TestMethodAttribute GetTestMethodAttribute(TestMethodAttribute testMethodAttribute)
        {
            var attribute = base.GetTestMethodAttribute(testMethodAttribute);
            return new ApiVersionAttribute(this.ApiVersion, attribute);
        }
    }

and in the ApiVersionAttribute I'm updating the displayName. This works well for TestMethods without a DataRow, but won't update the name if the TestMethod has a DataRow. I was hoping that this problem would also be solved if the TestMethod Name would get picked up.

Thanks a lot!

@Evangelink Evangelink added this to the 3.1.0 milestone Jun 5, 2023
@Evangelink Evangelink modified the milestones: 3.1.0, 3.2.0 Jul 10, 2023
@Evangelink
Copy link
Member

Moved to milestone 3.2

@makingbloke
Copy link

makingbloke commented Aug 8, 2023

@Evangelink @HenryvanderVegte

I found a related issue and asked about it on StackOverflow. They suggested I ask on this ticket if you consider this as part of the bug, if not I'll open a new ticket.

I'm using VS2022 17.6.5, .NET 7, MS Test SDK 17.7.0, MS Test Adapter and TestFramework 3.1.1.

  1. There is a work around for the DisplayName not being shown for a test that has a DataRow attribute. Add a dummy object parameter to your test and a "typeof(object)" for to each DataRow. (See sample below). The test DisplayName starts working.

  2. When the DisplayName is not shown (no dummy parameter) then the output for the test is a separate Test Detail Summary for each DataRow. But with the dummy parameter added the displays a Test Detail Summary that shows all the results (Far more useful IMO).

[TestClass]
public class UnitTest1
{
    [TestMethod("Test Method")]
    [DataRow("String#1", 1, DisplayName = "String#1-1 Display Name")]
    [DataRow("String#2", 2, DisplayName = "String#2-2 Display Name")]
    public void Test(string s, int i)
    {
        Console.WriteLine($"{s}-{i}");
    }

    [TestMethod("Test Method with Dummy object")]
    [DataRow("String#1", 1, typeof(object), DisplayName = "String#1-1 Display Name")]
    [DataRow("String#2", 2, typeof(object), DisplayName = "String#2-2 Display Name")]
    public void TestWithDummy(string s, int i, object dummy)
    {
        Console.WriteLine($"{s}-{i}-{dummy}");
    }
}

image

image

Cheers,

Mike

@Evangelink Evangelink modified the milestones: 3.2.0, 3.3.0 Dec 7, 2023
@nohwnd
Copy link
Member

nohwnd commented Jan 10, 2024

DataRow is implemented using the ITestDataRow extension point, so this looks like a bug in the "new" view, where each data row is shown as a single test.

The typeof(Object) forces the row to not be serializable according to the rules, and so the "old" approach of putting results under one test is used. Here are more details of what works and does not work, in this thread where I struggle to make it work for my custom ITestDataSource #1054

Imho the best solution here is to implement the same thing that was here before, where the display name is inherited and used together with the data, unless the user defines the name directly on the datarowattribute, in that case we overwrite the whole name.

@Evangelink
Copy link
Member

Thanks for the investigation @nowhnd.

I double checked the code and I think you are right, the only proper way to fix would be to do a breaking change to allow passing extra args. I would suggest to move this to the v4 milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Parameterized tests Help-Wanted The issue is up-for-grabs, and can be claimed by commenting In-PR State: Approved State: In-PR
Projects
None yet
4 participants