-
Notifications
You must be signed in to change notification settings - Fork 9
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
Handle illegal xml characters in test data #37
Handle illegal xml characters in test data #37
Conversation
…ML/XmlPersistence.cs
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #37 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 24 +1
Lines 608 698 +90
=========================================
+ Hits 608 698 +90
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@codito Could you take a look at the couple open questions I have here? |
@@ -40,13 +40,13 @@ public class XunitTestAdapter : ITestAdapter | |||
|
|||
foreach (var result in results) | |||
{ | |||
if (skippedTestNamesWithReason.TryGetValue(result.Result.TestCase.DisplayName, out var skipReason)) | |||
if (skippedTestNamesWithReason.TryGetValue(result.DisplayName, out var skipReason)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adapter specific implementation - we should validate if Xunit is copying Displayname to TestResult
and using it for skip reason instead of TestCase.Displayname
.
In case of Theory
test case, a single test case can have multiple TestResult
and accordingly the DisplayName
may change too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now have TestResultDisplayName
and TestCaseDisplayName
on the result info. So the string being used here won't have changed.
I made those properties internal so the downstream loggers would be forced to use Method
. Currently Xunit and Nunit loggers are using new XAttribute("name", result.TestCaseDisplayName),
So, I guess those loggers weren't getting the benefit of the ITestAdapters. Not sure how much of an impact it might make on results we output.
test/TestLogger.UnitTests/TestDoubles/JsonTestResultSerializer.cs
Outdated
Show resolved
Hide resolved
...utput/Json.TestLogger.NUnit.NetCore.Tests-;Parser=Legacy-IncludesParserFailures.verified.txt
Show resolved
Hide resolved
...ptanceTests/VerifyTestRunOutput/Json.TestLogger.NUnit.NetFull.Tests-WindowsOnly.verified.txt
Show resolved
Hide resolved
@codito Quick update before i stop for the week. I think this is pretty close to done.
|
test/Json.TestLogger.TestAdapter/Json.TestLogger.TestAdapter.csproj
Outdated
Show resolved
Hide resolved
@Siphonophora thank you for the contribution. PR is looking great to me. |
@codito Let me know what you think. I think this may be done |
No description provided.