feat: Add option to download matching artifacts for all job attempts (#…

…937) * Download based on attempt not last jobData * Download artifacts after an attempt * Remove unneeded properties * Add artifacts to result * Have downloader handle all logic * cleanup * Cleanup args list * Cleanup * fixup test mocks * update test * update schema * lint * add option to other schema * add cli flag for allAttempts * Update internal/saucecloud/downloader/downloader.go Co-authored-by: Alex Plischke <[email protected]> * fixup test * lint --------- Co-authored-by: Alex Plischke <[email protected]>
saucelabs · Aug 16, 2024 · f00fcc8 · f00fcc8
1 parent 04e0196
commit f00fcc8
Show file tree

Hide file tree

Showing 13 changed files with 88 additions and 37 deletions.
diff --git a/api/saucectl.schema.json b/api/saucectl.schema.json
@@ -70,6 +70,10 @@
                       "directory": {
                         "description": "Specifies the path to the folder in which to download artifacts. A separate subdirectory is generated in this location for each suite.",
                         "type": "string"
+                      },
+                      "allAttempts": {
+                        "description": "If true and a test is retried, artifacts for every attempt will be downloaded. Otherwise, only artifacts for the final attempt will be downloaded.",
+                        "type": "boolean"
                       }
                     },
                     "required": [
@@ -725,6 +729,10 @@
                       "directory": {
                         "description": "Specifies the path to the folder in which to download artifacts. A separate subdirectory is generated in this location for each suite.",
                         "type": "string"
+                      },
+                      "allAttempts": {
+                        "description": "If true and a test is retried, artifacts for every attempt will be downloaded. Otherwise, only artifacts for the final attempt will be downloaded.",
+                        "type": "boolean"
                       }
                     },
                     "required": [

diff --git a/api/v1/subschema/artifacts.schema.json b/api/v1/subschema/artifacts.schema.json
@@ -32,6 +32,10 @@
             "directory": {
               "description": "Specifies the path to the folder in which to download artifacts. A separate subdirectory is generated in this location for each suite.",
               "type": "string"
+            },
+            "allAttempts": {
+              "description": "If true and a test is retried, artifacts for every attempt will be downloaded. Otherwise, only artifacts for the final attempt will be downloaded.",
+              "type": "boolean"
             }
           },
           "required": [

diff --git a/api/v1alpha/subschema/artifacts.schema.json b/api/v1alpha/subschema/artifacts.schema.json
@@ -32,6 +32,10 @@
             "directory": {
               "description": "Specifies the path to the folder in which to download artifacts. A separate subdirectory is generated in this location for each suite.",
               "type": "string"
+            },
+            "allAttempts": {
+              "description": "If true and a test is retried, artifacts for every attempt will be downloaded. Otherwise, only artifacts for the final attempt will be downloaded.",
+              "type": "boolean"
             }
           },
           "required": [

diff --git a/internal/cmd/run/run.go b/internal/cmd/run/run.go
@@ -128,6 +128,7 @@ func Command() *cobra.Command {
 	sc.String("artifacts.download.when", "artifacts::download::when", "never", "Specifies when to download test artifacts")
 	sc.StringSlice("artifacts.download.match", "artifacts::download::match", []string{}, "Specifies which test artifacts to download")
 	sc.String("artifacts.download.directory", "artifacts::download::directory", "", "Specifies the location where to download test artifacts to")
+	sc.Bool("artifacts.download.allAttempts", "artifacts::download::allAttempts", false, "Specifies whether to download artifacts for all attempted tests if a test needs to be retried. If false, only the artifacts of the last attempt will be downloaded.")
 	sc.Bool("artifacts.cleanup", "artifacts::cleanup", false, "Specifies whether to remove all contents of artifacts directory")
 
 	// Reporters

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -124,9 +124,10 @@ func (w When) IsNow(passed bool) bool {
 
 // ArtifactDownload represents the test artifacts configuration.
 type ArtifactDownload struct {
-	Match     []string `yaml:"match,omitempty" json:"match"`
-	When      When     `yaml:"when,omitempty" json:"when"`
-	Directory string   `yaml:"directory,omitempty" json:"directory"`
+	Match       []string `yaml:"match,omitempty" json:"match"`
+	When        When     `yaml:"when,omitempty" json:"when"`
+	Directory   string   `yaml:"directory,omitempty" json:"directory"`
+	AllAttempts bool     `yaml:"allAttempts,omitempty" json:"allAttempts"`
 }
 
 // Notifications represents the test notifications configuration.

diff --git a/internal/job/job.go b/internal/job/job.go
@@ -93,5 +93,5 @@ type Service interface {
 // ArtifactDownloader represents the interface for downloading artifacts.
 type ArtifactDownloader interface {
 	// DownloadArtifact downloads artifacts and returns a list of what was downloaded.
-	DownloadArtifact(jobID, suiteName string, realDevice bool) []string
+	DownloadArtifact(job Job, attempt int, retries int) []string
 }
diff --git a/internal/mocks/download.go b/internal/mocks/download.go
@@ -1,11 +1,13 @@
 package mocks
 
+import "github.com/saucelabs/saucectl/internal/job"
+
 // FakeArtifactDownloader defines a fake Downloader
 type FakeArtifactDownloader struct {
-	DownloadArtifactFn func(jobID, suiteName string) []string
+	DownloadArtifactFn func(jobData job.Job, attempt int, retries int) []string
 }
 
 // DownloadArtifact defines a fake function for FakeDownloader
-func (f *FakeArtifactDownloader) DownloadArtifact(jobID, suiteName string, realDevice bool) []string {
-	return f.DownloadArtifactFn(jobID, suiteName)
+func (f *FakeArtifactDownloader) DownloadArtifact(jobData job.Job, attempt int, retries int) []string {
+	return f.DownloadArtifactFn(jobData, attempt, retries)
 }
diff --git a/internal/saucecloud/cloud.go b/internal/saucecloud/cloud.go
@@ -83,7 +83,8 @@ type result struct {
 	retries   int
 	attempts  []report.Attempt
 
-	details insights.Details
+	details   insights.Details
+	artifacts []report.Artifact
 }
 
 // ConsoleLogAsset represents job asset log file name.
@@ -146,15 +147,7 @@ func (r *CloudRunner) collectResults(artifactCfg config.ArtifactDownload, result
 				browser = fmt.Sprintf("%s %s", browser, res.job.BrowserVersion)
 			}
 
-			var artifacts []report.Artifact
-			files := r.downloadArtifacts(res.name, res.job, artifactCfg.When)
-			for _, f := range files {
-				artifacts = append(artifacts, report.Artifact{
-					FilePath: f,
-				})
-			}
-
-			r.FetchJUnitReports(&res, artifacts)
+			r.FetchJUnitReports(&res, res.artifacts)
 
 			var url string
 			if res.job.ID != "" {
@@ -171,7 +164,7 @@ func (r *CloudRunner) collectResults(artifactCfg config.ArtifactDownload, result
 				Platform:   platform,
 				DeviceName: res.job.DeviceName,
 				URL:        url,
-				Artifacts:  artifacts,
+				Artifacts:  res.artifacts,
 				Origin:     "sauce",
 				RDC:        res.job.IsRDC,
 				TimedOut:   res.job.TimedOut,
@@ -341,6 +334,7 @@ func (r *CloudRunner) runJobs(jobOpts chan job.StartOptions, results chan<- resu
 		}
 
 		if opts.Attempt < opts.Retries && ((!jobData.Passed && !skipped) || (opts.CurrentPassCount < opts.PassThreshold)) {
+			go r.JobService.DownloadArtifact(jobData, opts.Attempt, opts.Retries)
 			if !jobData.Passed {
 				log.Warn().Err(err).Msg("Suite errored.")
 			}
@@ -355,6 +349,7 @@ func (r *CloudRunner) runJobs(jobOpts chan job.StartOptions, results chan<- resu
 				TestSuites: junit.TestSuites{},
 			})
 			go r.Retrier.Retry(jobOpts, opts, jobData)
+
 			continue
 		}
 
@@ -375,6 +370,14 @@ func (r *CloudRunner) runJobs(jobOpts chan job.StartOptions, results chan<- resu
 			}
 		}
 
+		files := r.JobService.DownloadArtifact(jobData, opts.Attempt, opts.Retries)
+		var artifacts []report.Artifact
+		for _, f := range files {
+			artifacts = append(artifacts, report.Artifact{
+				FilePath: f,
+			})
+		}
+
 		results <- result{
 			name:      opts.DisplayName,
 			browser:   opts.BrowserName,
@@ -393,6 +396,7 @@ func (r *CloudRunner) runJobs(jobOpts chan job.StartOptions, results chan<- resu
 				EndTime:   time.Now(),
 				Status:    jobData.Status,
 			}),
+			artifacts: artifacts,
 		}
 	}
 }
@@ -999,14 +1003,6 @@ func (r *CloudRunner) loadJUnitReport(jobID string, isRDC bool) (junit.TestSuite
 	return junit.Parse(fileContent)
 }
 
-func (r *CloudRunner) downloadArtifacts(suiteName string, job job.Job, when config.When) []string {
-	if job.ID == "" || job.TimedOut || r.Async || !when.IsNow(job.Passed) {
-		return []string{}
-	}
-
-	return r.JobService.DownloadArtifact(job.ID, suiteName, job.IsRDC)
-}
-
 func arrayContains(list []string, want string) bool {
 	for _, item := range list {
 		if item == want {

diff --git a/internal/saucecloud/cloud_test.go b/internal/saucecloud/cloud_test.go
@@ -166,6 +166,12 @@ func TestRunJobTimeout(t *testing.T) {
 			VDCWriter: &mocks.FakeJobWriter{UploadAssetFn: func(jobID string, fileName string, contentType string, content []byte) error {
 				return nil
 			}},
+			VDCDownloader: &mocks.FakeArtifactDownloader{DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
+				return []string{}
+			}},
+			RDCDownloader: &mocks.FakeArtifactDownloader{DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
+				return []string{}
+			}},
 		},
 	}
 
@@ -221,6 +227,12 @@ func TestRunJobRetries(t *testing.T) {
 				VDCWriter: &mocks.FakeJobWriter{UploadAssetFn: func(jobID string, fileName string, contentType string, content []byte) error {
 					return nil
 				}},
+				VDCDownloader: &mocks.FakeArtifactDownloader{DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
+					return []string{}
+				}},
+				RDCDownloader: &mocks.FakeArtifactDownloader{DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
+					return []string{}
+				}},
 			},
 		}
 
@@ -257,6 +269,12 @@ func TestRunJobTimeoutRDC(t *testing.T) {
 					return job.Job{ID: id, TimedOut: true}, nil
 				},
 			},
+			VDCDownloader: &mocks.FakeArtifactDownloader{DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
+				return []string{}
+			}},
+			RDCDownloader: &mocks.FakeArtifactDownloader{DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
+				return []string{}
+			}},
 		},
 	}
 

diff --git a/internal/saucecloud/cypress_test.go b/internal/saucecloud/cypress_test.go
@@ -78,7 +78,7 @@ func TestRunSuites(t *testing.T) {
 					},
 				},
 				VDCDownloader: &mocks.FakeArtifactDownloader{
-					DownloadArtifactFn: func(jobID string, suiteName string) []string {
+					DownloadArtifactFn: func(jobData job.Job, attempt int, retries int) []string {
 						return []string{}
 					},
 				},

diff --git a/internal/saucecloud/downloader/downloader.go b/internal/saucecloud/downloader/downloader.go
@@ -23,22 +23,31 @@ func NewArtifactDownloader(reader job.Reader, artifactConfig config.ArtifactDown
 	}
 }
 
-func (d *ArtifactDownloader) DownloadArtifact(jobID string, suiteName string, realDevice bool) []string {
-	targetDir, err := config.GetSuiteArtifactFolder(suiteName, d.config)
+func (d *ArtifactDownloader) DownloadArtifact(jobData job.Job, attemptNumber int, retries int) []string {
+	if jobData.ID == "" ||
+		jobData.TimedOut || !job.Done(jobData.Status) ||
+		!d.config.When.IsNow(jobData.Passed) ||
+		(!d.config.AllAttempts && attemptNumber < retries) {
+		return []string{}
+	}
+
+	destDir, err := config.GetSuiteArtifactFolder(jobData.Name, d.config)
 	if err != nil {
 		log.Error().Msgf("Unable to create artifacts folder (%v)", err)
 		return []string{}
 	}
-	files, err := d.reader.GetJobAssetFileNames(context.Background(), jobID, realDevice)
+
+	files, err := d.reader.GetJobAssetFileNames(context.Background(), jobData.ID, jobData.IsRDC)
 	if err != nil {
 		log.Error().Msgf("Unable to fetch artifacts list (%v)", err)
 		return []string{}
 	}
 
 	filepaths := fpath.MatchFiles(files, d.config.Match)
 	var artifacts []string
+
 	for _, f := range filepaths {
-		targetFile, err := d.downloadArtifact(targetDir, jobID, f, realDevice)
+		targetFile, err := d.downloadArtifact(destDir, jobData.ID, f, jobData.IsRDC)
 		if err != nil {
 			log.Err(err).Msg("Unable to download artifacts")
 			return artifacts

diff --git a/internal/saucecloud/downloader/downloader_test.go b/internal/saucecloud/downloader/downloader_test.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/saucelabs/saucectl/internal/config"
 	httpServices "github.com/saucelabs/saucectl/internal/http"
+	"github.com/saucelabs/saucectl/internal/job"
 )
 
 func TestArtifactDownloader_DownloadArtifact(t *testing.T) {
@@ -43,10 +44,17 @@ func TestArtifactDownloader_DownloadArtifact(t *testing.T) {
 	artifactCfg := config.ArtifactDownload{
 		Directory: tempDir,
 		Match:     []string{"junit.xml"},
+		When:      config.WhenAlways,
 	}
-
 	downloader := NewArtifactDownloader(&rc, artifactCfg)
-	downloader.DownloadArtifact("test-123", "suite name", true)
+	downloader.DownloadArtifact(
+		job.Job{
+			ID:     "test-123",
+			Name:   "suite name",
+			IsRDC:  true,
+			Status: job.StateComplete,
+		}, 0, 0,
+	)
 
 	fileName := filepath.Join(tempDir, "suite_name", "junit.xml")
 	d, err := os.ReadFile(fileName)

diff --git a/internal/saucecloud/jobservice.go b/internal/saucecloud/jobservice.go
@@ -23,12 +23,12 @@ type JobService struct {
 	RDCDownloader job.ArtifactDownloader
 }
 
-func (s JobService) DownloadArtifact(jobID, suiteName string, realDevice bool) []string {
-	if realDevice {
-		return s.RDCDownloader.DownloadArtifact(jobID, suiteName, realDevice)
+func (s JobService) DownloadArtifact(jobData job.Job, attemptNumber int, retries int) []string {
+	if jobData.IsRDC {
+		return s.RDCDownloader.DownloadArtifact(jobData, attemptNumber, retries)
 	}
 
-	return s.VDCDownloader.DownloadArtifact(jobID, suiteName, realDevice)
+	return s.VDCDownloader.DownloadArtifact(jobData, attemptNumber, retries)
 }
 
 func (s JobService) StopJob(ctx context.Context, jobID string, realDevice bool) (job.Job, error) {