From 6e9a675b3f079f8cbb93024f49bc4b73af69b98d Mon Sep 17 00:00:00 2001 From: nielash Date: Sun, 24 Aug 2025 03:07:59 -0400 Subject: [PATCH] fstest: refactor to decouple package from implementation --- fstest/{test_all => runs}/config.go | 15 ++-- fstest/{test_all => runs}/report.go | 30 +++---- fstest/{test_all => runs}/run.go | 87 +++++++++++++++------ fstest/{test_all => runs}/run_test.go | 2 +- fstest/test_all/clean.go | 9 ++- fstest/test_all/test_all.go | 108 +++++++++++--------------- 6 files changed, 138 insertions(+), 113 deletions(-) rename fstest/{test_all => runs}/config.go (91%) rename fstest/{test_all => runs}/report.go (91%) rename fstest/{test_all => runs}/run.go (80%) rename fstest/{test_all => runs}/run_test.go (99%) diff --git a/fstest/test_all/config.go b/fstest/runs/config.go similarity index 91% rename from fstest/test_all/config.go rename to fstest/runs/config.go index cf299c300..82b19c8f3 100644 --- a/fstest/test_all/config.go +++ b/fstest/runs/config.go @@ -1,6 +1,7 @@ // Config handling -package main +// Package runs provides the types used by test_all +package runs import ( "fmt" @@ -141,10 +142,10 @@ func (c *Config) MakeRuns() (runs Runs) { return runs } -// Filter the Backends with the remotes passed in. +// FilterBackendsByRemotes filters the Backends with the remotes passed in. // // If no backend is found with a remote is found then synthesize one -func (c *Config) filterBackendsByRemotes(remotes []string) { +func (c *Config) FilterBackendsByRemotes(remotes []string) { var newBackends []Backend for _, name := range remotes { found := false @@ -167,8 +168,8 @@ func (c *Config) filterBackendsByRemotes(remotes []string) { c.Backends = newBackends } -// Filter the Backends with the backendNames passed in -func (c *Config) filterBackendsByBackends(backendNames []string) { +// FilterBackendsByBackends filters the Backends with the backendNames passed in +func (c *Config) FilterBackendsByBackends(backendNames []string) { var newBackends []Backend for _, name := range backendNames { for i := range c.Backends { @@ -180,8 +181,8 @@ func (c *Config) filterBackendsByBackends(backendNames []string) { c.Backends = newBackends } -// Filter the incoming tests into the backends selected -func (c *Config) filterTests(paths []string) { +// FilterTests filters the incoming tests into the backends selected +func (c *Config) FilterTests(paths []string) { var newTests []Test for _, path := range paths { for i := range c.Tests { diff --git a/fstest/test_all/report.go b/fstest/runs/report.go similarity index 91% rename from fstest/test_all/report.go rename to fstest/runs/report.go index c03e226bf..f3c7580aa 100644 --- a/fstest/test_all/report.go +++ b/fstest/runs/report.go @@ -1,4 +1,4 @@ -package main +package runs import ( "encoding/json" @@ -56,7 +56,7 @@ var parseVersion = regexp.MustCompile(`^v(?:[0-9.]+)-(?:\d+)-g([0-9a-f]+)(?:-(.* // FIXME take -issue or -pr parameter... // NewReport initialises and returns a Report -func NewReport() *Report { +func NewReport(Opt RunOpt) *Report { r := &Report{ StartTime: time.Now(), Version: fs.Version, @@ -67,20 +67,20 @@ func NewReport() *Report { r.DateTime = r.StartTime.Format(timeFormat) // Find previous log directory if possible - names, err := os.ReadDir(*outputDir) + names, err := os.ReadDir(Opt.OutputDir) if err == nil && len(names) > 0 { r.Previous = names[len(names)-1].Name() } // Create output directory for logs and report - r.LogDir = path.Join(*outputDir, r.DateTime) + r.LogDir = path.Join(Opt.OutputDir, r.DateTime) err = file.MkdirAll(r.LogDir, 0777) if err != nil { fs.Fatalf(nil, "Failed to make log directory: %v", err) } // Online version - r.URL = *urlBase + r.DateTime + "/index.html" + r.URL = Opt.URLBase + r.DateTime + "/index.html" // Get branch/commit out of version parts := parseVersion.FindStringSubmatch(r.Version) @@ -277,12 +277,12 @@ a:focus { var reportTemplate = template.Must(template.New("Report").Parse(reportHTML)) // EmailHTML sends the summary report to the email address supplied -func (r *Report) EmailHTML() { - if *emailReport == "" || r.IndexHTML == "" { +func (r *Report) EmailHTML(Opt RunOpt) { + if Opt.EmailReport == "" || r.IndexHTML == "" { return } - fs.Logf(nil, "Sending email summary to %q", *emailReport) - cmdLine := []string{"mail", "-a", "Content-Type: text/html", *emailReport, "-s", "rclone integration tests: " + r.Title()} + fs.Logf(nil, "Sending email summary to %q", Opt.EmailReport) + cmdLine := []string{"mail", "-a", "Content-Type: text/html", Opt.EmailReport, "-s", "rclone integration tests: " + r.Title()} cmd := exec.Command(cmdLine[0], cmdLine[1:]...) in, err := os.Open(r.IndexHTML) if err != nil { @@ -299,8 +299,8 @@ func (r *Report) EmailHTML() { } // uploadTo uploads a copy of the report online to the dir given -func (r *Report) uploadTo(uploadDir string) { - dst := path.Join(*uploadPath, uploadDir) +func (r *Report) uploadTo(Opt RunOpt, uploadDir string) { + dst := path.Join(Opt.UploadPath, uploadDir) fs.Logf(nil, "Uploading results to %q", dst) cmdLine := []string{"rclone", "sync", "--stats-log-level", "NOTICE", r.LogDir, dst} cmd := exec.Command(cmdLine[0], cmdLine[1:]...) @@ -313,12 +313,12 @@ func (r *Report) uploadTo(uploadDir string) { } // Upload uploads a copy of the report online -func (r *Report) Upload() { - if *uploadPath == "" || r.IndexHTML == "" { +func (r *Report) Upload(Opt RunOpt) { + if Opt.UploadPath == "" || r.IndexHTML == "" { return } // Upload into dated directory - r.uploadTo(r.DateTime) + r.uploadTo(Opt, r.DateTime) // And again into current - r.uploadTo("current") + r.uploadTo(Opt, "current") } diff --git a/fstest/test_all/run.go b/fstest/runs/run.go similarity index 80% rename from fstest/test_all/run.go rename to fstest/runs/run.go index ee1a13c98..2859e7392 100644 --- a/fstest/test_all/run.go +++ b/fstest/runs/run.go @@ -1,6 +1,6 @@ // Run a test -package main +package runs import ( "bytes" @@ -29,6 +29,27 @@ var ( oneOnly = map[string]*sync.Mutex{} ) +// RunOpt holds the options for the Run +type RunOpt struct { + MaxTries int // Number of times to try each test + MaxN int // Maximum number of tests to run at once + TestRemotes string // Comma separated list of remotes to test, e.g. 'TestSwift:,TestS3' + TestBackends string // Comma separated list of backends to test, e.g. 's3,googlecloudstorage + TestTests string // Comma separated list of tests to test, e.g. 'fs/sync,fs/operations' + Clean bool // Instead of testing, clean all left over test directories + RunOnly string // Run only those tests matching the regexp supplied + Timeout time.Duration // Maximum time to run each test for before giving up + Race bool // If set run the tests under the race detector + ConfigFile string // Path to config file + OutputDir string // Place to store results + EmailReport string // Set to email the report to the address supplied + DryRun bool // Print commands which would be executed only + URLBase string // Base for the online version + UploadPath string // Set this to an rclone path to upload the results here + Verbose bool // Set to enable verbose logging in the tests + ListRetries int // Number or times to retry listing - set to override the default +} + // Run holds info about a running test // // A run just runs one command line, but it can be run multiple times @@ -132,10 +153,10 @@ func match(current trie) []string { // This converts a slice of test names into a regexp which matches // them. func testsToRegexp(tests []string) string { - var split = trie{} + split := trie{} // Make a trie showing which parts are used at each level for _, test := range tests { - var parent = split + parent := split for _, name := range strings.Split(test, "/") { current := parent[name] if current == nil { @@ -172,7 +193,7 @@ func (r *Run) findFailures() { } } // Exclude the parents - var newTests = r.FailedTests[:0] + newTests := r.FailedTests[:0] for _, failedTest := range r.FailedTests { if _, excluded := excludeParents[failedTest]; !excluded { newTests = append(newTests, failedTest) @@ -210,10 +231,10 @@ func (r *Run) nextCmdLine() []string { } // trial runs a single test -func (r *Run) trial() { +func (r *Run) trial(Opt RunOpt) { CmdLine := r.nextCmdLine() CmdString := toShell(CmdLine) - msg := fmt.Sprintf("%q - Starting (try %d/%d)", CmdString, r.Try, *maxTries) + msg := fmt.Sprintf("%q - Starting (try %d/%d)", CmdString, r.Try, Opt.MaxTries) fs.Log(nil, msg) logName := path.Join(r.LogDir, r.TrialName) out, err := os.Create(logName) @@ -229,7 +250,7 @@ func (r *Run) trial() { _, _ = fmt.Fprintln(out, msg) // Early exit if --try-run - if *dryRun { + if Opt.DryRun { fs.Logf(nil, "Not executing as --dry-run: %v", CmdLine) _, _ = fmt.Fprintln(out, "--dry-run is set - not running") return @@ -260,9 +281,9 @@ func (r *Run) trial() { duration := time.Since(start) r.findFailures() if r.passed() { - msg = fmt.Sprintf("%q - Finished OK in %v (try %d/%d)", CmdString, duration, r.Try, *maxTries) + msg = fmt.Sprintf("%q - Finished OK in %v (try %d/%d)", CmdString, duration, r.Try, Opt.MaxTries) } else { - msg = fmt.Sprintf("%q - Finished ERROR in %v (try %d/%d): %v: Failed %v", CmdString, duration, r.Try, *maxTries, r.err, r.FailedTests) + msg = fmt.Sprintf("%q - Finished ERROR in %v (try %d/%d): %v: Failed %v", CmdString, duration, r.Try, Opt.MaxTries, r.err, r.FailedTests) } fs.Log(nil, msg) _, _ = fmt.Fprintln(out, msg) @@ -302,15 +323,15 @@ func (r *Run) PackagePath() string { } // MakeTestBinary makes the binary we will run -func (r *Run) MakeTestBinary() { +func (r *Run) MakeTestBinary(Opt RunOpt) { binary := r.BinaryPath() binaryName := r.BinaryName() fs.Logf(nil, "%s: Making test binary %q", r.Path, binaryName) CmdLine := []string{"go", "test", "-c"} - if *race { + if Opt.Race { CmdLine = append(CmdLine, "-race") } - if *dryRun { + if Opt.DryRun { fs.Logf(nil, "Not executing: %v", CmdLine) return } @@ -326,8 +347,8 @@ func (r *Run) MakeTestBinary() { } // RemoveTestBinary removes the binary made in makeTestBinary -func (r *Run) RemoveTestBinary() { - if *dryRun { +func (r *Run) RemoveTestBinary(Opt RunOpt) { + if Opt.DryRun { return } binary := r.BinaryPath() @@ -354,7 +375,7 @@ func (r *Run) Name() string { } // Init the Run -func (r *Run) Init() { +func (r *Run) Init(Opt RunOpt) { prefix := "-test." if r.NoBinary { prefix = "-" @@ -362,12 +383,12 @@ func (r *Run) Init() { } else { r.CmdLine = []string{"./" + r.BinaryName()} } - testTimeout := *timeout + testTimeout := Opt.Timeout if r.ExtraTime > 0 { testTimeout = time.Duration(float64(testTimeout) * r.ExtraTime) } r.CmdLine = append(r.CmdLine, prefix+"v", prefix+"timeout", testTimeout.String(), "-remote", r.Remote) - listRetries := *listRetries + listRetries := Opt.ListRetries if r.ListRetries > 0 { listRetries = r.ListRetries } @@ -376,12 +397,12 @@ func (r *Run) Init() { } r.Try = 1 ci := fs.GetConfig(context.Background()) - if *verbose { + if Opt.Verbose { r.CmdLine = append(r.CmdLine, "-verbose") ci.LogLevel = fs.LogLevelDebug } - if *runOnly != "" { - r.CmdLine = append(r.CmdLine, prefix+"run", *runOnly) + if Opt.RunOnly != "" { + r.CmdLine = append(r.CmdLine, prefix+"run", Opt.RunOnly) } if r.FastList { r.CmdLine = append(r.CmdLine, "-fast-list") @@ -412,7 +433,7 @@ func (r *Run) FailedTestsCSV() string { } // Run runs all the trials for this test -func (r *Run) Run(LogDir string, result chan<- *Run) { +func (r *Run) Run(Opt RunOpt, LogDir string, result chan<- *Run) { if r.OneOnly { oneOnlyMu.Lock() mu := oneOnly[r.Backend] @@ -424,13 +445,13 @@ func (r *Run) Run(LogDir string, result chan<- *Run) { mu.Lock() defer mu.Unlock() } - r.Init() + r.Init(Opt) r.LogDir = LogDir - for r.Try = 1; r.Try <= *maxTries; r.Try++ { + for r.Try = 1; r.Try <= Opt.MaxTries; r.Try++ { r.TrialName = r.Name() + ".txt" r.TrialNames = append(r.TrialNames, r.TrialName) fs.Logf(nil, "Starting run with log %q", r.TrialName) - r.trial() + r.trial(Opt) if r.passed() || r.NoRetries { break } @@ -440,3 +461,21 @@ func (r *Run) Run(LogDir string, result chan<- *Run) { } result <- r } + +// if matches then is definitely OK in the shell +var shellOK = regexp.MustCompile("^[A-Za-z0-9./_:-]+$") + +// converts an argv style input into a shell command +func toShell(args []string) (result string) { + for _, arg := range args { + if result != "" { + result += " " + } + if shellOK.MatchString(arg) { + result += arg + } else { + result += "'" + arg + "'" + } + } + return result +} diff --git a/fstest/test_all/run_test.go b/fstest/runs/run_test.go similarity index 99% rename from fstest/test_all/run_test.go rename to fstest/runs/run_test.go index 060074a6f..4353b1417 100644 --- a/fstest/test_all/run_test.go +++ b/fstest/runs/run_test.go @@ -1,4 +1,4 @@ -package main +package runs import ( "fmt" diff --git a/fstest/test_all/clean.go b/fstest/test_all/clean.go index 0e386dcaf..a6f52d199 100644 --- a/fstest/test_all/clean.go +++ b/fstest/test_all/clean.go @@ -11,6 +11,7 @@ import ( "github.com/rclone/rclone/fs/fspath" "github.com/rclone/rclone/fs/list" "github.com/rclone/rclone/fs/operations" + "github.com/rclone/rclone/fstest/runs" ) // MatchTestRemote matches the remote names used for testing (copied @@ -19,7 +20,7 @@ import ( var MatchTestRemote = regexp.MustCompile(`^rclone-test-[abcdefghijklmnopqrstuvwxyz0123456789]{12,24}(_segments)?$`) // cleanFs runs a single clean fs for left over directories -func cleanFs(ctx context.Context, remote string, cleanup bool) error { +func cleanFs(ctx context.Context, remote string, cleanup bool, Opt runs.RunOpt) error { f, err := fs.NewFs(context.Background(), remote) if err != nil { return err @@ -41,7 +42,7 @@ func cleanFs(ctx context.Context, remote string, cleanup bool) error { dirPath := dir.Remote() fullPath := fspath.JoinRootPath(remote, dirPath) if MatchTestRemote.MatchString(dirPath) { - if *dryRun { + if Opt.DryRun { fs.Logf(nil, "Not Purging %s - -dry-run", fullPath) return nil } @@ -70,12 +71,12 @@ func cleanFs(ctx context.Context, remote string, cleanup bool) error { } // cleanRemotes cleans the list of remotes passed in -func cleanRemotes(conf *Config) error { +func cleanRemotes(conf *runs.Config, Opt runs.RunOpt) error { var lastError error for _, backend := range conf.Backends { remote := backend.Remote fs.Logf(nil, "%q - Cleaning", remote) - err := cleanFs(context.Background(), remote, backend.CleanUp) + err := cleanFs(context.Background(), remote, backend.CleanUp, Opt) if err != nil { lastError = err fs.Logf(nil, "Failed to purge %q: %v", remote, err) diff --git a/fstest/test_all/test_all.go b/fstest/test_all/test_all.go index 07328387b..ae1c73ab6 100644 --- a/fstest/test_all/test_all.go +++ b/fstest/test_all/test_all.go @@ -17,58 +17,42 @@ import ( "math/rand" "os" "path" - "regexp" "strings" "time" _ "github.com/rclone/rclone/backend/all" // import all fs "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config/configfile" + "github.com/rclone/rclone/fstest/runs" "github.com/rclone/rclone/lib/pacer" ) -var ( +func init() { // Flags - maxTries = flag.Int("maxtries", 5, "Number of times to try each test") - maxN = flag.Int("n", 20, "Maximum number of tests to run at once") - testRemotes = flag.String("remotes", "", "Comma separated list of remotes to test, e.g. 'TestSwift:,TestS3'") - testBackends = flag.String("backends", "", "Comma separated list of backends to test, e.g. 's3,googlecloudstorage") - testTests = flag.String("tests", "", "Comma separated list of tests to test, e.g. 'fs/sync,fs/operations'") - clean = flag.Bool("clean", false, "Instead of testing, clean all left over test directories") - runOnly = flag.String("run", "", "Run only those tests matching the regexp supplied") - timeout = flag.Duration("timeout", 60*time.Minute, "Maximum time to run each test for before giving up") - race = flag.Bool("race", false, "If set run the tests under the race detector") - configFile = flag.String("config", "fstest/test_all/config.yaml", "Path to config file") - outputDir = flag.String("output", path.Join(os.TempDir(), "rclone-integration-tests"), "Place to store results") - emailReport = flag.String("email", "", "Set to email the report to the address supplied") - dryRun = flag.Bool("dry-run", false, "Print commands which would be executed only") - urlBase = flag.String("url-base", "https://pub.rclone.org/integration-tests/", "Base for the online version") - uploadPath = flag.String("upload", "", "Set this to an rclone path to upload the results here") - verbose = flag.Bool("verbose", false, "Set to enable verbose logging in the tests") - listRetries = flag.Int("list-retries", -1, "Number or times to retry listing - set to override the default") -) - -// if matches then is definitely OK in the shell -var shellOK = regexp.MustCompile("^[A-Za-z0-9./_:-]+$") - -// converts an argv style input into a shell command -func toShell(args []string) (result string) { - for _, arg := range args { - if result != "" { - result += " " - } - if shellOK.MatchString(arg) { - result += arg - } else { - result += "'" + arg + "'" - } - } - return result + flag.IntVar(&Opt.MaxTries, "maxtries", 5, "Number of times to try each test") + flag.IntVar(&Opt.MaxN, "n", 20, "Maximum number of tests to run at once") + flag.StringVar(&Opt.TestRemotes, "remotes", "", "Comma separated list of remotes to test, e.g. 'TestSwift:,TestS3'") + flag.StringVar(&Opt.TestBackends, "backends", "", "Comma separated list of backends to test, e.g. 's3,googlecloudstorage") + flag.StringVar(&Opt.TestTests, "tests", "", "Comma separated list of tests to test, e.g. 'fs/sync,fs/operations'") + flag.BoolVar(&Opt.Clean, "clean", false, "Instead of testing, clean all left over test directories") + flag.StringVar(&Opt.RunOnly, "run", "", "Run only those tests matching the regexp supplied") + flag.DurationVar(&Opt.Timeout, "timeout", 60*time.Minute, "Maximum time to run each test for before giving up") + flag.BoolVar(&Opt.Race, "race", false, "If set run the tests under the race detector") + flag.StringVar(&Opt.ConfigFile, "config", "fstest/test_all/config.yaml", "Path to config file") + flag.StringVar(&Opt.OutputDir, "output", path.Join(os.TempDir(), "rclone-integration-tests"), "Place to store results") + flag.StringVar(&Opt.EmailReport, "email", "", "Set to email the report to the address supplied") + flag.BoolVar(&Opt.DryRun, "dry-run", false, "Print commands which would be executed only") + flag.StringVar(&Opt.URLBase, "url-base", "https://pub.rclone.org/integration-tests/", "Base for the online version") + flag.StringVar(&Opt.UploadPath, "upload", "", "Set this to an rclone path to upload the results here") + flag.BoolVar(&Opt.Verbose, "verbose", false, "Set to enable verbose logging in the tests") + flag.IntVar(&Opt.ListRetries, "list-retries", -1, "Number or times to retry listing - set to override the default") } +var Opt = &runs.RunOpt{} + func main() { flag.Parse() - conf, err := NewConfig(*configFile) + conf, err := runs.NewConfig(Opt.ConfigFile) if err != nil { fs.Log(nil, "test_all should be run from the root of the rclone source code") fs.Fatal(nil, fmt.Sprint(err)) @@ -79,26 +63,26 @@ func main() { randInstance := rand.New(rand.NewSource(time.Now().UTC().UnixNano())) // Filter selection - if *testRemotes != "" { + if Opt.TestRemotes != "" { // CSV parse to support connection string remotes with commas like -remotes local,\"TestGoogleCloudStorage,directory_markers:\" - r := csv.NewReader(strings.NewReader(*testRemotes)) + r := csv.NewReader(strings.NewReader(Opt.TestRemotes)) remotes, err := r.Read() if err != nil { - fs.Fatalf(*testRemotes, "error CSV-parsing -remotes string: %v", err) + fs.Fatalf(Opt.TestRemotes, "error CSV-parsing -remotes string: %v", err) } - fs.Debugf(*testRemotes, "using remotes: %v", remotes) - conf.filterBackendsByRemotes(remotes) + fs.Debugf(Opt.TestRemotes, "using remotes: %v", remotes) + conf.FilterBackendsByRemotes(remotes) } - if *testBackends != "" { - conf.filterBackendsByBackends(strings.Split(*testBackends, ",")) + if Opt.TestBackends != "" { + conf.FilterBackendsByBackends(strings.Split(Opt.TestBackends, ",")) } - if *testTests != "" { - conf.filterTests(strings.Split(*testTests, ",")) + if Opt.TestTests != "" { + conf.FilterTests(strings.Split(Opt.TestTests, ",")) } // Just clean the directories if required - if *clean { - err := cleanRemotes(conf) + if Opt.Clean { + err := cleanRemotes(conf, *Opt) if err != nil { fs.Fatalf(nil, "Failed to clean: %v", err) } @@ -112,20 +96,20 @@ func main() { fs.Logf(nil, "Testing remotes: %s", strings.Join(names, ", ")) // Runs we will do for this test in random order - runs := conf.MakeRuns() - randInstance.Shuffle(len(runs), runs.Swap) + testRuns := conf.MakeRuns() + randInstance.Shuffle(len(testRuns), testRuns.Swap) // Create Report - report := NewReport() + report := runs.NewReport(*Opt) // Make the test binaries, one per Path found in the tests done := map[string]struct{}{} - for _, run := range runs { + for _, run := range testRuns { if _, found := done[run.Path]; !found { done[run.Path] = struct{}{} if !run.NoBinary { - run.MakeTestBinary() - defer run.RemoveTestBinary() + run.MakeTestBinary(*Opt) + defer run.RemoveTestBinary(*Opt) } } } @@ -134,14 +118,14 @@ func main() { _ = os.Setenv("RCLONE_CACHE_DB_WAIT_TIME", "30m") // start the tests - results := make(chan *Run, len(runs)) + results := make(chan *runs.Run, len(testRuns)) awaiting := 0 - tokens := pacer.NewTokenDispenser(*maxN) - for _, run := range runs { + tokens := pacer.NewTokenDispenser(Opt.MaxN) + for _, run := range testRuns { tokens.Get() - go func(run *Run) { + go func(run *runs.Run) { defer tokens.Put() - run.Run(report.LogDir, results) + run.Run(*Opt, report.LogDir, results) }(run) awaiting++ } @@ -157,8 +141,8 @@ func main() { report.LogSummary() report.LogJSON() report.LogHTML() - report.EmailHTML() - report.Upload() + report.EmailHTML(*Opt) + report.Upload(*Opt) if !report.AllPassed() { os.Exit(1) }