jstemmer / go-junit-report

Convert Go test output to JUnit XML
MIT License
763 stars 222 forks source link

Drop control characters from output #140

Closed janisz closed 1 year ago

janisz commented 1 year ago

Fixes: https://github.com/jstemmer/go-junit-report/issues/138

TomTardigradeSEL commented 1 year ago

I would caution you on just doping the invalid Unicode characters. I ran into this problem with test that are specifically for code that uses some of the Unicode characters that are invalid in the cdata section. Removing them will alter the output from the test results.

I found a way to replace the Unicode character with its quoted escape code. For the case of ANSI color codes, I also found a way to strip the entire color code from the output. By first striping the ANSI code, then reformatting the output for the Unicode characters, I have been able to get junit reports that are valid for XML while retaining any Unicode that is not allowed as their folded equivalent.

jstemmer commented 1 year ago

@TomTardigradeSEL thanks! Yeah, I agree we probably shouldn't just drop the illegal characters. From what I've seen I don't think we can't just escape these illegal characters, unless I'm missing something?

I had a look at the encoding/xml package in the standard library, they replace illegal characters with the unicode replacement character 0xfffd, which renders as �. Let's do the same here when we encounter illegal characters. At this point it's clear from the output that there were some characters that couldn't be displayed, and we can leave it up to the test author to do something about it (for example, change how they print their data).

As for the ANSI escape sequences, removing just the illegal character(s) will still leave some things behind. Detecting and removing these sequences is something we should do, but not in this PR.

janisz commented 1 year ago

Hey, thanks for feedback! I moved code to formatOutput and used xml.EscapeText

janisz commented 1 year ago

I'm not sure how to handle

=== RUN   TestRun/035-whitespace.txt
    go-junit-report_test.go:84: Unexpected report diff (-want, +got):
          strings.Join({
            ... // 983 identical bytes
            "assname=\"package/whitespace\" time=\"0.000\">\n\t\t\t<system-out><![CDA",
            "TA[    whitespace_test.go:31: no-tab\n    whitespace_test.go:32: ",
        -   `   `,
        +   "&#x9;",
            "one-tab\n    whitespace_test.go:33: ",
        -   "\t\ttwo-tab\nno-tab\n\tone-tab\n\t\t",
        +   "&#x9;&#x9;two-tab\nno-tab\n&#x9;one-tab\n&#x9;&#x9;",
            "two-tab]]></system-out>\n\t\t</testcase>\n\t\t<testcase name=\"TestWith",
            "NewlinesFlat\" classname=\"package/whitespace\" time=\"0.000\">\n\t\t\t<s",
            ... // 1118 identical bytes
            "assname=\"package/whitespace\" time=\"0.000\">\n\t\t\t<system-out><![CDA",
            "TA[    whitespace_test.go:31: no-tab\n    whitespace_test.go:32: ",
        -   `   `,
        +   "&#x9;",
            "one-tab\n    whitespace_test.go:33: ",
        -   "\t\ttwo-tab\nno-tab\n\tone-tab\n\t\t",
        +   "&#x9;&#x9;two-tab\nno-tab\n&#x9;one-tab\n&#x9;&#x9;",
            "two-tab]]></system-out>\n\t\t</testcase>\n\t\t<testcase name=\"TestSubT",
            `ests/TestWithNewlinesFlat" classname="package/whitespace" time="`,
            ... // 3[43](https://github.com/jstemmer/go-junit-report/runs/7925604213?check_suite_focus=true#step:5:44) identical bytes
          }, "")
=== RUN   TestRun/036-benchfail.txt
=== RUN   TestRun/037-legacy-fail.txt
    go-junit-report_test.go:84: Unexpected report diff (-want, +got):
          strings.Join({
            ... // 393 identical bytes
            "d\"></failure>\n\t\t</testcase>\n\t\t<testcase name=\"TestTwo\" classname",
            "=\"package/name\" time=\"0.130\"></testcase>\n\t\t<system-out><![CDATA[",
        -   `   `,
        +   "&#x9;",
            "file_test.go:11: Error message\n",
        -   `   `,
        +   "&#x9;",
            "file_test.go:11: Longer\n",
        -   "\t\terror\n\t\t",
        +   "&#x9;&#x9;error\n&#x9;&#x9;",
            "message.\nexit status 1]]></system-out>\n\t</testsuite>\n</testsuite",
            "s>\n",
          }, "")
=== RUN   TestRun/100-pass.gojson.txt
=== RUN   TestRun/101-fail.gojson.txt
=== RUN   TestRun/102-broken.gojson.txt
=== RUN   TestRun/103-subtests.gojson.txt
    go-junit-report_test.go:84: Unexpected report diff (-want, +got):
          strings.Join({
            ... // [56](https://github.com/jstemmer/go-junit-report/runs/7925604213?check_suite_focus=true#step:5:57)4 identical bytes
            `estMultiple/Single" classname="package/name/subtest" time="0.000`,
            "\">\n\t\t\t<failure message=\"Failed\"><![CDATA[    pkg_test.go:20: Do(",
        -   `"a"`,
        +   "&#34;a&#34;",
            "): got aaaaaaaaaa, want a]]></failure>\n\t\t</testcase>\n\t\t<testcase",
            ` name="TestMultiple/Multi" classname="package/name/subtest" time`,
            ... // 102 identical bytes
          }, "")
stefan-zh commented 1 year ago

Hey @janisz are you planning on completing this pull request? My team is also facing this issue https://github.com/jstemmer/go-junit-report/issues/138 and your fix will be very helpful

janisz commented 1 year ago

@stefan-zh I updated PR @jstemmer PTAL

jstemmer commented 1 year ago

Thanks! :)

stefan-zh commented 1 year ago

Thank you @janisz @jstemmer. I confirm that the GitLab CI pipeline for my team was fixed by this change.