carlpett
commented
6 years ago @s3rius2 Thank you for noticing and a very good analysis! And also a repro! I must say I'm not sure exactly where this happens, though. The output is basically passed straight through to TeamCity's internals: https://github.com/carlpett/xUnit-TeamCity/blob/26fd13e3764a7fa37e9d9e39123120fb8cce6c83/xunit-agent/src/main/java/se/capeit/dev/xunittestrunner/XUnitBuildProcess.java#L179-L188
I would have assumed this would avoid any encoding issues. However, I'll do a bit of digging and brush up the Java and see what I can find.
hboisselle
DerDreschner
Using xUnit 2.2 and TeamCity 10.0.4 on Windows.
Reproduction
Please see the reproduction repo. It contains one test.
Point TeamCity at the repo Add a 'Nuget Restore step' in the beginning Add an off-the-shelf xUnit runner step with '*/.Tests.dll' Observe: Despite the runner exits with non-zero code (thus correctly failing the build), the TeamCity report shows the test itself as successful. Analysis
Inspecting the log shows a message:
teamcity[testFailed name='xUnitInTeamCity.ReproForUnicodeEscapingIssue.Tests.IssueReproTests.FailWithMessageContainingNonAsciiChars' details='Assert.Equal() Failure|r|n (pos 0)|r|nExpected: ' +Ø%D|r|nActual: anything different so that test fails|r|n (pos 0)|r|n at xUnitInTeamCity.ReproForUnicodeEscapingIssue.Tests.IssueReproTests.FailWithMessageContainingNonAsciiChars() in d:\Teamcity\BuildAgent\work\c80283b4cb57ca79\xUnitInTeamCity.ReproForUnicodeEscapingIssue.Tests\IssueReproTests.cs:line 25' flowId='4862240aeba54317941ec7648b984516']
Incorrect property name. Valid property list format is (name( )=( )'escaped_value'( )) where escape symbol is "|" As noted in the repro source, we have traced this down to the problem of console encoding, which causes the special characters to be changed to their 'simplified non-Unicode' version upon output. Especially the ’ (RIGHT SINGLE QUOTATION MARK) character is of note, because it gets changed to simple ', which gets to TeamCity unescaped and prevents it from receiving the test failure command.
Severity
As for severity, while thanks to the non-zero exit code the developers should notice the problem, some wrapping scripts may sometimes not pass that to the end, making the failure completely unnoticed (notably xUnit.net-dotCover from meta-runner-power-pack has this issue as of present).
Fix suggestion
Even though some solutions may exist which make the pipeline not change the characters, I think that it might be good to implement a fix that works out-of-the-box in any setup. That is, just use the TeamCity's \uNNNN escaping for any special characters beyond ASCII. After all, these commands are not seen by human in a normal run. The extra benefit will be that all other special characters will start showing in TeamCity without getting changed.
Take note there's even a similar Pull Request to in JetBrains' utility library for outputting the commands that also suggest always doing it under the hood.