Build failure in tests are not reported

[tzachshabtay]

Description

We're running go test -coverprofile=coverage.out -v ./... 2>&1 | go-junit-report -iocopy -set-exit-code -out junit.xml, and even though we had tests that failed to build, we got an exit code of 0 and the failures were not in the report.

Note that the problem is in specific tests (more on that later in the analysis part), some types of tests do report when they have build failures.

In go test output, the failures were reported like this:

# our.company.name/some/path/parser_test [our.company.name/some/path/parser.test]
compiler error description here

...
later on
...

FAIL    our.company.name/some/path/parser [build failed]

In the xml output I see this:

<testsuite name="our.company.name/some/path/parser" tests="0" failures="0" errors="0" id="292" hostname="5e7b50c95ee9" time="0.000" timestamp="2022-09-16T18:48:08Z"></testsuite>

Analysis

As far as I can tell, for tests that have the same package name as the packages they're testing, this works fine. For tests that have a _test suffix, this works fine if there were no sub-tests, but in this case the compilation error was in a sub-test which resulting in the output above, i.e # test_package_name [package_name.test] and not # package_name like the other examples.

In the code (I looked at the code, I didn't debug it so I can be wrong) I see it's setting the package name to the first token, and ignoring the second one: https://github.com/jstemmer/go-junit-report/blob/7b10b42854626aba4122d985cb2624025470dd37/parser/gotest/gotest.go#L215

So it's using the test package name and not the package name when recording the build error.

When processing the summary event, it tries to compare the package name in the summary event (i.e parser) with the test package name in the build error (i.e parser_test), which means it misses out on the build failure: https://github.com/jstemmer/go-junit-report/blob/7b10b42854626aba4122d985cb2624025470dd37/parser/gotest/report_builder.go#L148

It then checks if the package "is empty" before even looking at the summary result: https://github.com/jstemmer/go-junit-report/blob/7b10b42854626aba4122d985cb2624025470dd37/parser/gotest/report_builder.go#L167

So even though the result states "FAIL", because we have no output for the package and no known tests it completely ignores the "FAIL" status and continues.

So in short, I think there are 2 issues here:

1. Package name is parsed wrong for this scenario (or alternatively the comparison should also match with _test suffix)
2. A "FAIL" status for a package is ignored if it's empty

I think the second issue is even more critical then the first as it safe-proofs future incidents of parsing errors like this, i.e not having the build failures in the output is annoying, but having the "FAIL" status ignored means exit code is 0 and we can unknowingly ship untested code to production

[jstemmer]

Thanks for the excellent report.

I agree that the second issue is the most important, we should never exit with code 0 if there was any failure or error. I've made some changes to make sure we catch some unusual cases where this could happen.

I've also implemented your first suggestion to make sure build errors in test packages with the _test suffix are correctly matched to the package under test.

I'll close this as fixed, but feel free to reopen if you think I've missed any other cases.