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

Allow Overriding the Test Name #410

Closed
spottedmahn opened this issue Apr 25, 2018 · 18 comments
Closed

Allow Overriding the Test Name #410

spottedmahn opened this issue Apr 25, 2018 · 18 comments

Comments

@spottedmahn
Copy link
Contributor

Description

It would be very handy if we could control the TestName so we could create nice human-readable test names that show up in the test explorer.

Test frameworks like Mocha support this 😊. One example.

Reference: SO Post

Expected behavior

[TestMethod("My user friendly test name with special characters like | or * or \\ etc.")]
public void MyTest()
{
    ...
}

or

[TestMethod()]
[TestName("My user friendly test name with special characters like | or * or \\ etc.")]
public void MyTest()
{
    ...
}

Actual behavior

The method name is the test name 😪.

@spottedmahn
Copy link
Contributor Author

spottedmahn commented Apr 25, 2018

Is the code that decides what to display in Visual Studio in this repo? I would imagine not. I would imagine, it is part the of Visual Studio code base?

This looks promising: Microsoft.VisualStudio.TestWindow.Interfaces

@spottedmahn
Copy link
Contributor Author

Example: Test name from From testx repo

ExecuteShouldSkipTestAndSkipFillingIgnoreMessageIfIgnoreAttributeIsPresentOnTestClassButHasNoMessage

vs

Execute should skip this test and skip filling IgnoreMessage If IgnoreAttribute is present on the TestClass but has no message

which one is easier for you to read? 😜

@cltshivash
Copy link
Contributor

@spottedmahn : This repo has the code for the test framework and decides the name that will be decided in VS. The ask is valid and we will look into it. we are open for contributions as well.

@spottedmahn
Copy link
Contributor Author

spottedmahn commented Apr 29, 2018

That’s awesome!

Where can I read more about how to test such a change? Let’s say I track down the code and change it. How do I point Visual Studio at my new custom assemblies?

@spottedmahn
Copy link
Contributor Author

How do I point Visual Studio at my new custom assemblies?

nvm, I think I found it (now that I'm a full pc and not my phone).

Contribution Guide

This article will help you build, test and consume local builds of the MSTest Framework and Adapter.

@spottedmahn
Copy link
Contributor Author

spottedmahn commented Jun 3, 2018

Making progress!!

image

image

Branch w/ changes: spottedmahn/testfx/tree/friendly-test-names

@spottedmahn
Copy link
Contributor Author

A few issues (and I'm sure there will be more)

  • If I put a '.' in my DisplayName the test explorer window strips the left hand side
    • i.e. "This is a test w/ a period. Something else" I get " Something Else"
  • I had to hack it up when the test is passed back from VS. Source
internal static UnitTestElement ToUnitTestElement(this TestCase testCase, string source)
{
	var isAsync = (testCase.GetPropertyValue(Constants.AsyncTestProperty) as bool?) ?? false;
	var testClassName = testCase.GetPropertyValue(Constants.TestClassNameProperty) as string;

	// method name from fully qualified name, feels hacky
	var parts = testCase.FullyQualifiedName.Split('.');
	var methodName = parts[parts.Length - 1];
	TestMethod testMethod = new TestMethod(methodName, testClassName, source, isAsync);

	UnitTestElement testElement = new UnitTestElement(testMethod)
	{
		IsAsync = isAsync,
		TestCategory = testCase.GetPropertyValue(Constants.TestCategoryProperty) as string[],
		Priority = testCase.GetPropertyValue(Constants.PriorityProperty) as int?
	};

	return testElement;
}

@jayaranigarg
Copy link
Member

@spottedmahn :

  • We will figure out the '.' part once your PR is out.
  • Can you tell us the scenario where earlier logic of testcase.DisplayName is not working for you? TestCase displayname is already constructed in a manner what you are trying to achieve. Refer this

@spottedmahn
Copy link
Contributor Author

We will figure out the '.' part once your PR is out.

Great 🎉!


Can you tell us the scenario where earlier logic of testcase.DisplayName is not working for you?

Hmm... not 100% what you are referring to. I'm going to assume you're referring to:

I had to hack it up when the test is passed back from VS.

When I run a test from the Test Explorer window, it is unable to find the test:

Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel.TypeInspectionException: 'Method UnitTestProject2.UnitTest1.Say What, You Have Spaces in Your C# Test Names??!! does not exist.'

The problem ToUnitTestElement() is using the DisplayName as the TestMethod.Name:

https://github.com/Microsoft/testfx/blob/e13ca085694aa9a9908afb3e9a90a58841b5fe03/src/Adapter/MSTest.CoreAdapter/Extensions/TestCaseExtensions.cs#L26

@spottedmahn
Copy link
Contributor Author

Hi @jayaranigarg - any updates?

@jayaranigarg
Copy link
Member

jayaranigarg commented Jun 28, 2018

@spottedmahn : Apologies for the delay.

Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel.TypeInspectionException: 'Method UnitTestProject2.UnitTest1.Say What, You Have Spaces in Your C# Test Names??!! does not exist.'

This exception is generated from here. This effectively means there is some discrepancy in testMethod.Name

I believe we will have to fix DisplayName in TestCase at the time of creation of testcase object here.

If you need further help debugging the issue, consider raising a PR on your clone of repo and share that PR with us?

@spottedmahn
Copy link
Contributor Author

Hi @jayaranigarg - PR submitted... sorry it took so long...

@spottedmahn
Copy link
Contributor Author

implemented via #466

@ahmedalejo
Copy link

ahmedalejo commented May 1, 2020

Just making sure everyone sees this solution for #466

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

This is what i´m currently using:

cheers.

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

        return attribute as PrettyTestTestMethodAttribute ?? new PrettyTestTestMethodAttribute(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 = testMethodAttribute is null
             ? base.Execute(testMethod)
             : testMethodAttribute.Execute(testMethod);

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

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

@spottedmahn
Copy link
Contributor Author

Hi @ahmedalejo - nice! Please share a link to the new PR once created, thanks!

@nohwnd
Copy link
Member

nohwnd commented May 4, 2020

@ahmedalejo Fixed the code above to actually compile and space after bug. That approach is genius.

using Microsoft.VisualStudio.TestTools.UnitTesting;
using System.Linq;

namespace UnitTestProject84
{
    [PrettyTestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void Bug_Test_method_1()
        {
        }
    }

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

            return attribute as PrettyTestMethodAttribute ?? 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 = wrappedTestMethodAttribute is null
                ? base.Execute(testMethod)
                : 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_", "🐞 ")
                    .Replace("_", " ");

            return results;
        }
    }
}

@mdpopescu
Copy link

mdpopescu commented Jul 19, 2020

Nice, I just discovered this!

One observation: the parameter description on the constructor here is incorrect:

/// <param name="displayName">Message specifies reason for ignoring.</param>
public TestMethodAttribute(string displayName)

It was probably copied from Ignore and hasn't been changed correctly.

@nohwnd
Copy link
Member

nohwnd commented Aug 26, 2020

@mdpopescu was fixed in #715

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

No branches or pull requests

6 participants