jstemmer / go-junit-report

Convert Go test output to JUnit XML
MIT License
776 stars 224 forks source link

support Go 1.10's test -json option #66

Closed ixdy closed 6 years ago

ixdy commented 6 years ago

Go 1.10 now supports printing test results as json.

There's probably still some munging necessary to turn this into JUnit suitable for CI systems.

Is this functionality worth implementing in go-junit-report?

Or should a separate wrapper be written?

jstemmer commented 6 years ago

Thanks for the link and suggestion, that's certainly interesting.

While it's nice that basic events would be easier to read from json, but it appears to fall back to just encoding the raw test output line by line in json for many different cases. We'd still have to parse the output field for a lot of the edge cases go-junit-report has accumulated over the years. Take a look at what go tool test2json produces for some of the outputs in the tests dir for example.

Glancing at the test2json.go source code it looks like it's a (much simpler) go test output parser. So by supporting this we'd basically introduce an extra layer of indirection. I'm not convinced at this point that it's worth the effort to add support for this. Is there maybe anything I overlooked?

sigma commented 6 years ago

The json output is definitely no panacea (see for example golang/go#23037) but I still think there's a bit of value there. In particular, the parsing should be slightly more robust due to https://golang.org/src/cmd/go/internal/test/test.go using a lockedStdout object in place of stdout, so that logs interleaving doesn't happen. It also has the benefit of always knowing the current package, which eliminates the need to rebuild that knowledge from the sequence of text output. My main point is that go tool test2json is a relatively bad approximation of what go test -json provides (again, not saying the latter is great :)).

Also, arguably the format can remain more stable over time, and is probably less likely to exhibit pathological cases. And since the generation takes place in the toolchain, a json parser should accrue version-specific hacks at a slower rate (hopefully?)

sigma commented 6 years ago

I added a pull request to introduce that feature. It's a larger change than I would have liked, but if there's an interest I'm happy to iterate on it and make it more palatable :)

jstemmer commented 6 years ago

I've looked at your proposed pull request and appreciate the work you put into it. However, at this point I'm not going to merge it. It's a lot of extra code (which also needs to be maintained going forward) for very little benefit.

The issue you linked is an interesting example, as I encountered a panic (because the current package was nil) when running the proposed code against some weird edge cases I've collected over time. And even though this can be fixed, it requires more code to handle switching back and forth between the text/json parsers.

As I understand it, while the lockedStdout prevents output from interleaving (so you don't get broken json), it doesn't prevent output from one test to be attributed to another test if they run in parallel. For example, running these tests in parallel:

func TestA(t *testing.T) {
   fmt.Printf("A")
}
func TestB(t *testing.T) {
   fmt.Printf("B")
}

can produce the following JSON (note that TestA contains output that belongs to TestB):

{"Time":"2018-04-05T23:16:17.933540135+01:00","Action":"output","Package":"pkg","Test":"TestA","Output":"B\n"}

And while it can be argued that you shouldn't rely on fmt.Printf in tests, having the json output actually doesn't really improve things that much in this area.

While I agree that the format will probably be more stable over time, we'd still have to keep track of changes in output since I'd like to keep supporting older Go versions for now.

In the end I do think having the Go tool output as much as possible in an easy to parse format has potential, but I'd prefer to wait and see what future versions will bring before seriously considering adding json support.

Xennis commented 5 years ago

@sigma @jstemmer Maybe it's time to support the JSON output now by integrating #69? Newer Go versions output the test results without tabs and just use spaces, like

=== RUN   TestRound
=== RUN   TestRound/empty
=== RUN   TestRound/round_success
--- FAIL: TestRound (0.00s)
    --- PASS: TestRound/empty (0.00s)
    --- FAIL: TestRound/round_success (0.00s)
        round_test.go:51: got Round()
            0
                want
            100

Since the test output contains no tabs anymore, the tool fails to detect the output correctly. It is going to be harder and harder to parse the output with the current attempt of regex (https://github.com/jstemmer/go-junit-report/blob/master/parser/parser.go#L66 ). And if the tool does not can correctly parse the output correctly anymore, it's maybe a better idea for people to switch to tools like https://github.com/gotestyourself/gotestsum .

alexec commented 4 years ago

SonarCloud only support JSON output. go-junit-report only support text output. It is not possible to use them together without running your suite twice.