-
Notifications
You must be signed in to change notification settings - Fork 263
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
Friendly test names #466
Friendly test names #466
Conversation
Hi @jayaranigarg - any thoughts? thanks! |
src/Adapter/MSTest.CoreAdapter/Extensions/TestCaseExtensions.cs
Outdated
Show resolved
Hide resolved
@spottedmahn : Apologies for the delay. I have added few comments. Please look into them. Can you please add UnitTests as well for these changes? Also, clicking on 'Details' showing |
@dotnet-bot : Test this please |
Guys, this one looks like it has been sit here for a while. If so, @spottedmahn is there anything I can help you with to complete this PR? If you don't have time to finish it, would you mind providing me with write-access to your remote branch, so I can push whatever is missing? Or, I could create a fork from it, and submit a new PR with your changes and what's missing. Let me know... |
Hi @parrainc 👋
Not sure how to do that. Wonder if that is only a thing with organizations? Got a URL you can share on how to do that? |
Hi @parrainc 👋
Need to tackle that... |
sure, you can follow this guide in order to provide me with access to your repo. This is needed so I can update this PR by pushing code to your repo, instead of having to open another PR from this.
|
Oh, that was easy 👍, done ✅. How didn't I see that 🤦♂️?! |
Hi @jayaranigarg - any updates for us? Thanks |
I think it's been ready for a while now. Could be merged any time when possible, @jayaranigarg |
Fixed tests to prove that UnitTestElement is setting the DisplayName and that it does not use the Display name as the method name. Using Display Name as method name leads to searching for methods like "Class1.Display Name" which won't be found and the test runner will fail.
@AbhitejJohn Fixed and seems good to go. The fixed tests actually prove on unit test level the thing that was causing it to fail only in acceptance. Is there more code review needed? Attaching the project I used to figure out the failures in case someone comes back to this in the future, or in case more changes are needed from my side. |
test/UnitTests/MSTest.CoreAdapter.Unit.Tests/Discovery/TypeEnumeratorTests.cs
Outdated
Show resolved
Hide resolved
Looks like |
The usage of ReflectHelper is a bit nuanced, the generic method I used originally uses a static method interallly which prevents it from being mocked. So in order to write the tests in the same way as the other tests are written I use the mock to act like the method is decorated with the correct attribute, and need to use a different method internally but at least the generic method is not the sigle outlier in that file.
Status: So when does this become available? Is this delivered via the |
Dunno all the details, I am on this team for just a few days, but my next step is to learn how to release this as a preview to nuget, so expect it soonish 😁 |
Thanks @spottedmahn and @nohwnd for getting this through. Daily builds should be available on the myget feed linked in the repo's readme. However we'd definitely want to put this out to nuget as a preview package post the regular validation so this can be consumed. |
Great! Did we change the adapter or the framework package? Seems like the adapter... |
@spottedmahn : That would be both the framework and the adapter because we had the TestMethod attribute change. It doesn't look like the latest build has your changes just yet. I'm hoping we should have a package tomorrow. |
Oh, cool, thanks!
🤞 |
Testing out |
Found a better and simpler extension point that could be baked into 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 |
related to #410 Allow Overriding the Test Name