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

Pass stdin to go test #234

Merged
merged 2 commits into from
Apr 9, 2022
Merged

Pass stdin to go test #234

merged 2 commits into from
Apr 9, 2022

Conversation

uhthomas
Copy link
Contributor

@uhthomas uhthomas commented Feb 22, 2022

It may be useful to pipe the go test JSON to gotestsum. There are a few ways in which this could be achieved:

  1. Passing stdin to testjson.ScanConfig seems the most sensible, but it may be complicated and may cause unintended side effects. For instance, assuming gotestsum were to contextually switch between using stdin and go test, existing invocations of gotestsum where data is being piped would no longer work. Providing a new flag to opt-in may be helpful? Maybe a sub-command?

  2. (This PR) Passing stdin to --raw-command allows stdin to be piped to stdout via use of a tool like cat.


Given the changes included in this PR, gotestsum can read go test JSON via stdin with:

cat out.json | gotestsum --raw-command -- cat

This may be useful when the JSON is generated via other tools (i.e Bazel). For example:

bazel test //... |& go tool test2json | gotestsum --raw-command -- cat
# or
bazel test //... |& gotestsum --raw-command -- go tool test2json

What do you think?

Copy link
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! I think it would be good to allow some way to read from stdin.

I thought this had come up before, but I was not able to find the discussion, so I'll need to think some more about any potential complications. One thing that comes to mind is that --watch changes the terminal mode to accept keystrokes. I'm not sure if that will cause any problems with passing os.Stdin to the command being run. You can see that in internal/filewatcher/term_unix.go.

cmd/main_test.go Outdated
{Action: "fail", Package: "pkg", Test: "TestOne"},
{Action: "fail", Package: "pkg"},
} {
assert.NilError(t, e.Encode(event))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think t.Fatal (and anything that results in t.FailNow) is only supposed to be called from the main test goroutine. Here assert.NilError will call t.FailNow. One way to work around this is to use t.Error (t.Fail) instead, which can be done with assert.Check(t, e.Encode(event)).

@dnephin
Copy link
Member

dnephin commented Apr 9, 2022

Thank you again for the PR! I had a chance to test it out and everything seems to work as expected. I made a couple small changes to the test to ensure that it would fail when the code change it is testing is removed. Also added another example to the docs to show how it works. If CI is happy I will merge.

The test would not fail without the change because it was not making any
assertions about the input. Adding a check for "DONE 1" causes it to
fail properly when the support is removed.

Also add an example to the docs of how to use stdin.
@dnephin dnephin merged commit cc0c1a1 into gotestyourself:main Apr 9, 2022
@uhthomas
Copy link
Contributor Author

Thanks! Happy to help. 🙂

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

Successfully merging this pull request may close these issues.

2 participants