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 dual-output for compatibility with existing tools #4

Closed
ascandella opened this issue Jan 22, 2017 · 5 comments
Closed

Add dual-output for compatibility with existing tools #4

ascandella opened this issue Jan 22, 2017 · 5 comments

Comments

@ascandella
Copy link

ascandella commented Jan 22, 2017

Many tools currently parse the output of go test, which means we can't just drop this helpful tool into a build pipeline without breaking things.

Of note, there's a proposal in upstream Golang to support a standard "go test" output (golang/go#2981) which would help this and many other tools. However this is just a "proposal" and there's no concrete timeline for implementing.

In the meantime, the output of "go test" is the canonical source-of-truth for test runs, and many tools (including go-junit-report(https://github.com/jstemmer/go-junit-report) which I use and contribute to expect the tests runs to output their results in a specific, human-friendly format.

In order to differentiate between output made for humans (which richgo does a great job at doing) and output for machines (which go test currently does, although it's currently both for-machine and for-human, and it isn't very great for humans), I'd like to augment richgo to support outputting both: a human-friendly mode, and a machine-friendly mode.

In the short term, the machine-friendly mode will simply be the input we get from running "go test" unmunged. In the future, it will probably be go test -json or whatever the upstream proposal lands on, which will make everybody's job easier.

The primary changes proposed are to implement two flags (and optionally environment variables):

  • the output path to send the human-friendly version to (default: os.Stdout)
  • the output path to send the machine-friendly version to (currently ioutil.Discard)

note that rather than hard-coding paths for defaults, will need to do some portability things to make sure we don't hardcode this as /dev/stdout and instead use the more portable os.Stdout and ioutil.Discard istead of /dev/null -- none of this will be difficult but writing it down here so I don't forget it.

Then somebody could run this tool using linux FIFOS (totally untested):

tmpdir=$(mkdtemp -d)
outfifo="${tmpdir}/MYFIFO
mkfifo a=rw "${outfifo}"
go-junit-report < ${outfifo} > junit.xml
richgo test --machine-friendly-output "${outfifo}"

And on their console they would have the default, nice, colorized output on os.Stdout, but go-junit-report would be parsing the "machine-friendly" version.

References #3

@ascandella
Copy link
Author

On second thought, this may be entirely unnecessary. I think I could use an appropriately-placed tee to split off the output before invoking richgo, which would allow go-junit-report to see the untouched version while sending richgo's output to stdout. Will do some more investigating.

@kyoh86
Copy link
Owner

kyoh86 commented Jan 23, 2017

I'm going to use Rich-Go with an alias go=richgo, so I don't want to prepare flags only for this app.

How about a following plan instead?

  • Create a new sub-command, richgo testfilter.
  • testfilter reads text lines from Stdin, decorates them (as go test output) and write out them.

Then you can use it like this:

# (with process substitution)
go test ./... | tee >(richgo testfilter) | go-junit-report`

# (with FIFO)
tmpdir=$(mkdtemp -d)
outfifo="${tmpdir}/MYFIFO
mkfifo a=rw "${outfifo}"
go-junit-report < ${outfifo} > junit.xml
go test ./... | tee ${outfifo} | richgo testfilter

@ascandella
Copy link
Author

Sounds good, and much simpler, I like it :)

@jspiro
Copy link

jspiro commented Feb 3, 2017

Why not just fix richgo ./... | tee /dev/tty | ... outputting plain ascii and not colored ascii? That seems to be the only problem here.

@ascandella
Copy link
Author

ascandella commented Feb 3, 2017 via email

@kyoh86 kyoh86 closed this as completed in #6 Feb 4, 2017
kyoh86 pushed a commit that referenced this issue Feb 4, 2017
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

3 participants