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.

Sign up for GitHub

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

Jump to bottom

Add IgnoreNonJSONOutputLines, --ignore-non-json-output-lines #194

Merged
merged 4 commits into from May 15, 2021
Merged

Add IgnoreNonJSONOutputLines, --ignore-non-json-output-lines #194

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cee4211
Add --ignore-non-json-output-lines option
dagood May 10, 2021
5707a34
Apply suggestions from code review
dagood May 12, 2021
226f339
Apply suggestions; add test case for non-JSON line
dagood May 12, 2021
4710cc6
small improvement to TestScanOutput_WithNonJSONLines
dnephin May 15, 2021
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
12 changes: 8 additions & 4 deletions cmd/main.go
View file
Open in desktop
Expand Up @@ -59,6 +59,8 @@ 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.StringVar(&opts.jsonFile, "jsonfile",
lookEnvWithDefault("GOTESTSUM_JSONFILE", ""),
"write all TestEvents to file")
Expand Down Expand Up @@ -138,6 +140,7 @@ type options struct {
format string
debug bool
rawCommand bool
ignoreNonJSONOutputLines bool
jsonFile string
junitFile string
postRunHookCmd *commandValue
Expand Down Expand Up @@ -194,10 +197,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
1 change: 1 addition & 0 deletions cmd/testdata/gotestsum-help-text
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ Flags:
--debug enabled debug logging
-f, --format string print format of test input (default "short")
--hide-summary summary hide sections of the summary: skipped,failed,errors,output (default none)
--ignore-non-json-output-lines write non-JSON 'go test' output lines to stderr instead of failing
--jsonfile string write all TestEvents to file
--junitfile string write a JUnit XML file
--junitfile-testcase-classname field-format format the testcase classname field as: full, relative, short (default full)
Expand Down
8 changes: 8 additions & 0 deletions testjson/execution.go
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"io"
"os"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -590,6 +591,9 @@ type ScanConfig struct {
Execution *Execution
// Stop is called when ScanTestOutput fails during a scan.
Stop func()
// IgnoreNonJSONOutputLines causes ScanTestOutput to print non-JSON lines to
// stderr rather than returning an error.
dagood marked this conversation as resolved.
Show resolved Hide resolved
IgnoreNonJSONOutputLines bool
}

// EventHandler is called by ScanTestOutput for each event and write to stderr.
Expand Down Expand Up @@ -662,6 +666,10 @@ func readStdout(config ScanConfig, execution *Execution) error {
config.Handler.Err(errBadEvent.Error() + ": " + scanner.Text())
continue
case err != nil:
if config.IgnoreNonJSONOutputLines {
fmt.Fprintf(os.Stderr, "%s\n", raw)
dagood marked this conversation as resolved.
Show resolved Hide resolved
continue
}
return errors.Wrapf(err, "failed to parse test output: %s", string(raw))
}

Expand Down