From 83f28644cf3fdedcf648dc205213d12f0e8b897f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 19 Jun 2022 15:06:45 -0400 Subject: [PATCH 1/9] Add tool ci-matrix --- cmd/tool/matrix/matrix.go | 250 +++++++++++++++++++++++++++++++++ cmd/tool/matrix/matrix_test.go | 112 +++++++++++++++ internal/log/log.go | 10 ++ main.go | 4 + 4 files changed, 376 insertions(+) create mode 100644 cmd/tool/matrix/matrix.go create mode 100644 cmd/tool/matrix/matrix_test.go diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go new file mode 100644 index 00000000..9ee07995 --- /dev/null +++ b/cmd/tool/matrix/matrix.go @@ -0,0 +1,250 @@ +package matrix + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "math" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/dnephin/pflag" + "gotest.tools/gotestsum/internal/log" + "gotest.tools/gotestsum/testjson" +) + +func Run(name string, args []string) error { + flags, opts := setupFlags(name) + switch err := flags.Parse(args); { + case err == pflag.ErrHelp: + return nil + case err != nil: + usage(os.Stderr, name, flags) + return err + } + return run(*opts) +} + +type options struct { + maxAge time.Duration + buckets uint + timingReportsPath string + debug bool +} + +func setupFlags(name string) (*pflag.FlagSet, *options) { + opts := &options{} + flags := pflag.NewFlagSet(name, pflag.ContinueOnError) + flags.SetInterspersed(false) + flags.Usage = func() { + usage(os.Stdout, name, flags) + } + flags.DurationVar(&opts.maxAge, "max-age", 7*24*time.Hour, + "Timing reports older than max age will be deleted") + flags.UintVar(&opts.buckets, "buckets", 4, + "number of parallel buckets to create in the test matrix") + flags.StringVar(&opts.timingReportsPath, "dir", "", + "path to directory that contains jsonfile timing reports") + flags.BoolVar(&opts.debug, "debug", false, + "enable debug logging.") + return flags, opts +} + +func usage(out io.Writer, name string, flags *pflag.FlagSet) { + fmt.Fprintf(out, `Usage: + %[1]s [flags] + +Flags: +`, name) + flags.SetOutput(out) + flags.PrintDefaults() +} + +func run(opts options) error { + log.SetLevel(log.InfoLevel) + if opts.debug { + log.SetLevel(log.DebugLevel) + } + if opts.buckets < 2 { + return fmt.Errorf("--buckets must be atleast 2") + } + if opts.timingReportsPath == "" { + return fmt.Errorf("--dir is required") + } + + pkgs, err := readPackages(os.Stdin) + if err != nil { + return fmt.Errorf("failed to read packages from stdin: %v", err) + } + + files, err := readAndPruneTimingReports(opts) + if err != nil { + return fmt.Errorf("failed to read or delete timing reports: %v", err) + } + defer closeFiles(files) + + pkgTiming, err := packageTiming(files) + if err != nil { + return err + } + + buckets := bucketPackages(packagePercentile(pkgTiming), pkgs, opts.buckets) + return writeBuckets(buckets) +} + +func readPackages(stdin io.Reader) ([]string, error) { + var packages []string + scan := bufio.NewScanner(stdin) + for scan.Scan() { + packages = append(packages, scan.Text()) + } + return packages, scan.Err() +} + +func readAndPruneTimingReports(opts options) ([]*os.File, error) { + entries, err := os.ReadDir(opts.timingReportsPath) + if err != nil { + return nil, err + } + + var files []*os.File + for _, entry := range entries { + if entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { + continue + } + + fh, err := os.Open(filepath.Join(opts.timingReportsPath, entry.Name())) + if err != nil { + return nil, err + } + + event, err := parseEvent(fh) + if err != nil { + return nil, fmt.Errorf("failed to read first event from %v: %v", fh.Name(), err) + } + + age := time.Since(event.Time) + if age < opts.maxAge { + if _, err := fh.Seek(0, io.SeekStart); err != nil { + return nil, fmt.Errorf("failed to reset file: %v", err) + } + files = append(files, fh) + continue + } + + log.Infof("Removing %v because it is from %v", fh.Name(), event.Time.Format(time.RFC1123)) + _ = fh.Close() + if err := os.Remove(fh.Name()); err != nil { + return nil, err + } + } + + log.Infof("Found %v timing reports in %v", len(files), opts.timingReportsPath) + return files, nil +} + +func parseEvent(reader io.Reader) (testjson.TestEvent, error) { + event := testjson.TestEvent{} + err := json.NewDecoder(reader).Decode(&event) + return event, err +} + +func packageTiming(files []*os.File) (map[string][]time.Duration, error) { + timing := make(map[string][]time.Duration) + for _, fh := range files { + exec, err := testjson.ScanTestOutput(testjson.ScanConfig{Stdout: fh}) + if err != nil { + return nil, fmt.Errorf("failed to read events from %v: %v", fh.Name(), err) + } + + for _, pkg := range exec.Packages() { + log.Debugf("package elapsed time %v %v", pkg, exec.Package(pkg).Elapsed()) + timing[pkg] = append(timing[pkg], exec.Package(pkg).Elapsed()) + } + } + return timing, nil +} + +func packagePercentile(timing map[string][]time.Duration) map[string]time.Duration { + result := make(map[string]time.Duration) + for pkg, times := range timing { + l := len(times) + if l == 0 { + result[pkg] = 0 + continue + } + + sort.Slice(times, func(i, j int) bool { + return times[i] < times[j] + }) + + r := int(math.Ceil(0.85 * float64(l))) + if r == 0 { + result[pkg] = times[0] + continue + } + result[pkg] = times[r-1] + } + return result +} + +func closeFiles(files []*os.File) { + for _, fh := range files { + _ = fh.Close() + } +} + +func bucketPackages(timing map[string]time.Duration, packages []string, n uint) []bucket { + sort.SliceStable(packages, func(i, j int) bool { + return timing[packages[i]] >= timing[packages[j]] + }) + + buckets := make([]bucket, n) + for _, pkg := range packages { + i := minBucket(buckets) + buckets[i].Total += timing[pkg] + buckets[i].Packages = append(buckets[i].Packages, pkg) + log.Debugf("adding %v (%v) to bucket %v with total %v", + pkg, timing[pkg], i, buckets[i].Total) + } + return buckets +} + +func minBucket(buckets []bucket) int { + var n int + var min time.Duration = -1 + for i, b := range buckets { + switch { + case min < 0 || b.Total < min: + min = b.Total + n = i + case b.Total == min && len(buckets[i].Packages) < len(buckets[n].Packages): + n = i + } + } + return n +} + +type bucket struct { + Total time.Duration + Packages []string +} + +func writeBuckets(buckets []bucket) error { + out := make(map[int]string) + for i, bucket := range buckets { + out[i] = strings.Join(bucket.Packages, " ") + } + + raw, err := json.Marshal(out) + if err != nil { + return fmt.Errorf("failed to json encode output: %v", err) + } + log.Debugf(string(raw)) + fmt.Println(string(raw)) + return nil +} diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go new file mode 100644 index 00000000..a19211fe --- /dev/null +++ b/cmd/tool/matrix/matrix_test.go @@ -0,0 +1,112 @@ +package matrix + +import ( + "strconv" + "testing" + "time" + + "gotest.tools/v3/assert" +) + +func TestPackagePercentile(t *testing.T) { + ms := time.Millisecond + timing := map[string][]time.Duration{ + "none": {}, + "one": {time.Second}, + "two": {4 * ms, 2 * ms}, + "three": {2 * ms, 3 * ms, 5 * ms}, + "four": {4 * ms, 3 * ms, ms, 2 * ms}, + "five": {6 * ms, 2 * ms, 3 * ms, 4 * ms, 9 * ms}, + "nine": {6 * ms, 2 * ms, 3 * ms, 4 * ms, 9 * ms, 1 * ms, 5 * ms, 7 * ms, 8 * ms}, + "ten": {6 * ms, 2 * ms, 3 * ms, 4 * ms, 9 * ms, 5 * ms, 7 * ms, 8 * ms, ms, ms}, + "twenty": { + 6 * ms, 2 * ms, 3 * ms, 4 * ms, 9 * ms, 5 * ms, 7 * ms, 8 * ms, ms, ms, + 100, 200, 600, 700, 800, 900, 200, 300, 400, 500, + }, + } + + out := packagePercentile(timing) + expected := map[string]time.Duration{ + "none": 0, + "one": time.Second, + "two": 4 * ms, + "three": 5 * ms, + "four": 4 * ms, + "five": 9 * ms, + "nine": 8 * ms, + "ten": 8 * ms, + "twenty": 6 * ms, + } + assert.DeepEqual(t, out, expected) +} + +func TestBucketPackages(t *testing.T) { + ms := time.Millisecond + timing := map[string]time.Duration{ + "one": 190 * ms, + "two": 200 * ms, + "three": 3800 * ms, + "four": 4000 * ms, + "five": 50 * ms, + "six": 606 * ms, + "rm1": time.Second, + "rm2": time.Second, + } + packages := []string{"new1", "new2", "one", "two", "three", "four", "five", "six"} + + type testCase struct { + n uint + expected []bucket + } + + run := func(t *testing.T, tc testCase) { + buckets := bucketPackages(timing, packages, tc.n) + assert.DeepEqual(t, buckets, tc.expected) + } + + testCases := []testCase{ + { + n: 2, + expected: []bucket{ + 0: {Total: 4440 * ms, Packages: []string{"four", "two", "one", "five"}}, + 1: {Total: 4406 * ms, Packages: []string{"three", "six", "new2", "new1"}}, + }, + }, + { + n: 3, + expected: []bucket{ + 0: {Total: 4000 * ms, Packages: []string{"four"}}, + 1: {Total: 3800 * ms, Packages: []string{"three"}}, + 2: {Total: 1046 * ms, Packages: []string{"six", "two", "one", "five", "new1", "new2"}}, + }, + }, + { + n: 4, + expected: []bucket{ + 0: {Total: 4000 * ms, Packages: []string{"four"}}, + 1: {Total: 3800 * ms, Packages: []string{"three"}}, + 2: {Total: 606 * ms, Packages: []string{"six"}}, + 3: {Total: 440 * ms, Packages: []string{"two", "one", "five", "new2", "new1"}}, + }, + }, + { + n: 8, + expected: []bucket{ + 0: {Total: 4000 * ms, Packages: []string{"four"}}, + 1: {Total: 3800 * ms, Packages: []string{"three"}}, + 2: {Total: 606 * ms, Packages: []string{"six"}}, + 3: {Total: 200 * ms, Packages: []string{"two"}}, + 4: {Total: 190 * ms, Packages: []string{"one"}}, + 5: {Total: 50 * ms, Packages: []string{"five"}}, + 6: {Packages: []string{"new1"}}, + 7: {Packages: []string{"new2"}}, + }, + }, + } + + for _, tc := range testCases { + t.Run(strconv.FormatUint(uint64(tc.n), 10), func(t *testing.T) { + run(t, tc) + }) + } +} diff --git a/internal/log/log.go b/internal/log/log.go index 9a3e85f3..d68955bf 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -11,6 +11,7 @@ type Level uint8 const ( ErrorLevel Level = iota WarnLevel + InfoLevel DebugLevel ) @@ -43,6 +44,15 @@ func Debugf(format string, args ...interface{}) { fmt.Fprint(out, "\n") } +// Infof prints the message to stderr, with no prefix. +func Infof(format string, args ...interface{}) { + if level < InfoLevel { + return + } + fmt.Fprintf(out, format, args...) + fmt.Fprint(out, "\n") +} + // Errorf prints the message to stderr, with a red ERROR prefix. func Errorf(format string, args ...interface{}) { if level < ErrorLevel { diff --git a/main.go b/main.go index d2ed49f7..f8af75e5 100644 --- a/main.go +++ b/main.go @@ -5,6 +5,7 @@ import ( "os" "gotest.tools/gotestsum/cmd" + "gotest.tools/gotestsum/cmd/tool/matrix" "gotest.tools/gotestsum/cmd/tool/slowest" "gotest.tools/gotestsum/internal/log" ) @@ -55,6 +56,7 @@ func toolRun(name string, args []string) error { Commands: %[1]s slowest find or skip the slowest tests + %[1]s ci-matrix use previous test runtime to place packages into optimal buckets Use '%[1]s COMMAND --help' for command specific help. `, name) @@ -67,6 +69,8 @@ Use '%[1]s COMMAND --help' for command specific help. return nil case "slowest": return slowest.Run(name+" "+next, rest) + case "ci-matrix": + return matrix.Run(name+" "+next, rest) default: fmt.Fprintln(os.Stderr, usage(name)) return fmt.Errorf("invalid command: %v %v", name, next) From f9f480bb6ca3d5f357bcb22b4ee3b424aa0f82a5 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 4 Sep 2022 13:47:14 -0400 Subject: [PATCH 2/9] ci-matrix: Make file pruning safer Accept a glob pattern instead of a directory so that the tool only reads files that match the pattern. Should generally only be used with a glob pattern that matches file extensions or file names. Rename the flag to timing-files Remove the default max age, to prevent accidentaly deleting files. Add test coverage for the function. --- cmd/tool/matrix/matrix.go | 45 +++++++++--------- cmd/tool/matrix/matrix_test.go | 84 ++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 24 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 9ee07995..1c9128b0 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -30,10 +30,10 @@ func Run(name string, args []string) error { } type options struct { - maxAge time.Duration - buckets uint - timingReportsPath string - debug bool + pruneFilesMaxAgeDays uint + buckets uint + timingFilesPattern string + debug bool } func setupFlags(name string) (*pflag.FlagSet, *options) { @@ -43,14 +43,14 @@ func setupFlags(name string) (*pflag.FlagSet, *options) { flags.Usage = func() { usage(os.Stdout, name, flags) } - flags.DurationVar(&opts.maxAge, "max-age", 7*24*time.Hour, - "Timing reports older than max age will be deleted") + flags.UintVar(&opts.pruneFilesMaxAgeDays, "max-age-days", 0, + "timing files older than this value will be deleted") flags.UintVar(&opts.buckets, "buckets", 4, "number of parallel buckets to create in the test matrix") - flags.StringVar(&opts.timingReportsPath, "dir", "", - "path to directory that contains jsonfile timing reports") + flags.StringVar(&opts.timingFilesPattern, "timing-files", "", + "glob pattern to match files that contain test2json events, ex: ./logs/*.log") flags.BoolVar(&opts.debug, "debug", false, - "enable debug logging.") + "enable debug logging") return flags, opts } @@ -72,8 +72,8 @@ func run(opts options) error { if opts.buckets < 2 { return fmt.Errorf("--buckets must be atleast 2") } - if opts.timingReportsPath == "" { - return fmt.Errorf("--dir is required") + if opts.timingFilesPattern == "" { + return fmt.Errorf("--timing-files is required") } pkgs, err := readPackages(os.Stdin) @@ -83,7 +83,7 @@ func run(opts options) error { files, err := readAndPruneTimingReports(opts) if err != nil { - return fmt.Errorf("failed to read or delete timing reports: %v", err) + return fmt.Errorf("failed to read or delete timing files: %v", err) } defer closeFiles(files) @@ -106,18 +106,14 @@ func readPackages(stdin io.Reader) ([]string, error) { } func readAndPruneTimingReports(opts options) ([]*os.File, error) { - entries, err := os.ReadDir(opts.timingReportsPath) + fileNames, err := filepath.Glob(opts.timingFilesPattern) if err != nil { return nil, err } var files []*os.File - for _, entry := range entries { - if entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { - continue - } - - fh, err := os.Open(filepath.Join(opts.timingReportsPath, entry.Name())) + for _, fileName := range fileNames { + fh, err := os.Open(fileName) if err != nil { return nil, err } @@ -128,7 +124,8 @@ func readAndPruneTimingReports(opts options) ([]*os.File, error) { } age := time.Since(event.Time) - if age < opts.maxAge { + maxAge := time.Duration(opts.pruneFilesMaxAgeDays) * 24 * time.Hour + if opts.pruneFilesMaxAgeDays == 0 || age < maxAge { if _, err := fh.Seek(0, io.SeekStart); err != nil { return nil, fmt.Errorf("failed to reset file: %v", err) } @@ -143,7 +140,7 @@ func readAndPruneTimingReports(opts options) ([]*os.File, error) { } } - log.Infof("Found %v timing reports in %v", len(files), opts.timingReportsPath) + log.Infof("Found %v timing files in %v", len(files), opts.timingFilesPattern) return files, nil } @@ -172,8 +169,8 @@ func packageTiming(files []*os.File) (map[string][]time.Duration, error) { func packagePercentile(timing map[string][]time.Duration) map[string]time.Duration { result := make(map[string]time.Duration) for pkg, times := range timing { - l := len(times) - if l == 0 { + lenTimes := len(times) + if lenTimes == 0 { result[pkg] = 0 continue } @@ -182,7 +179,7 @@ func packagePercentile(timing map[string][]time.Duration) map[string]time.Durati return times[i] < times[j] }) - r := int(math.Ceil(0.85 * float64(l))) + r := int(math.Ceil(0.85 * float64(lenTimes))) if r == 0 { result[pkg] = times[0] continue diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go index a19211fe..441b75ea 100644 --- a/cmd/tool/matrix/matrix_test.go +++ b/cmd/tool/matrix/matrix_test.go @@ -1,11 +1,16 @@ package matrix import ( + "bytes" + "encoding/json" + "os" "strconv" "testing" "time" + "gotest.tools/gotestsum/testjson" "gotest.tools/v3/assert" + "gotest.tools/v3/fs" ) func TestPackagePercentile(t *testing.T) { @@ -110,3 +115,82 @@ func TestBucketPackages(t *testing.T) { }) } } + +func TestReadAndPruneTimingReports(t *testing.T) { + events := func(t *testing.T, start time.Time) string { + t.Helper() + var buf bytes.Buffer + encoder := json.NewEncoder(&buf) + for _, i := range []int{0, 1, 2} { + assert.NilError(t, encoder.Encode(testjson.TestEvent{ + Time: start.Add(time.Duration(i) * time.Second), + Action: testjson.ActionRun, + Package: "pkg" + strconv.Itoa(i), + })) + buf.WriteString("\n") + } + return buf.String() + } + + now := time.Now() + dir := fs.NewDir(t, "timing-files", + fs.WithFile("report1.log", events(t, now.Add(-time.Hour))), + fs.WithFile("report2.log", events(t, now.Add(-47*time.Hour))), + fs.WithFile("report3.log", events(t, now.Add(-49*time.Hour))), + fs.WithFile("report4.log", events(t, now.Add(-101*time.Hour)))) + + t.Run("no prune", func(t *testing.T) { + opts := options{ + timingFilesPattern: dir.Join("*.log"), + } + + files, err := readAndPruneTimingReports(opts) + assert.NilError(t, err) + defer closeFiles(files) + assert.Equal(t, len(files), 4) + + for _, fh := range files { + // check the files are properly seeked to 0 + event, err := parseEvent(fh) + assert.NilError(t, err) + assert.Equal(t, event.Package, "pkg0") + } + + actual, err := os.ReadDir(dir.Path()) + assert.NilError(t, err) + assert.Equal(t, len(actual), 4) + }) + + t.Run("no glob match, func", func(t *testing.T) { + opts := options{ + timingFilesPattern: dir.Join("*.json"), + } + + files, err := readAndPruneTimingReports(opts) + assert.NilError(t, err) + assert.Equal(t, len(files), 0) + }) + + t.Run("prune older than max age", func(t *testing.T) { + opts := options{ + timingFilesPattern: dir.Join("*.log"), + pruneFilesMaxAgeDays: 2, + } + + files, err := readAndPruneTimingReports(opts) + assert.NilError(t, err) + defer closeFiles(files) + assert.Equal(t, len(files), 2) + + for _, fh := range files { + // check the files are properly seeked to 0 + event, err := parseEvent(fh) + assert.NilError(t, err) + assert.Equal(t, event.Package, "pkg0") + } + + actual, err := os.ReadDir(dir.Path()) + assert.NilError(t, err) + assert.Equal(t, len(actual), 2) + }) +} From d3cb1418def6499912133067d666deabf4737608 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 4 Sep 2022 14:01:17 -0400 Subject: [PATCH 3/9] ci-matrix: rename buckets to partitions And remove the default value --- cmd/tool/matrix/matrix.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 1c9128b0..775d7536 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -31,7 +31,7 @@ func Run(name string, args []string) error { type options struct { pruneFilesMaxAgeDays uint - buckets uint + numPartitions uint timingFilesPattern string debug bool } @@ -45,8 +45,8 @@ func setupFlags(name string) (*pflag.FlagSet, *options) { } flags.UintVar(&opts.pruneFilesMaxAgeDays, "max-age-days", 0, "timing files older than this value will be deleted") - flags.UintVar(&opts.buckets, "buckets", 4, - "number of parallel buckets to create in the test matrix") + flags.UintVar(&opts.numPartitions, "partitions", 0, + "number of parallel partitions to create in the test matrix") flags.StringVar(&opts.timingFilesPattern, "timing-files", "", "glob pattern to match files that contain test2json events, ex: ./logs/*.log") flags.BoolVar(&opts.debug, "debug", false, @@ -69,8 +69,8 @@ func run(opts options) error { if opts.debug { log.SetLevel(log.DebugLevel) } - if opts.buckets < 2 { - return fmt.Errorf("--buckets must be atleast 2") + if opts.numPartitions < 2 { + return fmt.Errorf("--partitions must be atleast 2") } if opts.timingFilesPattern == "" { return fmt.Errorf("--timing-files is required") @@ -92,7 +92,7 @@ func run(opts options) error { return err } - buckets := bucketPackages(packagePercentile(pkgTiming), pkgs, opts.buckets) + buckets := bucketPackages(packagePercentile(pkgTiming), pkgs, opts.numPartitions) return writeBuckets(buckets) } From 6f3c7939b3f5e4cfbc34737914c803cee14df2f1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 4 Sep 2022 15:00:58 -0400 Subject: [PATCH 4/9] ci-matrix: output the entire matrix Which should make it easier to consume in the github workflow. --- cmd/tool/matrix/matrix.go | 73 +++++++++++++++++++++++++++++----- cmd/tool/matrix/matrix_test.go | 53 +++++++++++++++++++++++- 2 files changed, 116 insertions(+), 10 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 775d7536..29a0f65d 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -9,7 +9,6 @@ import ( "os" "path/filepath" "sort" - "strings" "time" "github.com/dnephin/pflag" @@ -26,6 +25,8 @@ func Run(name string, args []string) error { usage(os.Stderr, name, flags) return err } + opts.stdin = os.Stdin + opts.stdout = os.Stdout return run(*opts) } @@ -34,6 +35,10 @@ type options struct { numPartitions uint timingFilesPattern string debug bool + + // shims for testing + stdin io.Reader + stdout io.Writer } func setupFlags(name string) (*pflag.FlagSet, *options) { @@ -58,6 +63,19 @@ func usage(out io.Writer, name string, flags *pflag.FlagSet) { fmt.Fprintf(out, `Usage: %[1]s [flags] +Read a list of packages from stdin and output a GitHub Actions matrix strategy +that splits the packages by previous run times to minimize overall CI runtime. + +Example + + echo -n "::set-output name=matrix::" + go list ./... | \ + %[1]s --partitions 4 --timing-files ./*.log --max-age-days 10 + +The output of the command is a JSON object that can be used as the matrix +strategy for a test job. + + Flags: `, name) flags.SetOutput(out) @@ -76,7 +94,7 @@ func run(opts options) error { return fmt.Errorf("--timing-files is required") } - pkgs, err := readPackages(os.Stdin) + pkgs, err := readPackages(opts.stdin) if err != nil { return fmt.Errorf("failed to read packages from stdin: %v", err) } @@ -93,7 +111,7 @@ func run(opts options) error { } buckets := bucketPackages(packagePercentile(pkgTiming), pkgs, opts.numPartitions) - return writeBuckets(buckets) + return writeMatrix(opts.stdout, buckets) } func readPackages(stdin io.Reader) ([]string, error) { @@ -231,17 +249,54 @@ type bucket struct { Packages []string } -func writeBuckets(buckets []bucket) error { - out := make(map[int]string) +type matrix struct { + Include []Partition `json:"include"` +} + +type Partition struct { + ID int `json:"id"` + EstimatedRuntime time.Duration `json:"estimatedRuntime"` + Packages []string `json:"packages"` + Description string `json:"description"` +} + +func writeMatrix(out io.Writer, buckets []bucket) error { + m := matrix{Include: make([]Partition, len(buckets))} for i, bucket := range buckets { - out[i] = strings.Join(bucket.Packages, " ") + p := Partition{ + ID: i, + EstimatedRuntime: bucket.Total, + Packages: bucket.Packages, + } + if len(p.Packages) > 0 { + var extra string + if len(p.Packages) > 1 { + extra = fmt.Sprintf(" and %d others", len(p.Packages)-1) + } + p.Description = fmt.Sprintf("package %v%v (%v)", + testjson.RelativePackagePath(p.Packages[0]), + extra, + p.EstimatedRuntime) + } + + m.Include[i] = p } - raw, err := json.Marshal(out) + log.Debugf("%v\n", debugMatrix(m)) + + err := json.NewEncoder(out).Encode(m) if err != nil { return fmt.Errorf("failed to json encode output: %v", err) } - log.Debugf(string(raw)) - fmt.Println(string(raw)) return nil } + +type debugMatrix matrix + +func (d debugMatrix) String() string { + raw, err := json.MarshalIndent(d, "", " ") + if err != nil { + return fmt.Sprintf("failed to marshal: %v", err.Error()) + } + return string(raw) +} diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go index 441b75ea..d88e7d2f 100644 --- a/cmd/tool/matrix/matrix_test.go +++ b/cmd/tool/matrix/matrix_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "os" "strconv" + "strings" "testing" "time" @@ -127,7 +128,6 @@ func TestReadAndPruneTimingReports(t *testing.T) { Action: testjson.ActionRun, Package: "pkg" + strconv.Itoa(i), })) - buf.WriteString("\n") } return buf.String() } @@ -194,3 +194,54 @@ func TestReadAndPruneTimingReports(t *testing.T) { assert.Equal(t, len(actual), 2) }) } + +func TestRun(t *testing.T) { + events := func(t *testing.T) string { + t.Helper() + var buf bytes.Buffer + encoder := json.NewEncoder(&buf) + for _, i := range []int{0, 1, 2} { + elapsed := time.Duration(i+1) * 2 * time.Second + end := time.Now().Add(-5 * time.Second) + start := end.Add(-elapsed) + + assert.NilError(t, encoder.Encode(testjson.TestEvent{ + Time: start, + Action: testjson.ActionRun, + Package: "pkg" + strconv.Itoa(i), + })) + assert.NilError(t, encoder.Encode(testjson.TestEvent{ + Time: end, + Action: testjson.ActionPass, + Package: "pkg" + strconv.Itoa(i), + Elapsed: elapsed.Seconds(), + })) + } + return buf.String() + } + + dir := fs.NewDir(t, "timing-files", + fs.WithFile("report1.log", events(t)), + fs.WithFile("report2.log", events(t)), + fs.WithFile("report3.log", events(t)), + fs.WithFile("report4.log", events(t)), + fs.WithFile("report5.log", events(t))) + + stdout := new(bytes.Buffer) + opts := options{ + numPartitions: 3, + timingFilesPattern: dir.Join("*.log"), + debug: true, + stdout: stdout, + stdin: strings.NewReader("pkg0\npkg1\npkg2\nother"), + } + err := run(opts) + assert.NilError(t, err) + + assert.Equal(t, stdout.String(), expectedMatrix) +} + +// expectedMatrix can be automatically updated by running tests with -update +// nolint:lll +var expectedMatrix = `{"include":[{"id":0,"estimatedRuntime":6000000000,"packages":["pkg2"],"description":"package pkg2 (6s)"},{"id":1,"estimatedRuntime":4000000000,"packages":["pkg1"],"description":"package pkg1 (4s)"},{"id":2,"estimatedRuntime":2000000000,"packages":["pkg0","other"],"description":"package pkg0 and 1 others (2s)"}]} +` From b4cf789403d94777624016096db977d05c0fedd3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 4 Sep 2022 15:55:26 -0400 Subject: [PATCH 5/9] ci-matrix: improve description and format elapsed time in the output and remove overly verbose debug log --- cmd/tool/matrix/matrix.go | 17 +++++++---------- cmd/tool/matrix/matrix_test.go | 2 +- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 29a0f65d..889fc9d6 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -177,7 +177,6 @@ func packageTiming(files []*os.File) (map[string][]time.Duration, error) { } for _, pkg := range exec.Packages() { - log.Debugf("package elapsed time %v %v", pkg, exec.Package(pkg).Elapsed()) timing[pkg] = append(timing[pkg], exec.Package(pkg).Elapsed()) } } @@ -254,10 +253,10 @@ type matrix struct { } type Partition struct { - ID int `json:"id"` - EstimatedRuntime time.Duration `json:"estimatedRuntime"` - Packages []string `json:"packages"` - Description string `json:"description"` + ID int `json:"id"` + EstimatedRuntime string `json:"estimatedRuntime"` + Packages []string `json:"packages"` + Description string `json:"description"` } func writeMatrix(out io.Writer, buckets []bucket) error { @@ -265,7 +264,7 @@ func writeMatrix(out io.Writer, buckets []bucket) error { for i, bucket := range buckets { p := Partition{ ID: i, - EstimatedRuntime: bucket.Total, + EstimatedRuntime: bucket.Total.String(), Packages: bucket.Packages, } if len(p.Packages) > 0 { @@ -273,10 +272,8 @@ func writeMatrix(out io.Writer, buckets []bucket) error { if len(p.Packages) > 1 { extra = fmt.Sprintf(" and %d others", len(p.Packages)-1) } - p.Description = fmt.Sprintf("package %v%v (%v)", - testjson.RelativePackagePath(p.Packages[0]), - extra, - p.EstimatedRuntime) + p.Description = fmt.Sprintf("partition %d - package %v%v", + p.ID, testjson.RelativePackagePath(p.Packages[0]), extra) } m.Include[i] = p diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go index d88e7d2f..7b01b371 100644 --- a/cmd/tool/matrix/matrix_test.go +++ b/cmd/tool/matrix/matrix_test.go @@ -243,5 +243,5 @@ func TestRun(t *testing.T) { // expectedMatrix can be automatically updated by running tests with -update // nolint:lll -var expectedMatrix = `{"include":[{"id":0,"estimatedRuntime":6000000000,"packages":["pkg2"],"description":"package pkg2 (6s)"},{"id":1,"estimatedRuntime":4000000000,"packages":["pkg1"],"description":"package pkg1 (4s)"},{"id":2,"estimatedRuntime":2000000000,"packages":["pkg0","other"],"description":"package pkg0 and 1 others (2s)"}]} +var expectedMatrix = `{"include":[{"id":0,"estimatedRuntime":"6s","packages":["pkg2"],"description":"partition 0 - package pkg2"},{"id":1,"estimatedRuntime":"4s","packages":["pkg1"],"description":"partition 1 - package pkg1"},{"id":2,"estimatedRuntime":"2s","packages":["pkg0","other"],"description":"partition 2 - package pkg0 and 1 others"}]} ` From e1eee65374e7d7f17eff3020c76f994a03dce849 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 5 Sep 2022 14:53:10 -0400 Subject: [PATCH 6/9] ci-matrix: output packages as a single string So that we don't need to join them in the github action And so that when we add splitting of tests in a single package we can continue to use this package field to set the package name. --- cmd/tool/matrix/matrix.go | 19 ++++++++-------- cmd/tool/matrix/matrix_test.go | 40 +++++++++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 889fc9d6..4b6d8b55 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "sort" + "strings" "time" "github.com/dnephin/pflag" @@ -253,10 +254,10 @@ type matrix struct { } type Partition struct { - ID int `json:"id"` - EstimatedRuntime string `json:"estimatedRuntime"` - Packages []string `json:"packages"` - Description string `json:"description"` + ID int `json:"id"` + EstimatedRuntime string `json:"estimatedRuntime"` + Packages string `json:"packages"` + Description string `json:"description"` } func writeMatrix(out io.Writer, buckets []bucket) error { @@ -265,15 +266,15 @@ func writeMatrix(out io.Writer, buckets []bucket) error { p := Partition{ ID: i, EstimatedRuntime: bucket.Total.String(), - Packages: bucket.Packages, + Packages: strings.Join(bucket.Packages, " "), } - if len(p.Packages) > 0 { + if len(bucket.Packages) > 0 { var extra string - if len(p.Packages) > 1 { - extra = fmt.Sprintf(" and %d others", len(p.Packages)-1) + if len(bucket.Packages) > 1 { + extra = fmt.Sprintf(" and %d others", len(bucket.Packages)-1) } p.Description = fmt.Sprintf("partition %d - package %v%v", - p.ID, testjson.RelativePackagePath(p.Packages[0]), extra) + p.ID, testjson.RelativePackagePath(bucket.Packages[0]), extra) } m.Include[i] = p diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go index 7b01b371..aa4d226f 100644 --- a/cmd/tool/matrix/matrix_test.go +++ b/cmd/tool/matrix/matrix_test.go @@ -3,6 +3,7 @@ package matrix import ( "bytes" "encoding/json" + "io" "os" "strconv" "strings" @@ -237,11 +238,44 @@ func TestRun(t *testing.T) { } err := run(opts) assert.NilError(t, err) + assert.Equal(t, strings.Count(stdout.String(), "\n"), 1, + "the output should be a single line") - assert.Equal(t, stdout.String(), expectedMatrix) + assert.Equal(t, formatJSON(t, stdout), expectedMatrix) } // expectedMatrix can be automatically updated by running tests with -update // nolint:lll -var expectedMatrix = `{"include":[{"id":0,"estimatedRuntime":"6s","packages":["pkg2"],"description":"partition 0 - package pkg2"},{"id":1,"estimatedRuntime":"4s","packages":["pkg1"],"description":"partition 1 - package pkg1"},{"id":2,"estimatedRuntime":"2s","packages":["pkg0","other"],"description":"partition 2 - package pkg0 and 1 others"}]} -` +var expectedMatrix = `{ + "include": [ + { + "description": "partition 0 - package pkg2", + "estimatedRuntime": "6s", + "id": 0, + "packages": "pkg2" + }, + { + "description": "partition 1 - package pkg1", + "estimatedRuntime": "4s", + "id": 1, + "packages": "pkg1" + }, + { + "description": "partition 2 - package pkg0 and 1 others", + "estimatedRuntime": "2s", + "id": 2, + "packages": "pkg0 other" + } + ] +}` + +func formatJSON(t *testing.T, v io.Reader) string { + t.Helper() + raw := map[string]interface{}{} + err := json.NewDecoder(v).Decode(&raw) + assert.NilError(t, err) + + formatted, err := json.MarshalIndent(raw, "", " ") + assert.NilError(t, err) + return string(formatted) +} From ff9bd18eb9019133f950c13d910798c1a38279c3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 5 Sep 2022 14:58:13 -0400 Subject: [PATCH 7/9] ci-matrix: add test of only pruning files --- cmd/tool/matrix/matrix.go | 2 +- cmd/tool/matrix/matrix_test.go | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 4b6d8b55..6604234c 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -130,7 +130,7 @@ func readAndPruneTimingReports(opts options) ([]*os.File, error) { return nil, err } - var files []*os.File + files := make([]*os.File, 0, len(fileNames)) for _, fileName := range fileNames { fh, err := os.Open(fileName) if err != nil { diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go index aa4d226f..9f227fa2 100644 --- a/cmd/tool/matrix/matrix_test.go +++ b/cmd/tool/matrix/matrix_test.go @@ -279,3 +279,42 @@ func formatJSON(t *testing.T, v io.Reader) string { assert.NilError(t, err) return string(formatted) } + +func TestRun_PruneOnly(t *testing.T) { + events := func(t *testing.T, start time.Time) string { + t.Helper() + var buf bytes.Buffer + encoder := json.NewEncoder(&buf) + for _, i := range []int{0, 1, 2} { + assert.NilError(t, encoder.Encode(testjson.TestEvent{ + Time: start.Add(time.Duration(i) * time.Second), + Action: testjson.ActionRun, + Package: "pkg" + strconv.Itoa(i), + })) + } + return buf.String() + } + + now := time.Now() + dir := fs.NewDir(t, "timing-files", + fs.WithFile("report1.log", events(t, now.Add(-time.Hour))), + fs.WithFile("report2.log", events(t, now.Add(-47*time.Hour))), + fs.WithFile("report3.log", events(t, now.Add(-49*time.Hour))), + fs.WithFile("report4.log", events(t, now.Add(-101*time.Hour)))) + + stdout := new(bytes.Buffer) + opts := options{ + numPartitions: 3, + timingFilesPattern: dir.Join("*.log"), + debug: true, + stdout: stdout, + stdin: strings.NewReader(""), + pruneFilesMaxAgeDays: 2, + } + err := run(opts) + assert.NilError(t, err) + + assert.Assert(t, fs.Equal(dir.Path(), fs.Expected(t, + fs.WithFile("report1.log", events(t, now.Add(-time.Hour))), + fs.WithFile("report2.log", events(t, now.Add(-47*time.Hour)))))) +} From d0adc241710a44f874fccf54e821fc6c5c430616 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 5 Sep 2022 18:10:09 -0400 Subject: [PATCH 8/9] remove file pruning This can be done easily with the find command, no need to write it again. --- cmd/tool/matrix/matrix.go | 39 ++++--------------- cmd/tool/matrix/matrix_test.go | 70 ++-------------------------------- 2 files changed, 11 insertions(+), 98 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 6604234c..0b7a4fbe 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -32,10 +32,9 @@ func Run(name string, args []string) error { } type options struct { - pruneFilesMaxAgeDays uint - numPartitions uint - timingFilesPattern string - debug bool + numPartitions uint + timingFilesPattern string + debug bool // shims for testing stdin io.Reader @@ -49,8 +48,6 @@ func setupFlags(name string) (*pflag.FlagSet, *options) { flags.Usage = func() { usage(os.Stdout, name, flags) } - flags.UintVar(&opts.pruneFilesMaxAgeDays, "max-age-days", 0, - "timing files older than this value will be deleted") flags.UintVar(&opts.numPartitions, "partitions", 0, "number of parallel partitions to create in the test matrix") flags.StringVar(&opts.timingFilesPattern, "timing-files", "", @@ -67,11 +64,9 @@ func usage(out io.Writer, name string, flags *pflag.FlagSet) { Read a list of packages from stdin and output a GitHub Actions matrix strategy that splits the packages by previous run times to minimize overall CI runtime. -Example - echo -n "::set-output name=matrix::" go list ./... | \ - %[1]s --partitions 4 --timing-files ./*.log --max-age-days 10 + %[1]s --timing-files ./*.log --partitions 4 The output of the command is a JSON object that can be used as the matrix strategy for a test job. @@ -100,7 +95,7 @@ func run(opts options) error { return fmt.Errorf("failed to read packages from stdin: %v", err) } - files, err := readAndPruneTimingReports(opts) + files, err := readTimingReports(opts) if err != nil { return fmt.Errorf("failed to read or delete timing files: %v", err) } @@ -124,7 +119,7 @@ func readPackages(stdin io.Reader) ([]string, error) { return packages, scan.Err() } -func readAndPruneTimingReports(opts options) ([]*os.File, error) { +func readTimingReports(opts options) ([]*os.File, error) { fileNames, err := filepath.Glob(opts.timingFilesPattern) if err != nil { return nil, err @@ -136,27 +131,7 @@ func readAndPruneTimingReports(opts options) ([]*os.File, error) { if err != nil { return nil, err } - - event, err := parseEvent(fh) - if err != nil { - return nil, fmt.Errorf("failed to read first event from %v: %v", fh.Name(), err) - } - - age := time.Since(event.Time) - maxAge := time.Duration(opts.pruneFilesMaxAgeDays) * 24 * time.Hour - if opts.pruneFilesMaxAgeDays == 0 || age < maxAge { - if _, err := fh.Seek(0, io.SeekStart); err != nil { - return nil, fmt.Errorf("failed to reset file: %v", err) - } - files = append(files, fh) - continue - } - - log.Infof("Removing %v because it is from %v", fh.Name(), event.Time.Format(time.RFC1123)) - _ = fh.Close() - if err := os.Remove(fh.Name()); err != nil { - return nil, err - } + files = append(files, fh) } log.Infof("Found %v timing files in %v", len(files), opts.timingFilesPattern) diff --git a/cmd/tool/matrix/matrix_test.go b/cmd/tool/matrix/matrix_test.go index 9f227fa2..b537318d 100644 --- a/cmd/tool/matrix/matrix_test.go +++ b/cmd/tool/matrix/matrix_test.go @@ -118,7 +118,7 @@ func TestBucketPackages(t *testing.T) { } } -func TestReadAndPruneTimingReports(t *testing.T) { +func TestReadTimingReports(t *testing.T) { events := func(t *testing.T, start time.Time) string { t.Helper() var buf bytes.Buffer @@ -140,12 +140,12 @@ func TestReadAndPruneTimingReports(t *testing.T) { fs.WithFile("report3.log", events(t, now.Add(-49*time.Hour))), fs.WithFile("report4.log", events(t, now.Add(-101*time.Hour)))) - t.Run("no prune", func(t *testing.T) { + t.Run("match files", func(t *testing.T) { opts := options{ timingFilesPattern: dir.Join("*.log"), } - files, err := readAndPruneTimingReports(opts) + files, err := readTimingReports(opts) assert.NilError(t, err) defer closeFiles(files) assert.Equal(t, len(files), 4) @@ -167,33 +167,10 @@ func TestReadAndPruneTimingReports(t *testing.T) { timingFilesPattern: dir.Join("*.json"), } - files, err := readAndPruneTimingReports(opts) + files, err := readTimingReports(opts) assert.NilError(t, err) assert.Equal(t, len(files), 0) }) - - t.Run("prune older than max age", func(t *testing.T) { - opts := options{ - timingFilesPattern: dir.Join("*.log"), - pruneFilesMaxAgeDays: 2, - } - - files, err := readAndPruneTimingReports(opts) - assert.NilError(t, err) - defer closeFiles(files) - assert.Equal(t, len(files), 2) - - for _, fh := range files { - // check the files are properly seeked to 0 - event, err := parseEvent(fh) - assert.NilError(t, err) - assert.Equal(t, event.Package, "pkg0") - } - - actual, err := os.ReadDir(dir.Path()) - assert.NilError(t, err) - assert.Equal(t, len(actual), 2) - }) } func TestRun(t *testing.T) { @@ -279,42 +256,3 @@ func formatJSON(t *testing.T, v io.Reader) string { assert.NilError(t, err) return string(formatted) } - -func TestRun_PruneOnly(t *testing.T) { - events := func(t *testing.T, start time.Time) string { - t.Helper() - var buf bytes.Buffer - encoder := json.NewEncoder(&buf) - for _, i := range []int{0, 1, 2} { - assert.NilError(t, encoder.Encode(testjson.TestEvent{ - Time: start.Add(time.Duration(i) * time.Second), - Action: testjson.ActionRun, - Package: "pkg" + strconv.Itoa(i), - })) - } - return buf.String() - } - - now := time.Now() - dir := fs.NewDir(t, "timing-files", - fs.WithFile("report1.log", events(t, now.Add(-time.Hour))), - fs.WithFile("report2.log", events(t, now.Add(-47*time.Hour))), - fs.WithFile("report3.log", events(t, now.Add(-49*time.Hour))), - fs.WithFile("report4.log", events(t, now.Add(-101*time.Hour)))) - - stdout := new(bytes.Buffer) - opts := options{ - numPartitions: 3, - timingFilesPattern: dir.Join("*.log"), - debug: true, - stdout: stdout, - stdin: strings.NewReader(""), - pruneFilesMaxAgeDays: 2, - } - err := run(opts) - assert.NilError(t, err) - - assert.Assert(t, fs.Equal(dir.Path(), fs.Expected(t, - fs.WithFile("report1.log", events(t, now.Add(-time.Hour))), - fs.WithFile("report2.log", events(t, now.Add(-47*time.Hour)))))) -} From 5396f350139f70f2eef170744a700627b3a665a1 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sat, 26 Nov 2022 13:35:48 -0500 Subject: [PATCH 9/9] Update help text for new github output mechanism --- cmd/tool/matrix/matrix.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/tool/matrix/matrix.go b/cmd/tool/matrix/matrix.go index 0b7a4fbe..b5db7972 100644 --- a/cmd/tool/matrix/matrix.go +++ b/cmd/tool/matrix/matrix.go @@ -64,9 +64,8 @@ func usage(out io.Writer, name string, flags *pflag.FlagSet) { Read a list of packages from stdin and output a GitHub Actions matrix strategy that splits the packages by previous run times to minimize overall CI runtime. - echo -n "::set-output name=matrix::" - go list ./... | \ - %[1]s --timing-files ./*.log --partitions 4 + echo -n "matrix=" >> $GITHUB_OUTPUT + go list ./... | %[1]s --timing-files ./*.log --partitions 4 >> $GITHUB_OUTPUT The output of the command is a JSON object that can be used as the matrix strategy for a test job.