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

'Test Failure' reported as 'Success' in TeamCity when message contains Unicode ’ char (RIGHT SINGLE QUOTATION MARK) #30

Open
s3rius2 opened this issue Oct 4, 2017 · 3 comments

Comments

@s3rius2
Copy link

s3rius2 commented Oct 4, 2017

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.

@carlpett
Copy link
Owner

carlpett commented Oct 7, 2017

@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:

private void redirectStreamToLogger(final InputStream s, final RedirectionTarget target) {
new Thread(new Runnable() {
public void run() {
Scanner sc = new Scanner(s);
while (sc.hasNextLine()) {
target.redirect(sc.nextLine());
}
}
}).start();
}

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
Copy link

hboisselle commented Nov 20, 2017

I opened a pull request in XUnit to fix this xunit/xunit#1578. The plugin will have to support XUnit 2.3.X eventually though.

@DerDreschner
Copy link

This pull request from @hboisselle was merged and released with the xUnit 2.4.0 release.

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

4 participants