chore: Refine generating JSON&JUint report (#812)

* chore: Refine generating JSON&JUint report * cleanup empty values * refine comments * RDC doesn't need to complete artifact filePath * Update internal/saucecloud/cloud.go Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com> * Klotski * fix typo * remove removed function * oops * cleanup * rename * make some fields emptyable * add comments for cleanup function * Add unknown artifact type * refine function comment * Update internal/report/json/json.go Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com> * Update internal/saucecloud/cloud.go Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com> * re-format and remove JSONArtifact * Update internal/http/resto.go Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com> * fix ppl --------- Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>
saucelabs · Jul 28, 2023 · c18eaf8 · c18eaf8
1 parent ed49827
commit c18eaf8
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 46 deletions.
diff --git a/internal/http/resto.go b/internal/http/resto.go
@@ -361,7 +361,7 @@ func (c *Resto) DownloadArtifact(jobID, suiteName string, realDevice bool) []str
 	for _, f := range files {
 		for _, pattern := range c.ArtifactConfig.Match {
 			if glob.Glob(pattern, f) {
-				artifacts = append(artifacts, f)
+				artifacts = append(artifacts, filepath.Join(targetDir, f))
 				if err := c.downloadArtifact(targetDir, jobID, f); err != nil {
 					log.Error().Err(err).Msgf("Failed to download file: %s", f)
 				}

diff --git a/internal/report/json/json.go b/internal/report/json/json.go
@@ -25,6 +25,7 @@ func (r *Reporter) Add(t report.TestResult) {
 
 // Render sends the result to specified webhook WebhookURL and log the result to the specified json file
 func (r *Reporter) Render() {
+	r.cleanup()
 	body, err := json.Marshal(r.Results)
 	if err != nil {
 		log.Error().Msgf("failed to generate test result (%v)", err)
@@ -54,12 +55,27 @@ func (r *Reporter) Render() {
 	}
 }
 
+// cleanup removes any information that isn't relevant in the rendered report. Particularly when it comes to
+// artifacts, this reporter is only interested in those that have been persisted to the file system.
+func (r *Reporter) cleanup() {
+	for i, result := range r.Results {
+		var artifacts []report.Artifact
+		for _, a := range result.Artifacts {
+			if a.FilePath == "" {
+				continue
+			}
+			artifacts = append(artifacts, a)
+		}
+		r.Results[i].Artifacts = artifacts
+	}
+}
+
 // Reset resets the reporter to its initial state. This action will delete all test results.
 func (r *Reporter) Reset() {
 	r.Results = make([]report.TestResult, 0)
 }
 
 // ArtifactRequirements returns a list of artifact types this reporter requires to create a proper report.
 func (r *Reporter) ArtifactRequirements() []report.ArtifactType {
-	return []report.ArtifactType{report.JSONArtifact}
+	return nil
 }
diff --git a/internal/report/report.go b/internal/report/report.go
@@ -9,27 +9,27 @@ type TestResult struct {
 	StartTime     time.Time     `json:"startTime"`
 	EndTime       time.Time     `json:"endTime"`
 	Status        string        `json:"status"`
-	Browser       string        `json:"browser"`
+	Browser       string        `json:"browser,omitempty"`
 	Platform      string        `json:"platform"`
-	DeviceName    string        `json:"deviceName"`
+	DeviceName    string        `json:"deviceName,omitempty"`
 	URL           string        `json:"url"`
-	Artifacts     []Artifact    `json:"artifacts"`
+	Artifacts     []Artifact    `json:"artifacts,omitempty"`
 	Origin        string        `json:"origin"`
 	Attempts      int           `json:"attempts"`
 	RDC           bool          `json:"-"`
 	TimedOut      bool          `json:"-"`
-	PassThreshold bool          `json:"passThreshold"`
+	PassThreshold bool          `json:"-"`
 	ParentJUnits  []Artifact    `json:"-"`
 }
 
 // ArtifactType represents the type of assets (e.g. a junit report). Semantically similar to Content-Type.
 type ArtifactType int
 
 const (
+	// Unknown represents an asset with an unknown purpose that isn't further defined.
+	Unknown ArtifactType = iota
 	// JUnitArtifact represents the junit artifact type (https://llg.cubic.org/docs/junit/).
-	JUnitArtifact ArtifactType = iota
-	// JSONArtifact represents the json artifact type
-	JSONArtifact
+	JUnitArtifact
 )
 
 // Artifact represents an artifact (aka asset) that was generated as part of a job.

diff --git a/internal/saucecloud/cloud.go b/internal/saucecloud/cloud.go
@@ -103,9 +103,6 @@ func (r *CloudRunner) collectResults(artifactCfg config.ArtifactDownload, result
 	inProgress := expected
 	passed := true
 
-	junitRequired := report.IsArtifactRequired(r.Reporters, report.JUnitArtifact)
-	jsonResultRequired := report.IsArtifactRequired(r.Reporters, report.JSONArtifact)
-
 	done := make(chan interface{})
 	go func(r *CloudRunner) {
 		t := time.NewTicker(10 * time.Second)
@@ -146,32 +143,14 @@ func (r *CloudRunner) collectResults(artifactCfg config.ArtifactDownload, result
 			}
 
 			var artifacts []report.Artifact
-			var parentJUnits []report.Artifact
-
-			if junitRequired {
-				jb, err := r.JobService.GetJobAssetFileContent(
-					context.Background(),
-					res.job.ID,
-					junit.JunitFileName,
-					res.job.IsRDC)
+			junitArtifact, parentJUnits := r.GetJUnitArtifacts(res)
+			artifacts = append(artifacts, junitArtifact)
+
+			files := r.downloadArtifacts(res.name, res.job, artifactCfg.When)
+			for _, f := range files {
 				artifacts = append(artifacts, report.Artifact{
-					AssetType: report.JUnitArtifact,
-					Body:      jb,
-					Error:     err,
+					FilePath: f,
 				})
-				for _, id := range res.previous {
-					jb, err := r.JobService.GetJobAssetFileContent(
-						context.Background(),
-						id,
-						junit.JunitFileName,
-						res.job.IsRDC,
-					)
-					parentJUnits = append(parentJUnits, report.Artifact{
-						AssetType: report.JUnitArtifact,
-						Body:      jb,
-						Error:     err,
-					})
-				}
 			}
 
 			var url string
@@ -195,16 +174,6 @@ func (r *CloudRunner) collectResults(artifactCfg config.ArtifactDownload, result
 				TimedOut:     res.job.TimedOut,
 				ParentJUnits: parentJUnits,
 			}
-
-			files := r.downloadArtifacts(res.name, res.job, artifactCfg.When)
-			if jsonResultRequired {
-				for _, f := range files {
-					artifacts = append(artifacts, report.Artifact{
-						FilePath: f,
-					})
-				}
-			}
-
 			for _, rep := range r.Reporters {
 				rep.Add(tr)
 			}
@@ -507,6 +476,39 @@ func (r *CloudRunner) remoteArchiveFiles(project interface{}, files []string, sa
 	return strings.Join(uris, ","), nil
 }
 
+// GetJUnitArtifacts retrieves the JUnit report and parent JUnit reports for the current job.
+func (r *CloudRunner) GetJUnitArtifacts(res result) (junitArifact report.Artifact, parentJUnits []report.Artifact) {
+	if !report.IsArtifactRequired(r.Reporters, report.JUnitArtifact) {
+		return
+	}
+
+	content, err := r.JobService.GetJobAssetFileContent(
+		context.Background(),
+		res.job.ID,
+		junit.JunitFileName,
+		res.job.IsRDC)
+	junitArifact = report.Artifact{
+		AssetType: report.JUnitArtifact,
+		Body:      content,
+		Error:     err,
+	}
+
+	for _, id := range res.previous {
+		content, err := r.JobService.GetJobAssetFileContent(
+			context.Background(),
+			id,
+			junit.JunitFileName,
+			res.job.IsRDC,
+		)
+		parentJUnits = append(parentJUnits, report.Artifact{
+			AssetType: report.JUnitArtifact,
+			Body:      content,
+			Error:     err,
+		})
+	}
+	return
+}
+
 type uploadType string
 
 var (