Merge pull request #262 from pixeleye-io/handing-test-rerunning

Moving duplicate errors to warnings

apps/backend/app/queries/snapshot/snapshot_setters_query.go

@@ -76,7 +76,7 @@ func (tx *SnapshotQueriesTx) BatchUpdateSnapshotStatus(ctx context.Context, snap
 	return err
 }
 
-func getDuplicateSnapError(snap models.Snapshot) string {
+func getDuplicateSnapWarning(snap models.Snapshot) string {
 	errTxt := "Duplicate snapshots with "
 
 	conflicting := []string{fmt.Sprintf("name: %s", snap.Name)}
@@ -97,6 +97,8 @@ func getDuplicateSnapError(snap models.Snapshot) string {
 		errTxt += utils.JoinStringsGrammatically(conflicting)
 	}
 
+	errTxt += " - this could be due to a test running multiple times or different snapshots with the same details"
+
 	return errTxt
 }
 
@@ -107,9 +109,9 @@ func getDuplicateSnapError(snap models.Snapshot) string {
 // Assumes we have no duplicate snapshots passed in
 func (q *SnapshotQueries) CreateBatchSnapshots(ctx context.Context, snapshots []models.Snapshot, build *models.Build) ([]models.Snapshot, bool, error) {
 	selectExistingSnapshotsQuery := `SELECT * FROM snapshot WHERE build_id = $1`
-	snapQuery := `INSERT INTO snapshot (id, build_id, name, variant, target, target_icon, viewport, created_at, updated_at, snap_image_id, status, error) VALUES (:id, :build_id, :name, :variant, :target, :target_icon, :viewport, :created_at, :updated_at, :snap_image_id, :status, :error)`
+	snapQuery := `INSERT INTO snapshot (id, build_id, name, variant, target, target_icon, viewport, created_at, updated_at, snap_image_id, status, error) VALUES (:id, :build_id, :name, :variant, :target, :target_icon, :viewport, :created_at, :updated_at, :snap_image_id, :status, :error) ON CONFLICT (build_id, name, variant, target, viewport) DO UPDATE SET updated_at = EXCLUDED.updated_at, snap_image_id = EXCLUDED.snap_image_id, status = EXCLUDED.status, error = EXCLUDED.error RETURNING *`
 
-	buildQueryAppend := `UPDATE build SET errors = array_append(errors, $1), status = 'failed', updated_at = $2 WHERE id = $3`
+	buildQueryAppend := `UPDATE build SET warnings = array_append(warnings, $1), updated_at = $2 WHERE id = $3`
 
 	if len(snapshots) == 0 {
 		return nil, false, echo.NewHTTPError(http.StatusBadRequest, "no snapshots to create")
@@ -127,7 +129,7 @@ func (q *SnapshotQueries) CreateBatchSnapshots(ctx context.Context, snapshots []
 
 	newSnapshots := []models.Snapshot{}
 
-	errors := pq.StringArray{}
+	warnings := pq.StringArray{}
 
 	// TODO move this to a separate function, so we can test it
 	for i, snap := range snapshots {
@@ -137,33 +139,34 @@ func (q *SnapshotQueries) CreateBatchSnapshots(ctx context.Context, snapshots []
 			snapAfter := snapshots[j]
 
 			if models.CompareSnaps(snap, snapAfter) {
+
 				isDup = true
-				errorTxt := getDuplicateSnapError(snap)
-				if !utils.ContainsString(build.Errors, errorTxt) || !utils.ContainsString(errors, errorTxt) {
-					// No need to update build if the error for this snapshot already exists.
-					errors = append(errors, errorTxt)
+				warningsTxt := getDuplicateSnapWarning(snap)
+				if !utils.ContainsString(build.Warnings, warningsTxt) || !utils.ContainsString(warnings, warningsTxt) {
+					// No need to update build if the warning for this snapshot already exists.
+					warnings = append(warnings, warningsTxt)
 				}
-				break // No need to check for anymore duplicates of this snapshot.
+				break
 			}
 		}
-		if isDup {
-			// No need to check for anymore duplicates. Continue to next snapshot.
-			isDup = false
-			continue
-		}
-		// Check there aren't any duplicates in the existing snapshots.
-		for _, existingSnapshot := range existingSnapshots {
-			if models.CompareSnaps(snap, existingSnapshot) {
-				isDup = true
-				errorTxt := getDuplicateSnapError(snap)
-				if !utils.ContainsString(build.Errors, errorTxt) || !utils.ContainsString(errors, errorTxt) {
-					// No need to update build if the error for this snapshot already exists.
-					errors = append(errors, errorTxt)
+		if !isDup {
+			// No need to check for any more duplicates as we've already found one.
+			// Check there aren't any duplicates in the existing snapshots.
+			for _, existingSnapshot := range existingSnapshots {
+				if models.CompareSnaps(snap, existingSnapshot) {
+					// We still want to update the snapshot even if it's a duplicate
+
+					warningTxt := getDuplicateSnapWarning(snap)
+					if !utils.ContainsString(build.Warnings, warningTxt) || !utils.ContainsString(warnings, warningTxt) {
+						// No need to update build if the warning for this snapshot already exists.
+						warnings = append(warnings, warningTxt)
+					}
 				}
 			}
 		}
 
 		if !isDup {
+
 			var err error
 			snap.ID, err = nanoid.New()
 			if err != nil {
@@ -181,7 +184,9 @@ func (q *SnapshotQueries) CreateBatchSnapshots(ctx context.Context, snapshots []
 
 			snap.BuildID = build.ID
 			newSnapshots = append(newSnapshots, snap)
+
 		}
+
 	}
 
 	if len(newSnapshots) == 0 {
@@ -199,17 +204,27 @@ func (q *SnapshotQueries) CreateBatchSnapshots(ctx context.Context, snapshots []
 		return nil, false, echo.NewHTTPError(http.StatusBadRequest, string(msg))
 	}
 
-	if len(errors) > 0 {
+	if len(warnings) > 0 {
 		build.UpdatedAt = utils.CurrentTime()
-		build.Errors = append(build.Errors, errors...)
-		if _, err := q.ExecContext(ctx, buildQueryAppend, errors, build.UpdatedAt, build.ID); err != nil {
+		build.Warnings = append(build.Warnings, warnings...)
+		if _, err := q.ExecContext(ctx, buildQueryAppend, warnings, build.UpdatedAt, build.ID); err != nil {
 			return nil, false, err
 		}
 	}
 
-	if _, err := q.NamedExecContext(ctx, snapQuery, newSnapshots); err != nil {
+	if rows, err := q.NamedQueryContext(ctx, snapQuery, newSnapshots); err != nil {
 		return nil, false, err
+	} else {
+		insertedSnapshots := []models.Snapshot{}
+		for rows.Next() {
+			var snap models.Snapshot
+			if err := rows.StructScan(&snap); err != nil {
+				return nil, false, err
+			}
+			insertedSnapshots = append(insertedSnapshots, snap)
+		}
+
+		return insertedSnapshots, false, nil
 	}
 
-	return newSnapshots, len(errors) > 0, nil
 }

apps/backend/platform/database/schema.hcl

@@ -923,8 +923,8 @@ table "snapshot" {
     null = false
   }
 
-  index "idx_snapshot-build_id__name__variant__target" {
-    columns = [column.build_id, column.name, column.variant, column.target]
+  index "idx_snapshot-build_id__name__variant__viewport__target" {
+    columns = [column.build_id, column.name, column.variant, column.viewport, column.target]
     unique  = true
   }
 

apps/backend/platform/migrations/20240403104028_adding_viewport_index.sql

@@ -0,0 +1,4 @@
+-- Drop index "idx_snapshot-build_id__name__variant__target" from table: "snapshot"
+DROP INDEX "public"."idx_snapshot-build_id__name__variant__target";
+-- Create index "idx_snapshot-build_id__name__variant__viewport__target" to table: "snapshot"
+CREATE UNIQUE INDEX "idx_snapshot-build_id__name__variant__viewport__target" ON "public"."snapshot" ("build_id", "name", "variant", "viewport", "target");

apps/backend/platform/migrations/atlas.sum

@@ -1,7 +1,8 @@
-h1:Zk1pfr8cTfulBkLHdlj/vZaBiQob2YDaSWni84DlmMQ=
+h1:uHQ7YbnoNqa2o85HxH7UE+nEAkRd6oh4gUbOo57VVhg=
 20240206184012_hello_world.sql h1:2tWJi8mxvFj98tUcj2MZ8L0+ttJqWs804g60ibJj4xM=
 20240215132932_updating_default_threshold_and_blur.sql h1:vezUttRRPbd6ee/AM7whpY6CE8eLnPTp6tyW5jgLQRA=
 20240218154121_adding_github_check_id.sql h1:K+IU3TWpIWtsLf0wHdmsfFDV4p6RMS5zgoiCxiGcWKQ=
 20240222235822_add_history_checks.sql h1:mshLBGWRymbuLgRjtsA1dONKiDBAlq2SiKvIlDsyiL0=
 20240226223846_adding_refferal_table.sql h1:YojH2CFCm7LYKWTQAVrqPSAzV6YMlpMmZpF09QZFOV0=
 20240308122338_build_sharding.sql h1:VgU4Sjku4WyQT7N438QPOd36P3GvIarcVHBVKQRceCM=
+20240403104028_adding_viewport_index.sql h1:NEKBWzsgSyT9TCArK3WNVQNF1uAyaOwws9XxIQDRLNo=