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 IgnoreNonJSONOutputLines, --ignore-non-json-output-lines #194

Merged
merged 4 commits into from May 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions README.md
Expand Up @@ -220,11 +220,17 @@ gotestsum -- -coverprofile=cover.out ./...
gotestsum --raw-command -- ./scripts/run_tests.sh
```

Note: when using `--raw-command` you must ensure that the stdout produced by
the script only contains the `test2json` output. Any stderr produced by the script
will be considered an error (this behaviour is necessary because package build errors
are only reported by writting to stderr, not the `test2json` stdout). Any stderr
produced by tests is not considered an error (it will be in the `test2json` stdout).
Note: when using `--raw-command`, the script must follow a few rules about
stdout and stderr output:

* The stdout produced by the script must only contain the `test2json` output, or
`gotestsum` will fail. If it isn't possible to change the script to avoid
non-JSON output, you can use `--ignore-non-json-output-lines` to ignore
non-JSON lines and write them to `gotestsum`'s stderr instead.
* Any stderr produced by the script will be considered an error (this behaviour
is necessary because package build errors are only reported by writting to
stderr, not the `test2json` stdout). Any stderr produced by tests is not
considered an error (it will be in the `test2json` stdout).

**Example: using `TEST_DIRECTORY`**
```
Expand Down
13 changes: 9 additions & 4 deletions cmd/main.go
Expand Up @@ -59,6 +59,9 @@ func setupFlags(name string) (*pflag.FlagSet, *options) {
"print format of test input")
flags.BoolVar(&opts.rawCommand, "raw-command", false,
"don't prepend 'go test -json' to the 'go test' command")
flags.BoolVar(&opts.ignoreNonJSONOutputLines, "ignore-non-json-output-lines", false,
"write non-JSON 'go test' output lines to stderr instead of failing")
Comment on lines +62 to +63
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we should hide this flag (similar to no-summary below). My thinking is that this flag should only be necessary in very rare cases (possibly only when running tests for the Go project itself), and by hiding it we prevent misuse of the flag in cases where there are better options. It would also remove a potential distraction when vieweing the --help output.

We can still include it in the README in the Custom go test command section. There's already a note there about the previous limitation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hid (and updated the test baseline).

When I tried to put this in the readme, it seemed like that paragraph was getting long and the difference between the script's stderr vs. gotestsum's stderr seemed muddy. So, I split the existing bit into two bullet points to try to make it a bit clearer what the new sentence is talking about. Otherwise kept it mostly the same:

Note: when using --raw-command, the script must follow a few rules about stdout and stderr output:

  • The stdout produced by the script must only contain the test2json output, or gotestsum will fail. If it isn't possible to change the script to avoid non-JSON output, you can use --ignore-non-json-output-lines to ignore
    non-JSON lines and write them to gotestsum's stderr instead.
  • Any stderr produced by the script will be considered an error (this behaviour is necessary because package build errors are only reported by writting to stderr, not the test2json stdout). Any stderr produced by tests is not considered an error (it will be in the test2json stdout).

flags.Lookup("ignore-non-json-output-lines").Hidden = true
flags.StringVar(&opts.jsonFile, "jsonfile",
lookEnvWithDefault("GOTESTSUM_JSONFILE", ""),
"write all TestEvents to file")
Expand Down Expand Up @@ -138,6 +141,7 @@ type options struct {
format string
debug bool
rawCommand bool
ignoreNonJSONOutputLines bool
jsonFile string
junitFile string
postRunHookCmd *commandValue
Expand Down Expand Up @@ -194,10 +198,11 @@ func run(opts *options) error {
}
defer handler.Close() // nolint: errcheck
cfg := testjson.ScanConfig{
Stdout: goTestProc.stdout,
Stderr: goTestProc.stderr,
Handler: handler,
Stop: cancel,
Stdout: goTestProc.stdout,
Stderr: goTestProc.stderr,
Handler: handler,
Stop: cancel,
IgnoreNonJSONOutputLines: opts.ignoreNonJSONOutputLines,
}
exec, err := testjson.ScanTestOutput(cfg)
if err != nil {
Expand Down
8 changes: 8 additions & 0 deletions testjson/execution.go
Expand Up @@ -590,6 +590,9 @@ type ScanConfig struct {
Execution *Execution
// Stop is called when ScanTestOutput fails during a scan.
Stop func()
// IgnoreNonJSONOutputLines causes ScanTestOutput to ignore non-JSON lines received from
// the Stdout reader. Instead of causing an error, the lines will be sent to Handler.Err.
IgnoreNonJSONOutputLines bool
}

// EventHandler is called by ScanTestOutput for each event and write to stderr.
Expand Down Expand Up @@ -662,6 +665,11 @@ func readStdout(config ScanConfig, execution *Execution) error {
config.Handler.Err(errBadEvent.Error() + ": " + scanner.Text())
continue
case err != nil:
if config.IgnoreNonJSONOutputLines {
// nolint: errcheck
config.Handler.Err(string(raw))
continue
}
return errors.Wrapf(err, "failed to parse test output: %s", string(raw))
}

Expand Down
30 changes: 30 additions & 0 deletions testjson/execution_test.go
Expand Up @@ -205,8 +205,37 @@ func TestScanOutput_WithMissingEvents(t *testing.T) {
assert.DeepEqual(t, expected, handler.events[start:], cmpTestEventShallow)
}

func TestScanOutput_WithNonJSONLines(t *testing.T) {
source := golden.Get(t, "go-test-json-with-nonjson-stdout.out")
nonJSONLine := "|||This line is not valid test2json output.|||"

// Test that when we ignore non-JSON lines, scanning completes, and test
// that when we don't ignore non-JSON lines, scanning fails.
for _, ignore := range []bool{true, false} {
t.Run(fmt.Sprintf("ignore-non-json=%v", ignore), func(t *testing.T) {
handler := &captureHandler{}
cfg := ScanConfig{
Stdout: bytes.NewReader(source),
Handler: handler,
IgnoreNonJSONOutputLines: ignore,
}
_, err := ScanTestOutput(cfg)
if ignore {
assert.DeepEqual(t, handler.errs, []string{nonJSONLine})
assert.NilError(t, err)
return
}
assert.DeepEqual(t, handler.errs, []string{}, cmpopts.EquateEmpty())
expected := "failed to parse test output: " +
nonJSONLine + ": invalid character '|' looking for beginning of value"
assert.Error(t, err, expected)
})
}
}

type captureHandler struct {
events []TestEvent
errs []string
}

func (s *captureHandler) Event(event TestEvent, _ *Execution) error {
Expand All @@ -215,5 +244,6 @@ func (s *captureHandler) Event(event TestEvent, _ *Execution) error {
}

func (s *captureHandler) Err(text string) error {
s.errs = append(s.errs, text)
return fmt.Errorf(text)
}
8 changes: 8 additions & 0 deletions testjson/testdata/go-test-json-with-nonjson-stdout.out
@@ -0,0 +1,8 @@
{"Time":"2021-05-12T13:53:07.462687619-05:00","Action":"run","Package":"gotest.tools/gotestsum/testjson/internal/good","Test":"TestPassed"}
|||This line is not valid test2json output.|||
{"Time":"2021-05-12T13:53:07.46279664-05:00","Action":"output","Package":"gotest.tools/gotestsum/testjson/internal/good","Test":"TestPassed","Output":"=== RUN TestPassed\n"}
{"Time":"2021-05-12T13:53:07.462812837-05:00","Action":"output","Package":"gotest.tools/gotestsum/testjson/internal/good","Test":"TestPassed","Output":"--- PASS: TestPassed (0.00s)\n"}
{"Time":"2021-05-12T13:53:07.462819251-05:00","Action":"pass","Package":"gotest.tools/gotestsum/testjson/internal/good","Test":"TestPassed","Elapsed":0}
{"Time":"2021-05-12T13:53:07.462825108-05:00","Action":"output","Package":"gotest.tools/gotestsum/testjson/internal/good","Output":"PASS\n"}
{"Time":"2021-05-12T13:53:07.462848483-05:00","Action":"output","Package":"gotest.tools/gotestsum/testjson/internal/good","Output":"ok \tgotest.tools/gotestsum/testjson/internal/good\t0.001s\n"}
{"Time":"2021-05-12T13:53:07.46309146-05:00","Action":"pass","Package":"gotest.tools/gotestsum/testjson/internal/good","Elapsed":0.001}