TRT-2924: Component Readiness cell status is non-deterministic when mixing MissingBasis/MissingSample with NotSignificant tests

pkg/api/componentreadiness/component_report.go

@@ -455,9 +455,7 @@ type cellStatus struct {
 func getNewCellStatus(testID crtest.Identification, testStats testdetails.TestComparison, existingCellStatus *cellStatus, includeAllTests bool) cellStatus {
 	var newCellStatus cellStatus
 	if existingCellStatus != nil {
-		if (testStats.ReportStatus < crtest.NotSignificant && testStats.ReportStatus < existingCellStatus.status) ||
-			(existingCellStatus.status == crtest.NotSignificant && testStats.ReportStatus == crtest.SignificantImprovement) {
-			// We want to show the significant improvement if assessment is not regression
+		if crtest.CompareCellStatus(testStats.ReportStatus, existingCellStatus.status) < 0 {
 			newCellStatus.status = testStats.ReportStatus
 		} else {
 			newCellStatus.status = existingCellStatus.status

pkg/api/componentreadiness/component_report_test.go

@@ -1869,5 +1869,56 @@ func Test_componentReportGenerator_assessComponentStatus(t *testing.T) {
 	}
 }
 
+func TestGetNewCellStatusOrderIndependent(t *testing.T) {
+	testID := crtest.Identification{
+		RowIdentification: crtest.RowIdentification{
+			Component:  "component",
+			Capability: "cap",
+			TestName:   "test",
+		},
+	}
+	makeStats := func(s crtest.Status) testdetails.TestComparison {
+		return testdetails.TestComparison{ReportStatus: s}
+	}
+	statusForOrder := func(first, second crtest.Status) crtest.Status {
+		cell := getNewCellStatus(testID, makeStats(first), nil, false)
+		cell = getNewCellStatus(testID, makeStats(second), &cell, false)
+		return cell.status
+	}
+
+	tests := []struct {
+		name   string
+		first  crtest.Status
+		second crtest.Status
+		want   crtest.Status
+	}{
+		{
+			name:   "NotSignificant and MissingBasis",
+			first:  crtest.NotSignificant,
+			second: crtest.MissingBasis,
+			want:   crtest.NotSignificant,
+		},
+		{
+			name:   "NotSignificant and MissingSample",
+			first:  crtest.NotSignificant,
+			second: crtest.MissingSample,
+			want:   crtest.MissingSample,
+		},
+		{
+			name:   "NotSignificant and SignificantImprovement",
+			first:  crtest.NotSignificant,
+			second: crtest.SignificantImprovement,
+			want:   crtest.SignificantImprovement,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, statusForOrder(tt.first, tt.second))
+			assert.Equal(t, tt.want, statusForOrder(tt.second, tt.first))
+		})
+	}
+}
+
 // TestCopyIncludeVariantsAndRemoveOverrides moved to dataprovider/bigquery package
 // where the function now lives.

pkg/apis/api/componentreport/crtest/types.go

@@ -1,6 +1,7 @@
 package crtest
 
 import (
+	"cmp"
 	"sort"
 	"strings"
 	"time"
@@ -63,6 +64,25 @@ const (
 	SignificantImprovement Status = 300
 )
 
+// CompareCellStatus compares two statuses for aggregation into a Component
+// Readiness cell. It returns a negative value when first wins cell aggregation,
+// zero when the statuses have equal precedence, and a positive value when
+// second wins cell aggregation. Lower raw status values win, except that
+// SignificantImprovement wins over every other status greater than or equal to
+// NotSignificant.
+func CompareCellStatus(first, second Status) int {
+	if first == second {
+		return 0
+	}
+	if first == SignificantImprovement && second >= NotSignificant {
+		return -1
+	}
+	if second == SignificantImprovement && first >= NotSignificant {
+		return 1
+	}
+	return cmp.Compare(first, second)
+}
+
 func StringForStatus(s Status) string {
 	switch s {
 	case ExtremeRegression:

pkg/apis/api/componentreport/crtest/types_test.go

@@ -108,6 +108,64 @@ func TestVariantRoundTrip(t *testing.T) {
 	assert.Equal(t, value, gotValue)
 }
 
+func TestCompareCellStatus(t *testing.T) {
+	tests := []struct {
+		name   string
+		first  Status
+		second Status
+		want   int
+	}{
+		{
+			name:   "equal improvements have equal precedence",
+			first:  SignificantImprovement,
+			second: SignificantImprovement,
+			want:   0,
+		},
+		{
+			name:   "ordinary lower status wins",
+			first:  SignificantRegression,
+			second: NotSignificant,
+			want:   -1,
+		},
+		{
+			name:   "arbitrary negative status follows raw ordering",
+			first:  Status(-9999),
+			second: FailedFixedRegression,
+			want:   -1,
+		},
+		{
+			name:   "arbitrary positive status follows raw ordering",
+			first:  Status(50),
+			second: MissingBasis,
+			want:   -1,
+		},
+		{
+			name:   "improvement does not override a negative status",
+			first:  SignificantImprovement,
+			second: Status(-1),
+			want:   1,
+		},
+		{
+			name:   "improvement wins at NotSignificant boundary",
+			first:  SignificantImprovement,
+			second: NotSignificant,
+			want:   -1,
+		},
+		{
+			name:   "improvement wins as second status above boundary",
+			first:  Status(1),
+			second: SignificantImprovement,
+			want:   1,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, CompareCellStatus(tt.first, tt.second))
+		})
+	}
+}
+
 func TestColumnEncodeEmpty(t *testing.T) {
 	col := ColumnIdentification{Variants: map[string]string{}}
 	encoded := col.Encode()