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

Add API to convert Execution to JUnit XML when using gotestsum as a library #193

Closed
dagood opened this issue May 6, 2021 · 5 comments
Closed
Labels
enhancement New feature or request

Comments

@dagood
Copy link
Contributor

dagood commented May 6, 2021

This is along the lines of #19: I'm trying to convert some go test -json output mixed with non-JSON lines into a format my CI server understands, and JUnit XML would work great. gotestsum caught my eye--I don't see anything else that looks actively maintained and does this conversion.

I'm able to get gotestsum to parse a filtered stream like this (v1.6.4):

formatter := testjson.NewEventFormatter(os.Stdout, "standard-quiet")

execution, err := testjson.ScanTestOutput(testjson.ScanConfig{
	Stdout: filteredReader,
	Handler: &conversionEventHandler{
		formatter: formatter,
	},
})

But I don't see how to turn that into JUnit XML with any existing API. This is what I think I should call next:

func Write(out io.Writer, exec *testjson.Execution, cfg Config) error {

That works, but since it's internal, I had to copy the source code to my project or use module tricks to wrap the internal API in one that I can access. Having an exported function would help. (Or, internal/junitxml could just be exposed and I could feed in the args myself.)


More background on why I ended up at this point:

It works fine to filter the test output, save the JSON to a file, and use gotestsum --junitxml testresults.xml --raw-command cat testoutput.json (#19 (comment)), but:

  • When I post-process the file, gotestsum writes out a bunch of output I don't need. It takes a while to print and makes my build log cluttered.
    • I can disable the summary with --hide-summary all, but I don't see a way to disable the per-test/package lines.
    • I could pipe it to null or save the output to a file to print if the command exits non-zero. I'm worried I might miss an error/warning message this way.
  • I would like to use gotestsum's nice output formatting, but I don't see a way to get the command line tool to accept a stream (stdin) rather than a file.
    • If I try cat /dev/stdin, it terminates immediately. It's possible I'm doing something wrong, though. This has worked for me in the past for a different tool and I'm not sure what's different here. 😄

My first thought was to try to get my tests to only emit JSON output, so I can plug my test command directly into --raw-command. I think this is what gotestsum is meant for. But, this seems complicated to do for my project in particular. (I'm building Go, and the project's test runner (dist) has a lot of stuff going on I don't fully understand yet. I tried to get rid of non-JSON stdout, but it was very hard to know if I was covering all cases.)

My second thought (where I am now) is to use gotestsum as a module dependency and use the API directly, so I can get a stream to work.

I'm mentioning all of this in case I went wrong somewhere, corrections welcome. 🙂 I'm relatively new to Go.

@dnephin dnephin added the enhancement New feature or request label May 8, 2021
@dnephin
Copy link
Member

dnephin commented May 8, 2021

Thank you for your interest in gotestsum! I think there are a few options for getting this working.

When I post-process the file, gotestsum writes out a bunch of output I don't need. It takes a while to print and makes my build log cluttered.

I've been considering adding a new none format that does not print any of the per-test/package lines. I think that would allow you to hide all of this output with --format none, without the risk of hiding any potential error messages or build errors.

I would like to use gotestsum's nice output formatting, but I don't see a way to get the command line tool to accept a stream (stdin) rather than a file.

I suspect the reason this isn't working is because in startGoTest, cmd.Stdin is not being set to os.Stdin and it defaults to /dev/null. I think it might be fine to connect up stdin, although I'm not sure how that will work with the --watch mode (which accepts key presses to re-run tests).

Another option is to use a named pipe. This seems to work for me, but is not something I've used very often.

mkfifo /tmp/testpipe
go test -json ./... > /tmp/testpipe | gotestsum --raw-command cat /tmp/testpipe

My first thought was to try to get my tests to only emit JSON output, so I can plug my test command directly into --raw-command. I think this is what gotestsum is meant for.

Yup, exactly! I did not want to simply read from stdin (as other tools do) because when a package fails to build, that output goes to stderr, and I think it's important to capture that as part of the "output". Piping would either merge stderr with stdout making it harder to parse properly, or would miss the stderr entirely. So gotestsum instead runs the go test command so it can capture both. Also pipes can hide the non-zero exit code if -o pipefail is not used in bash shells, which makes it a bit harder to use correctly in CI.

I'm building Go, and the project's test runner (dist) has a lot of stuff going on I don't fully understand yet. I tried to get rid of non-JSON stdout, but it was very hard to know if I was covering all cases.)

The few times I've run the tests for the Go project I've been able to first build it with Make.dist (or I guess go tool dist works as well), then I run the tests with gotestsum by setting these two env vars so that when gotestsum execs go it runs the version that was just built:

# $GOCHECKOUT is the path to the root of the golang/go git repo
export PATH="$GOCHECKOUT/bin:$PATH"
export GOROOT="$GOCHECKOUT"

That might allow you to run the tests with gotestsum directly without having to change any scripts.

I'm also open to the idea of allowing gotestsum to ignore "invalid JSON" lines, and just echo them to stderr, possibly with a new command line flag instead of by default. That seems like it might help? Something like --ignore-non-json-output-lines, and would be implemented here.

My second thought (where I am now) is to use gotestsum as a module dependency and use the API directly, so I can get a stream to work.

I'm not against that approach, but I would prefer to keep the exported API small. I'm particularly hesitant to expose the Junit XML code because it's not really standardized and it might need to change at some point to make it work better with other CI systems.

I hope this helps! If for some reason none of these options work out and we can't get the gotestsum command to do what you want, we can explore exporting a single function to write a JUnit XML file, but I am hopeful we will be able to get the command working.

@dagood
Copy link
Contributor Author

dagood commented May 10, 2021

I'm also open to the idea of allowing gotestsum to ignore "invalid JSON" lines, and just echo them to stderr, possibly with a new command line flag instead of by default. That seems like it might help? Something like --ignore-non-json-output-lines, and would be implemented here.

This would be perfect! My current filter just attempts to parse each incoming line as a test2json line and sends it to gotestsum if so, and stdout if not. (Basically duplicating the check gotestsum is doing.)

I tried writing this up locally, and it seems pretty simple to implement--I'll send a quick PR shortly to go further down this road and see if it'll work.

I've been considering adding a new none format that does not print any of the per-test/package lines. I think that would allow you to hide all of this output with --format none, without the risk of hiding any potential error messages or build errors.

That would certainly let me filter/save JSON output to a file and send it though later without clutter. (Although I would prefer to show the nice gotestsum formatted output rather than having to parse the JSON and make something up myself, and it would be nice to not have the extra buffer file hanging around.)

Another option is to use a named pipe.

I'd kind of expect this to work the same way as reading from /dev/stdin, but I haven't used named pipes before. 😄 This could work. My concern would be making it work on both Linux and Windows, on my end. (The first thing I see is some naming convention strictness on Windows--but for all I know, it's smooth sailing after taking care of that.)

I did not want to simply read from stdin (as other tools do) because [...]

Yeah, these reasons makes sense to me (as well as --watch). From a new-to-Go perspective it seems odd that tests commonly write stuff to stdout and stderr that you have to collect and associate manually... I would have expected the testing API to have a way to incorporate that nicely into the test output (and test2json). I guess it's not something that can be dealt with here, anyway.

The few times I've run the tests for the Go project I've been able to first build it with Make.dist (or I guess go tool dist works as well), then I run the tests with gotestsum by setting these two env vars

I actually use make.bash, (wasn't familiar with using dist to build,) then use go tool dist test to run the tests. dist test runs a number of tests that don't use go test at all, and some that use go test in a way that doesn't support JSON output, and it reports all of this to stdout/stderr. I think if you just run go test -json (I believe what gotestsum would do), you only get the library tests, not the deeper ones.

I'm not against that approach, but I would prefer to keep the exported API small. I'm particularly hesitant to expose the Junit XML code because it's not really standardized and it might need to change at some point to make it work better with other CI systems.

Very reasonable. FWIW, I got around this by creating a local module gotest.tools/gotestsum/expose that exports one func:

func Write(out io.Writer, exec *testjson.Execution) error {
	return junitxml.Write(out, exec, junitxml.Config{})
}

I think it would be fine for me to keep using that on my side so there's no pressure to add this export to gotestsum directly. Then, once --ignore-non-json-output-lines exists, I can probably delete the module and use that. That way, this one exported API will never end up with people depending on it who expect it to continue to work the same way in future releases.

@dnephin
Copy link
Member

dnephin commented May 11, 2021

From a new-to-Go perspective it seems odd that tests commonly write stuff to stdout and stderr that you have to collect and associate manually... I would have expected the testing API to have a way to incorporate that nicely into the test output (and test2json)

Ah, the go test command does do this pretty well. It will incorporate all of the stdout/stderr into the test2json output. It sounds like the problem is that go tool dist test does some special things that aren't captured. So it seems like only a problem with testing Go itself (not projects using Go).

@dagood
Copy link
Contributor Author

dagood commented May 11, 2021

Sorry, I wasn't too clear about what I was thinking about when I wrote that--it was the idea that the stdout/stderr streams need to be kept separate and tracked for build errors:

I did not want to simply read from stdin (as other tools do) because when a package fails to build, that output goes to stderr, and I think it's important to capture that as part of the "output". Piping would either merge stderr with stdout making it harder to parse properly, or would miss the stderr entirely.

I didn't think about some building happening during go test, and the build output going straight to stderr rather than being treated as part of the go test output. So I ended up conflating build output with test output. This is making more sense to me now. 😄

It sounds like the problem is that go tool dist test does some special things that aren't captured. So it seems like only a problem with testing Go itself (not projects using Go).

Agreed. I think technically any Go project could create a testing tool that mixes in JSON with other output, but I don't expect many projects would want to do that. Probably only needed if there are extraordinary requirements (like Go itself has).

Something like --ignore-non-json-output-lines, and would be implemented here.

In case you didn't see, I submitted a quick PR for this: #194. 🙂

@dagood
Copy link
Contributor Author

dagood commented May 17, 2021

Something like --ignore-non-json-output-lines

This is now merged with #194. Thanks for the quick review+feedback+fixup+merge! 😄 Closing.

@dagood dagood closed this as completed May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants