Prettify Test Method Display Name

[ahmedalejo]

Friendly "Display Name" for test method names

[TestMethod]
void Method_names_should_be_readable_when_hinted()
{
}

"Method_names_should_be_readable_when_hinted" => "Method names should be readable when hinted"


[TestMethod]
void Say_What_You_Have_Spaces_in_Your_CSharp_Test_Names()
{
}

"Say_What_You_Have_Spaces_in_Your_CSharp_Test_Names" => "Say What You Have Spaces in Your CSharp Test Names"

similar to

Complements #466

This spec can be improved for other cases PascalCase, CamelCase.
and specified DisplayName should be favoured

[msftclas]

All CLA requirements met.

[ahmedalejo]

There´s also been other specs #515
and [Proposal Support Automatic "Pretty" Display Name for Test Cases] by xunit/xunit#759

[spottedmahn]

I love this idea. I was thinking about this same topic.

• How do we "fix" all of the existing test names? I'm thinking a lot of test method names are written in PascalCase.
• Do method names matter anymore? What conventions should we be using if using TestMethod(displayName: "Some name")?

[TestMethod("Now I can read my test names like a human not a computer 😜")]
public void Blah1()
{
}

[ahmedalejo]

@spottedmahn

It can be done via two steps

1. Add the string DisplayName property to the TestMethodAttribute
2. Add a string GetDisplayName(System.Reflection.MethodInfo ) that is extensible and by default returns the method name and returns DisplayName if not empty

[TestMethod("Consecutive authentication attempts with invalid credentials should fail")]
public void MyTest()
{ ... }
//or

[MyAppTestMethod]
public void Consecutive_authentication_attempts_with_invalid_credentials_should_fail()
{ ... }

//Given the following definitions
public class TestMethodAttrubte : Attribute
{
    public string DisplayName { get; set; }

    public virtual string GetDisplayName(System.Reflection.MethodInfo methodInfo) =>
        string.IsNullOrWhiteSpace(this.DisplayName) 
        ? methodInfo.Name 
        : this.DisplayName?.Trim();

   ...
}

public class MyAppsTestMethodAttrubte : TestMethodAttrubte
{
    public override string GetDisplayName(MethodInfo methodInfo) =>
        methodInfo
            .Name
            .Replace("_", " ")
            .Replace(".", ", ");
}

[nohwnd]

I like the general idea, and the extension point. But looking at the xunit spec, and the amount of work and iteration they did on it, I am inclined to keep the implementation at just that. No built in automatic rewrites.

That way you can provide your own implementation, which can be simple enough to cover just the conventions you use. This would get the change merged quicker because we will not have to provide a good generic implementation, and multiple configuration options to cater every possible edge case.

[sebainones]

I've seen all 5 commits and reviewed the 'files changed".
In my opinion, it's good but I don't know If I need an specific role in order to Approve this Pull Request.
@nohwnd apologies for the newbie question but : is there any formal procedure (documentation) that I should follow in order to Approve this PR or should I just hit the button? ;)

[sebainones]

I haven't found any Unit Test for this change.
It could be added something like this:

 [TestClass]
    public class TestMethodTest
    {
        [TestMethod]
        public void TestMethodTestConstructorWithTestNameWithUnderscoreShouldReturnReadbleDisplayName()
        {
            MSTest.TestAdapter.ObjectModel.TestMethod testMethod = new MSTest.TestAdapter.ObjectModel.TestMethod("Dummy_Test_Name", "dummyClassName", "dummyAssemblyName", false);

            Assert.AreEqual("Dummy Test Name", testMethod.DisplayName);
        }
    }

[spottedmahn]

Java JUnit 5 Display Name Generators

#interesting 🤔

[nohwnd]

@spottedmahn could you elaborate a bit more on why you find that interesting? 🙂 as I read it it is basically the same concept as here, or am I missing something important?

[ahmedalejo]

Hi guys

cc: @spottedmahn

I plan to close this PR. I prefer sending in one that satisfies the below use case.

What do you think as the following partially implemented this.
#410 issue
#466 PR

[TestMethod("Consecutive authentication attempts with invalid credentials should fail")]
public void MyTest()
{ ... }
//or

[MyAppTestMethod]
public void Consecutive_authentication_attempts_with_invalid_credentials_should_fail()
{ ... }

//Given the following definitions
public class TestMethodAttrubte : Attribute
{
    public string DisplayName { get; set; }

    public virtual string GetDisplayName(System.Reflection.MethodInfo methodInfo) =>
        string.IsNullOrWhiteSpace(this.DisplayName) 
        ? methodInfo.Name 
        : this.DisplayName.Trim();

   ...
}

public class MyAppsTestMethodAttrubte : TestMethodAttrubte
{
    public override string GetDisplayName(MethodInfo methodInfo) =>
        methodInfo
            .Name
            .Replace("_", " ")
            .Replace(".", ", ");
}
public class MyHappyAppsTestMethodAttrubte : MyAppsTestMethodAttrubte 
{
    public override string GetDisplayName(MethodInfo methodInfo) =>
        base.GetDisplayName(methodInfo)
            .Replace("fails", "😢")
            .Replace("should succeed", "😅");
}

[spottedmahn]

🙂 as I read it it is basically the same concept as here

Yep, seems like it. It was interesting to me that another "team" came to the same "discovery" independently.

There might also be some ideas in their approach we could learn from

[ahmedalejo]

@nohwnd I missing something important?
The main thing is that there is more traction in other libraries about this topic. Basically, they´ve implemented it one way or the other.
It's a common need that we have to pull off for testfx

[ahmedalejo]

@spottedmahn @nohwnd

Found a better extension point that could be baked into TestMethodAttribute with ease.

The below is what i´m currently using

cheers.

public class PrettyTestClassAttribute : TestClassAttribute
{
    public override TestMethodAttribute GetTestMethodAttribute(TestMethodAttribute wrappedTestMethodAttribute)
    {
        var attribute = base.GetTestMethodAttribute(wrappedTestMethodAttribute);

        return attribute is PrettyTestMethodAttribute
            ? attribute
            : new PrettyTestMethodAttribute(attribute);
    }
}

public class PrettyTestMethodAttribute : TestMethodAttribute
{
    private readonly TestMethodAttribute wrappedTestMethodAttribute;

    public PrettyTestMethodAttribute(){ }

    public PrettyTestMethodAttribute(TestMethodAttribute wrappedTestMethodAttribute) =>
        this.wrappedTestMethodAttribute = wrappedTestMethodAttribute;

    public override TestResult[] Execute(ITestMethod testMethod)
    {
        TestResult[] results;

        if (wrappedTestMethodAttribute is null)
            results = base.Execute(testMethod);
        else
            results = wrappedTestMethodAttribute.Execute(testMethod);

        if(results.Any())
            results[0].DisplayName = testMethod.TestMethodName
                .Replace("_eq_", " == ")
                .Replace("_neq_", " != ")
                .Replace("_gt_", " > ")
                .Replace("_gte_", " >= ")
                .Replace("_lt_", " < ")
                .Replace("_lte_", " <= ")
                .Replace("Bug_", "🐞");
        return results;
    }
}

Hopefully will create a new pull request suggesting to bake this into TestMethodAttribute