diff --git a/flags.go b/flags.go index c77920f8..29afb5ce 100644 --- a/flags.go +++ b/flags.go @@ -7,6 +7,7 @@ import ( "github.com/google/shlex" "github.com/pkg/errors" + "github.com/spf13/pflag" "gotest.tools/gotestsum/internal/junitxml" "gotest.tools/gotestsum/testjson" ) @@ -112,3 +113,22 @@ func (c *commandValue) Value() []string { } return c.command } + +var _ pflag.Value = (*stringSlice)(nil) + +// stringSlice is a flag.Value which populates the string slice by splitting +// the raw flag value on spaces. +type stringSlice []string + +func (s *stringSlice) String() string { + return strings.Join(*s, " ") +} + +func (s *stringSlice) Set(raw string) error { + *s = append(*s, strings.Split(raw, " ")...) + return nil +} + +func (s *stringSlice) Type() string { + return "list" +} diff --git a/main.go b/main.go index b180b5ce..36c69e1f 100644 --- a/main.go +++ b/main.go @@ -93,7 +93,6 @@ Formats: standard-verbose standard go test -v format `) } - flags.BoolVar(&opts.debug, "debug", false, "enabled debug") flags.StringVarP(&opts.format, "format", "f", lookEnvWithDefault("GOTESTSUM_FORMAT", "short"), "print format of test input") @@ -102,18 +101,28 @@ Formats: flags.StringVar(&opts.jsonFile, "jsonfile", lookEnvWithDefault("GOTESTSUM_JSONFILE", ""), "write all TestEvents to file") - flags.StringVar(&opts.junitFile, "junitfile", - lookEnvWithDefault("GOTESTSUM_JUNITFILE", ""), - "write a JUnit XML file") flags.BoolVar(&opts.noColor, "no-color", color.NoColor, "disable color output") flags.Var(opts.noSummary, "no-summary", "do not print summary of: "+testjson.SummarizeAll.String()) + flags.Var(opts.postRunHookCmd, "post-run-command", + "command to run after the tests have completed") + + flags.StringVar(&opts.junitFile, "junitfile", + lookEnvWithDefault("GOTESTSUM_JUNITFILE", ""), + "write a JUnit XML file") flags.Var(opts.junitTestSuiteNameFormat, "junitfile-testsuite-name", "format the testsuite name field as: "+junitFieldFormatValues) flags.Var(opts.junitTestCaseClassnameFormat, "junitfile-testcase-classname", "format the testcase classname field as: "+junitFieldFormatValues) - flags.Var(opts.postRunHookCmd, "post-run-command", - "command to run after the tests have completed") + + flags.IntVar(&opts.rerunFailsMaxAttempts, "rerun-fails-max-attempts", 0, + "rerun failed tests until each one passes once, or attempts exceeds max") + flags.IntVar(&opts.rerunFailsMaxInitialFailures, "rerun-fails-max-failures", 10, + "do not rerun any tests if the initial run has more than this number of failures") + flags.Var((*stringSlice)(&opts.packages), "packages", + "space separated list of package to test") + + flags.BoolVar(&opts.debug, "debug", false, "enabled debug logging") flags.BoolVar(&opts.version, "version", false, "show version and exit") return flags, opts } @@ -137,6 +146,9 @@ type options struct { noSummary *noSummaryValue junitTestSuiteNameFormat *junitFieldFormatValue junitTestCaseClassnameFormat *junitFieldFormatValue + rerunFailsMaxAttempts int + rerunFailsMaxInitialFailures int + packages []string version bool // shims for testing @@ -144,6 +156,15 @@ type options struct { stderr io.Writer } +func (o options) Validate() error { + if o.rerunFailsMaxAttempts > 0 && len(o.args) > 0 && !o.rawCommand && len(o.packages) == 0 { + return fmt.Errorf( + "when go test args are used with --rerun-fails-max-attempts " + + "the list of packages to test must be specified by the --packages flag") + } + return nil +} + func setupLogging(opts *options) { if opts.debug { log.SetLevel(log.DebugLevel) @@ -152,28 +173,38 @@ func setupLogging(opts *options) { } func run(opts *options) error { - ctx := context.Background() - goTestProc, err := startGoTest(ctx, goTestCmdArgs(opts)) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + if err := opts.Validate(); err != nil { + return err + } + + goTestProc, err := startGoTest(ctx, goTestCmdArgs(opts, rerunOpts{})) if err != nil { - return errors.Wrapf(err, "failed to run %s %s", - goTestProc.cmd.Path, - strings.Join(goTestProc.cmd.Args, " ")) + return errors.Wrapf(err, "failed to run %s", strings.Join(goTestProc.cmd.Args, " ")) } - defer goTestProc.cancel() handler, err := newEventHandler(opts) if err != nil { return err } defer handler.Close() // nolint: errcheck - exec, err := testjson.ScanTestOutput(testjson.ScanConfig{ + cfg := testjson.ScanConfig{ Stdout: goTestProc.stdout, Stderr: goTestProc.stderr, Handler: handler, - }) + } + exec, err := testjson.ScanTestOutput(cfg) if err != nil { return err } + goTestExitErr := goTestProc.cmd.Wait() + if opts.rerunFailsMaxAttempts > 0 { + cfg := testjson.ScanConfig{Execution: exec, Handler: handler} + goTestExitErr = rerunFailed(ctx, opts, cfg) + } + testjson.PrintSummary(opts.stdout, exec, opts.noSummary.value) if err := writeJUnitFile(opts, exec); err != nil { return err @@ -181,44 +212,100 @@ func run(opts *options) error { if err := postRunHook(opts, exec); err != nil { return err } - return goTestProc.cmd.Wait() + return goTestExitErr } -func goTestCmdArgs(opts *options) []string { +func goTestCmdArgs(opts *options, rerunOpts rerunOpts) []string { + if opts.rawCommand { + var result []string + result = append(result, opts.args...) + result = append(result, rerunOpts.Args()...) + return result + } + args := opts.args - defaultArgs := []string{"go", "test"} - switch { - case opts.rawCommand: - return args - case len(args) == 0: - return append(defaultArgs, "-json", pathFromEnv("./...")) - case !hasJSONArg(args): - defaultArgs = append(defaultArgs, "-json") + result := []string{"go", "test"} + + if len(args) == 0 { + result = append(result, "-json") + if rerunOpts.runFlag != "" { + result = append(result, rerunOpts.runFlag) + } + return append(result, cmdArgPackageList(opts, rerunOpts, "./...")...) + } + + if boolArgIndex("json", args) < 0 { + result = append(result, "-json") + } + + if rerunOpts.runFlag != "" { + // Remove any existing run arg, it needs to be replaced with our new one + // and duplicate args are not allowed by 'go test'. + runIndex, runIndexEnd := argIndex("run", args) + if runIndex >= 0 && runIndexEnd < len(args) { + args = append(args[:runIndex], args[runIndexEnd+1:]...) + } + result = append(result, rerunOpts.runFlag) } - if testPath := pathFromEnv(""); testPath != "" { - args = append(args, testPath) + + pkgArgIndex := findPkgArgPosition(args) + result = append(result, args[:pkgArgIndex]...) + result = append(result, cmdArgPackageList(opts, rerunOpts)...) + result = append(result, args[pkgArgIndex:]...) + return result +} + +func cmdArgPackageList(opts *options, rerunOpts rerunOpts, defPkgList ...string) []string { + switch { + case rerunOpts.pkg != "": + return []string{rerunOpts.pkg} + case len(opts.packages) > 0: + return opts.packages + case os.Getenv("TEST_DIRECTORY") != "": + return []string{os.Getenv("TEST_DIRECTORY")} + default: + return defPkgList } - return append(defaultArgs, args...) } -func pathFromEnv(defaultPath string) string { - return lookEnvWithDefault("TEST_DIRECTORY", defaultPath) +func boolArgIndex(flag string, args []string) int { + for i, arg := range args { + if arg == "-"+flag || arg == "--"+flag { + return i + } + } + return -1 } -func hasJSONArg(args []string) bool { - for _, arg := range args { - if arg == "-json" || arg == "--json" { - return true +func argIndex(flag string, args []string) (start, end int) { + for i, arg := range args { + if arg == "-"+flag || arg == "--"+flag { + return i, i + 1 + } + if strings.HasPrefix(arg, "-"+flag+"=") || strings.HasPrefix(arg, "--"+flag+"=") { + return i, i } } - return false + return -1, -1 +} + +// The package list is before the -args flag, or at the end of the args list +// if the -args flag is not in args. +// The -args flag is a 'go test' flag that indicates that all subsequent +// args should be passed to the test binary. It requires that the list of +// packages comes before -args, so we re-use it as a placeholder in the case +// where some args must be passed to the test binary. +func findPkgArgPosition(args []string) int { + if i := boolArgIndex("args", args); i >= 0 { + return i + } + return len(args) } type proc struct { cmd *exec.Cmd stdout io.Reader stderr io.Reader - cancel func() } func startGoTest(ctx context.Context, args []string) (proc, error) { @@ -226,10 +313,8 @@ func startGoTest(ctx context.Context, args []string) (proc, error) { return proc{}, errors.New("missing command to run") } - ctx, cancel := context.WithCancel(ctx) p := proc{ - cmd: exec.CommandContext(ctx, args[0], args[1:]...), - cancel: cancel, + cmd: exec.CommandContext(ctx, args[0], args[1:]...), } log.Debugf("exec: %s", p.cmd.Args) var err error diff --git a/main_test.go b/main_test.go new file mode 100644 index 00000000..984c607d --- /dev/null +++ b/main_test.go @@ -0,0 +1,223 @@ +package main + +import ( + "testing" + + "gotest.tools/v3/assert" + "gotest.tools/v3/env" +) + +func TestGoTestCmdArgs(t *testing.T) { + type testCase struct { + opts *options + rerunOpts rerunOpts + env []string + expected []string + } + fn := func(t *testing.T, tc testCase) { + defer env.PatchAll(t, env.ToMap(tc.env))() + actual := goTestCmdArgs(tc.opts, tc.rerunOpts) + assert.DeepEqual(t, actual, tc.expected) + } + var testcases = map[string]testCase{ + "raw command": { + opts: &options{ + rawCommand: true, + args: []string{"./script", "-test.timeout=20m"}, + }, + expected: []string{"./script", "-test.timeout=20m"}, + }, + "no args": { + opts: &options{}, + expected: []string{"go", "test", "-json", "./..."}, + }, + "no args, with rerunPackageList arg": { + opts: &options{ + packages: []string{"./pkg"}, + }, + expected: []string{"go", "test", "-json", "./pkg"}, + }, + "TEST_DIRECTORY env var no args": { + opts: &options{}, + env: []string{"TEST_DIRECTORY=testdir"}, + expected: []string{"go", "test", "-json", "testdir"}, + }, + "TEST_DIRECTORY env var with args": { + opts: &options{ + args: []string{"-tags=integration"}, + }, + env: []string{"TEST_DIRECTORY=testdir"}, + expected: []string{"go", "test", "-json", "-tags=integration", "testdir"}, + }, + "no -json arg": { + opts: &options{ + args: []string{"-timeout=2m", "./pkg"}, + }, + expected: []string{"go", "test", "-json", "-timeout=2m", "./pkg"}, + }, + "with -json arg": { + opts: &options{ + args: []string{"-json", "-timeout=2m", "./pkg"}, + }, + expected: []string{"go", "test", "-json", "-timeout=2m", "./pkg"}, + }, + "raw command, with rerunOpts": { + opts: &options{ + rawCommand: true, + args: []string{"./script", "-test.timeout=20m"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"./script", "-test.timeout=20m", "-run=TestOne|TestTwo", "./fails"}, + }, + "no args, with rerunOpts": { + opts: &options{}, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "./fails"}, + }, + "TEST_DIRECTORY env var, no args, with rerunOpts": { + opts: &options{}, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + env: []string{"TEST_DIRECTORY=testdir"}, + // TEST_DIRECTORY should be overridden by rerun opts + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "./fails"}, + }, + "TEST_DIRECTORY env var, with args, with rerunOpts": { + opts: &options{ + args: []string{"-tags=integration"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + env: []string{"TEST_DIRECTORY=testdir"}, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "-tags=integration", "./fails"}, + }, + "no -json arg, with rerunOpts": { + opts: &options{ + args: []string{"-timeout=2m"}, + packages: []string{"./pkg"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "-timeout=2m", "./fails"}, + }, + "with -json arg, with rerunOpts": { + opts: &options{ + args: []string{"-json", "-timeout=2m"}, + packages: []string{"./pkg"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-run=TestOne|TestTwo", "-json", "-timeout=2m", "./fails"}, + }, + "with args, with reunFailsPackageList args, with rerunOpts": { + opts: &options{ + args: []string{"-timeout=2m"}, + packages: []string{"./pkg1", "./pkg2", "./pkg3"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "-timeout=2m", "./fails"}, + }, + "with args, with reunFailsPackageList": { + opts: &options{ + args: []string{"-timeout=2m"}, + packages: []string{"./pkg1", "./pkg2", "./pkg3"}, + }, + expected: []string{"go", "test", "-json", "-timeout=2m", "./pkg1", "./pkg2", "./pkg3"}, + }, + "reunFailsPackageList args, with rerunOpts ": { + opts: &options{ + packages: []string{"./pkg1", "./pkg2", "./pkg3"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "./fails"}, + }, + "reunFailsPackageList args, with rerunOpts, with -args ": { + opts: &options{ + args: []string{"before", "-args", "after"}, + packages: []string{"./pkg1"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "before", "./fails", "-args", "after"}, + }, + "reunFailsPackageList args, with rerunOpts, with -args at end": { + opts: &options{ + args: []string{"before", "-args"}, + packages: []string{"./pkg1"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "before", "./fails", "-args"}, + }, + "reunFailsPackageList args, with -args at start": { + opts: &options{ + args: []string{"-args", "after"}, + packages: []string{"./pkg1"}, + }, + expected: []string{"go", "test", "-json", "./pkg1", "-args", "after"}, + }, + "-run arg at start, with rerunOpts ": { + opts: &options{ + args: []string{"-run=TestFoo", "-args"}, + packages: []string{"./pkg"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "./fails", "-args"}, + }, + "-run arg in middle, with rerunOpts ": { + opts: &options{ + args: []string{"-count", "1", "--run", "TestFoo", "-args"}, + packages: []string{"./pkg"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "-count", "1", "./fails", "-args"}, + }, + "-run arg at end with missing value, with rerunOpts ": { + opts: &options{ + args: []string{"-count", "1", "-run"}, + packages: []string{"./pkg"}, + }, + rerunOpts: rerunOpts{ + runFlag: "-run=TestOne|TestTwo", + pkg: "./fails", + }, + expected: []string{"go", "test", "-json", "-run=TestOne|TestTwo", "-count", "1", "-run", "./fails"}, + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + fn(t, tc) + }) + } +} diff --git a/rerunfails.go b/rerunfails.go new file mode 100644 index 00000000..c5fdbcd2 --- /dev/null +++ b/rerunfails.go @@ -0,0 +1,113 @@ +package main + +import ( + "context" + "fmt" + "strings" + + "github.com/pkg/errors" + "gotest.tools/gotestsum/testjson" +) + +type rerunOpts struct { + runFlag string + pkg string +} + +func (o rerunOpts) Args() []string { + var result []string + if o.runFlag != "" { + result = append(result, o.runFlag) + } + if o.pkg != "" { + result = append(result, o.pkg) + } + return result +} + +func rerunFailed(ctx context.Context, opts *options, scanConfig testjson.ScanConfig) error { + failed := len(scanConfig.Execution.Failed()) + if failed > opts.rerunFailsMaxInitialFailures { + return fmt.Errorf( + "number of test failures (%d) exceeds maximum (%d) set by --rerun-fails-max-failures", + failed, opts.rerunFailsMaxInitialFailures) + } + + rec := newFailureRecorderFromExecution(scanConfig.Execution) + + var lastErr error + for count := 0; rec.count() > 0 && count < opts.rerunFailsMaxAttempts; count++ { + nextRec := newFailureRecorder(scanConfig.Handler) + + for pkg, testCases := range rec.pkgFailures { + rerun := rerunOpts{ + runFlag: goTestRunFlagFromTestCases(testCases), + pkg: pkg, + } + goTestProc, err := startGoTest(ctx, goTestCmdArgs(opts, rerun)) + if err != nil { + return errors.Wrapf(err, "failed to run %s", strings.Join(goTestProc.cmd.Args, " ")) + } + + cfg := testjson.ScanConfig{ + Stdout: goTestProc.stdout, + Stderr: goTestProc.stderr, + Handler: nextRec, + Execution: scanConfig.Execution, + } + if _, err := testjson.ScanTestOutput(cfg); err != nil { + return err + } + lastErr = goTestProc.cmd.Wait() + rec = nextRec + } + } + return lastErr +} + +type failureRecorder struct { + testjson.EventHandler + pkgFailures map[string][]string +} + +func newFailureRecorder(handler testjson.EventHandler) *failureRecorder { + return &failureRecorder{ + EventHandler: handler, + pkgFailures: make(map[string][]string), + } +} + +func newFailureRecorderFromExecution(exec *testjson.Execution) *failureRecorder { + r := newFailureRecorder(nil) + for _, tc := range exec.Failed() { + r.pkgFailures[tc.Package] = append(r.pkgFailures[tc.Package], tc.Test) + } + return r +} + +func (r *failureRecorder) Event(event testjson.TestEvent, execution *testjson.Execution) error { + if !event.PackageEvent() && event.Action == testjson.ActionFail { + r.pkgFailures[event.Package] = append(r.pkgFailures[event.Package], event.Test) + } + return r.EventHandler.Event(event, execution) +} + +func (r *failureRecorder) count() int { + total := 0 + for _, tcs := range r.pkgFailures { + total += len(tcs) + } + return total +} + +func goTestRunFlagFromTestCases(tcs []string) string { + buf := new(strings.Builder) + buf.WriteString("-run=") + for i, tc := range tcs { + if i != 0 { + buf.WriteString("|") + } + buf.WriteString(tc) + } + return buf.String() +} diff --git a/testjson/execution.go b/testjson/execution.go index e245f7f4..20c941f4 100644 --- a/testjson/execution.go +++ b/testjson/execution.go @@ -371,6 +371,9 @@ func (e *Execution) Elapsed() time.Duration { // Failed returns a list of all the failed test cases. func (e *Execution) Failed() []TestCase { + if e == nil { + return nil + } var failed []TestCase //nolint:prealloc for _, name := range sortedKeys(e.packages) { pkg := e.packages[name] @@ -449,6 +452,9 @@ type ScanConfig struct { Stderr io.Reader // Handler is a set of callbacks for receiving TestEvents and stderr text. Handler EventHandler + // Execution to populate while scanning. If nil a new one will be created + // and returned from ScanTestOutput. + Execution *Execution } // EventHandler is called by ScanTestOutput for each event and write to stderr. @@ -461,21 +467,26 @@ type EventHandler interface { Err(text string) error } -// ScanTestOutput reads lines from config.Stdout and config.Stderr, creates an +// ScanTestOutput reads lines from config.Stdout and config.Stderr, populates an // Execution, calls the Handler for each event, and returns the Execution. // // If config.Handler is nil, a default no-op handler will be used. +// +// TODO: should the Execution return value be removed func ScanTestOutput(config ScanConfig) (*Execution, error) { + if config.Stdout == nil { + return nil, fmt.Errorf("stdout reader must be non-nil") + } if config.Handler == nil { config.Handler = noopHandler{} } if config.Stderr == nil { config.Stderr = new(bytes.Reader) } - if config.Stdout == nil { - return nil, fmt.Errorf("stdout reader must be non-nil") + execution := config.Execution + if execution == nil { + execution = newExecution() } - execution := newExecution() var group errgroup.Group group.Go(func() error { return readStdout(config, execution)