Address review

cmd/main.go

@@ -360,6 +360,15 @@ func goTestCmdArgs(opts *options, rerunOpts rerunOpts) []string {
 		result = append(result, rerunOpts.runFlag)
 	}
 
+	if rerunOpts.coverprofileFlag != "" {
+		// Replace the existing coverprofile arg with our new one in the re-run case.
+		coverprofileIndex, coverprofileIndexEnd := argIndex("coverprofile", args)
+		if coverprofileIndex >= 0 && coverprofileIndexEnd < len(args) {
+			args = append(args[:coverprofileIndex], args[coverprofileIndexEnd+1:]...)
+		}
+		result = append(result, rerunOpts.coverprofileFlag)
+	}
+
 	pkgArgIndex := findPkgArgPosition(args)
 	result = append(result, args[:pkgArgIndex]...)
 	result = append(result, cmdArgPackageList(opts, rerunOpts)...)

cmd/rerunfails.go

@@ -8,12 +8,15 @@ import (
 	"sort"
 	"strings"
 
+	"golang.org/x/tools/cover"
+	"gotest.tools/gotestsum/internal/coverprofile"
 	"gotest.tools/gotestsum/testjson"
 )
 
 type rerunOpts struct {
-	runFlag string
-	pkg     string
+	runFlag          string
+	pkg              string
+	coverprofileFlag string
 }
 
 func (o rerunOpts) Args() []string {
@@ -24,13 +27,22 @@ func (o rerunOpts) Args() []string {
 	if o.pkg != "" {
 		result = append(result, o.pkg)
 	}
+	if o.coverprofileFlag != "" {
+		result = append(result, o.coverprofileFlag)
+	}
 	return result
 }
 
+func (o rerunOpts) withCoverprofile(coverprofile string) rerunOpts {
+	o.coverprofileFlag = "-coverprofile=" + coverprofile
+	return o
+}
+
 func newRerunOptsFromTestCase(tc testjson.TestCase) rerunOpts {
 	return rerunOpts{
-		runFlag: goTestRunFlagForTestCase(tc.Test),
-		pkg:     tc.Package,
+		runFlag:          goTestRunFlagForTestCase(tc.Test),
+		pkg:              tc.Package,
+		coverprofileFlag: "",
 	}
 }
 
@@ -56,18 +68,31 @@ func rerunFailed(ctx context.Context, opts *options, scanConfig testjson.ScanCon
 	defer cancel()
 	tcFilter := rerunFailsFilter(opts)
 
+	// We need to take special care for the coverprofile file in the rerun
+	// failed case. If we pass the same `-coverprofile` flag to the `go test`
+	// command, it will overwrite the file. We need to combine the coverprofile
+	// files from the original run and the rerun.
+	isCoverprofile, mainProfilePath := coverprofile.ParseCoverProfile(opts.args)
+	rerunProfiles := []*cover.Profile{}
+
 	rec := newFailureRecorderFromExecution(scanConfig.Execution)
 	for attempts := 0; rec.count() > 0 && attempts < opts.rerunFailsMaxAttempts; attempts++ {
 		testjson.PrintSummary(opts.stdout, scanConfig.Execution, testjson.SummarizeNone)
 		opts.stdout.Write([]byte("\n")) // nolint: errcheck
 
 		nextRec := newFailureRecorder(scanConfig.Handler)
-		for _, tc := range tcFilter(rec.failures) {
-			goTestProc, err := startGoTestFn(ctx, "", goTestCmdArgs(opts, newRerunOptsFromTestCase(tc)))
+		for i, tc := range tcFilter(rec.failures) {
+			rerunOpts := newRerunOptsFromTestCase(tc)
+			rerunProfilePath := ""
+			if isCoverprofile {
+				// create a new unique coverprofile filenames for each rerun
+				rerunProfilePath = fmt.Sprintf("%s.%d.%d", mainProfilePath, attempts, i)
+				rerunOpts = rerunOpts.withCoverprofile(rerunProfilePath)
+			}
+			goTestProc, err := startGoTestFn(ctx, "", goTestCmdArgs(opts, rerunOpts))
 			if err != nil {
 				return err
 			}
-
 			cfg := testjson.ScanConfig{
 				RunID:     attempts + 1,
 				Stdout:    goTestProc.stdout,
@@ -83,12 +108,41 @@ func rerunFailed(ctx context.Context, opts *options, scanConfig testjson.ScanCon
 			if exitErr != nil {
 				nextRec.lastErr = exitErr
 			}
+
+			// Need to wait for the go test command to finish before combining
+			// the coverprofile files, but before checking for errors.  Even if
+			// there is errors, we still need to combine the coverprofile files.
+			if isCoverprofile {
+				rerunProfile, err := cover.ParseProfiles(rerunProfilePath)
+				if err != nil {
+					return fmt.Errorf("failed to parse coverprofile %s: %v", rerunProfilePath, err)
+				}
+
+				rerunProfiles = append(rerunProfiles, rerunProfile...)
+
+				// Once we read the rerun profiles from files to memory, we can
+				// safely delete the rerun profile. This will allow us to avoid
+				// extra clean up in the case that we error out in future
+				// attempts.
+				if err := os.Remove(rerunProfilePath); err != nil {
+					return fmt.Errorf("failed to remove coverprofile %s after combined with the main profile: %v", rerunProfilePath, err)
+				}
+			}
+
 			if err := hasErrors(exitErr, scanConfig.Execution); err != nil {
 				return err
 			}
 		}
 		rec = nextRec
 	}
+
+	// Write the combined coverprofile files with the main coverprofile file
+	if isCoverprofile {
+		if err := coverprofile.Combine(mainProfilePath, rerunProfiles); err != nil {
+			return fmt.Errorf("failed to combine coverprofiles: %v", err)
+		}
+	}
+
 	return rec.lastErr
 }
 

internal/coverprofile/coverprofile.go

@@ -0,0 +1,78 @@
+package coverprofile
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	"golang.org/x/tools/cover"
+)
+
+// ParseCoverProfile parse the coverprofile file from the flag
+func ParseCoverProfile(args []string) (bool, string) {
+	for _, arg := range args {
+		if strings.HasPrefix(arg, "-coverprofile=") {
+			return true, strings.TrimPrefix(arg, "-coverprofile=")
+		}
+	}
+
+	return false, ""
+}
+
+// WriteCoverProfile writes the cover profile to the file
+func WriteCoverProfile(profiles []*cover.Profile, filename string) error {
+	// Create a tmp file to write the merged profiles to. Then use os.Rename to
+	// atomically move the file to the main profile to mimic the effect of
+	// atomic replacement of the file. Note, we can't put the file on tempfs
+	// using the all the nice utilities around tempfiles. In places like docker
+	// containers, calling os.Rename on a file that is on tempfs to a file on
+	// normal filesystem partition will fail with errno 18 invalid cross device
+	// link.
+	tempFile := fmt.Sprintf("%s.tmp", filename)
+	f, err := os.Create(tempFile)
+	if err != nil {
+		return fmt.Errorf("failed to create coverprofile file: %v", err)
+	}
+
+	dumpProfiles(profiles, f)
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("failed to close temp file %s: %v", tempFile, err)
+	}
+
+	if err := os.Rename(tempFile, filename); err != nil {
+		return fmt.Errorf("failed to rename temp file %s to %s: %v", tempFile, filename, err)
+	}
+
+	return nil
+}
+
+// CombineProfiles combines the cover profiles together
+func CombineProfiles(this []*cover.Profile, others ...*cover.Profile) []*cover.Profile {
+	merged := this
+	for _, p := range others {
+		merged = addProfile(merged, p)
+	}
+	return merged
+}
+
+// A helper function to expose the destination of the merged profile so we
+// testing is easier.
+func combine(main string, others []*cover.Profile, out string) error {
+	mainProfiles, err := cover.ParseProfiles(main)
+	if err != nil {
+		return fmt.Errorf("failed to parse coverprofile %s: %v", main, err)
+	}
+
+	merged := mainProfiles
+
+	for _, other := range others {
+		merged = CombineProfiles(mainProfiles, other)
+	}
+
+	return WriteCoverProfile(merged, out)
+}
+
+// Combine merges the `others` cover profile with the main cover profile file
+func Combine(main string, others []*cover.Profile) error {
+	return combine(main, others, main)
+}