Skip to content

Commit b3abc73

Browse files
Merge pull request #2781 from dgoodwin/master
Allow JobTier=candidate in featuregate-test-analyzer with warning
2 parents aa86301 + 429b507 commit b3abc73

3 files changed

Lines changed: 216 additions & 5 deletions

File tree

tools/codegen/cmd/featuregate-test-analyzer.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ func (o *FeatureGateTestAnalyzerOptions) Run(ctx context.Context) error {
217217
md.Textf("* Tests must be be run on every TechPreview platform (ask for an exception if your feature doesn't support a variant)")
218218
md.Textf("* All tests must run at least 14 times on every platform")
219219
md.Textf("* All tests must pass at least 95%% of the time")
220-
md.Textf("* JobTier must be one of: standard, informing, blocking\n")
220+
md.Textf("* JobTier must be one of: standard, informing, blocking, candidate (candidate is allowed but produces a warning as it is not covered by Component Readiness)\n")
221221
md.Text("")
222222

223223
if len(warnings) > 0 {
@@ -375,6 +375,18 @@ func checkIfTestingIsSufficient(featureGate string, testingResults map[JobVarian
375375
// Optional variants (like RHEL 10 in 4.22) have non-blocking warnings
376376
isOptional := jobVariant.Optional
377377

378+
// If candidate-tier queries returned results for this variant, emit a warning.
379+
// Candidate tier jobs are not covered by the Component Readiness main view and
380+
// do not have our standard regression protection mechanisms. The results are still
381+
// included in the pass/fail calculation alongside other tiers.
382+
if testedVariant.HasCandidateTierResults {
383+
results = append(results, ValidationResult{
384+
Error: fmt.Errorf("warning: variant %v includes test data from candidate-tier jobs which are not covered by Component Readiness and lack standard regression protection",
385+
jobVariant),
386+
IsWarning: true,
387+
})
388+
}
389+
378390
if len(testedVariant.TestResults) < requiredNumberOfTests {
379391
results = append(results, ValidationResult{
380392
Error: fmt.Errorf("error: only %d tests found, need at least %d for %q on %v",
@@ -639,7 +651,8 @@ func (a OrderedJobVariants) Less(i, j int) bool {
639651
type TestingResults struct {
640652
JobVariant JobVariant
641653

642-
TestResults []TestResults
654+
TestResults []TestResults
655+
HasCandidateTierResults bool // true if candidate-tier queries returned any test data
643656
}
644657

645658
type TestResults struct {
@@ -674,6 +687,7 @@ func validateJobTiers(jobVariant JobVariant) error {
674687
"standard": true,
675688
"informing": true,
676689
"blocking": true,
690+
"candidate": true,
677691
}
678692

679693
hasValidTier := false
@@ -682,7 +696,7 @@ func validateJobTiers(jobVariant JobVariant) error {
682696
if tier != "" {
683697
hasValidTier = true
684698
if !validTiers[tier] {
685-
return fmt.Errorf("invalid JobTier %q in variant %+v - must be one of: standard, informing, blocking", tier, jobVariant)
699+
return fmt.Errorf("invalid JobTier %q in variant %+v - must be one of: standard, informing, blocking, candidate", tier, jobVariant)
686700
}
687701
}
688702
}
@@ -838,6 +852,7 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi
838852
}
839853

840854
testNameToResults := map[string]*TestResults{}
855+
hasCandidateTierResults := false
841856
queries := sippy.QueriesFor(jobVariant.Cloud, jobVariant.Architecture, jobVariant.Topology, jobVariant.NetworkStack, jobVariant.OS, jobVariant.JobTiers, testPattern)
842857
release, err := getRelease()
843858
if err != nil {
@@ -884,6 +899,10 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi
884899
return nil, err
885900
}
886901

902+
if currQuery.TierName == "candidate" && len(testInfos) > 0 {
903+
hasCandidateTierResults = true
904+
}
905+
887906
for _, currTest := range testInfos {
888907
testResults, ok := testNameToResults[currTest.Name]
889908
if !ok {
@@ -912,8 +931,9 @@ func listTestResultForVariant(featureGate string, jobVariant JobVariant) (*Testi
912931
}
913932

914933
jobVariantResults := &TestingResults{
915-
JobVariant: jobVariant,
916-
TestResults: nil,
934+
JobVariant: jobVariant,
935+
TestResults: nil,
936+
HasCandidateTierResults: hasCandidateTierResults,
917937
}
918938
testNames := sets.StringKeySet(testNameToResults)
919939
for _, testName := range testNames.List() {

tools/codegen/cmd/featuregate-test-analyzer_test.go

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,195 @@ func Test_listTestResultFor(t *testing.T) {
7979
}
8080
}
8181

82+
func Test_validateJobTiers_candidate(t *testing.T) {
83+
tests := []struct {
84+
name string
85+
variant JobVariant
86+
wantErr bool
87+
}{
88+
{
89+
name: "candidate is valid",
90+
variant: JobVariant{Cloud: "aws", Architecture: "amd64", Topology: "ha", JobTiers: "candidate"},
91+
wantErr: false,
92+
},
93+
{
94+
name: "candidate with standard is valid",
95+
variant: JobVariant{Cloud: "aws", Architecture: "amd64", Topology: "ha", JobTiers: "standard,candidate"},
96+
wantErr: false,
97+
},
98+
{
99+
name: "invalid tier still rejected",
100+
variant: JobVariant{Cloud: "aws", Architecture: "amd64", Topology: "ha", JobTiers: "bogus"},
101+
wantErr: true,
102+
},
103+
{
104+
name: "candidate with invalid tier rejected",
105+
variant: JobVariant{Cloud: "aws", Architecture: "amd64", Topology: "ha", JobTiers: "candidate,bogus"},
106+
wantErr: true,
107+
},
108+
}
109+
110+
for _, tt := range tests {
111+
t.Run(tt.name, func(t *testing.T) {
112+
err := validateJobTiers(tt.variant)
113+
if (err != nil) != tt.wantErr {
114+
t.Errorf("validateJobTiers() error = %v, wantErr %v", err, tt.wantErr)
115+
}
116+
})
117+
}
118+
}
119+
120+
func Test_checkIfTestingIsSufficient_CandidateVariants(t *testing.T) {
121+
sufficientTests := []TestResults{
122+
{TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15},
123+
{TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15},
124+
{TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15},
125+
{TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15},
126+
{TestName: "test5", TotalRuns: 15, SuccessfulRuns: 15},
127+
}
128+
insufficientTests := []TestResults{
129+
{TestName: "test1", TotalRuns: 15, SuccessfulRuns: 15},
130+
{TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15},
131+
// Only 2 tests, need 5
132+
}
133+
134+
tests := []struct {
135+
name string
136+
featureGate string
137+
testingResults map[JobVariant]*TestingResults
138+
wantBlockingErrors int
139+
wantWarnings int
140+
}{
141+
{
142+
name: "candidate tier returned results with sufficient tests - warning about component readiness",
143+
featureGate: "TestFeature",
144+
testingResults: map[JobVariant]*TestingResults{
145+
{
146+
Cloud: "aws",
147+
Architecture: "amd64",
148+
Topology: "ha",
149+
}: {
150+
TestResults: sufficientTests,
151+
HasCandidateTierResults: true,
152+
},
153+
},
154+
wantBlockingErrors: 0,
155+
wantWarnings: 1, // component readiness warning
156+
},
157+
{
158+
name: "candidate tier returned results with insufficient tests - blocking error plus warning",
159+
featureGate: "TestFeature",
160+
testingResults: map[JobVariant]*TestingResults{
161+
{
162+
Cloud: "aws",
163+
Architecture: "amd64",
164+
Topology: "ha",
165+
}: {
166+
TestResults: insufficientTests,
167+
HasCandidateTierResults: true,
168+
},
169+
},
170+
wantBlockingErrors: 1, // insufficient tests is still blocking
171+
wantWarnings: 1, // component readiness warning
172+
},
173+
{
174+
name: "no candidate tier results - no warning",
175+
featureGate: "TestFeature",
176+
testingResults: map[JobVariant]*TestingResults{
177+
{
178+
Cloud: "aws",
179+
Architecture: "amd64",
180+
Topology: "ha",
181+
}: {
182+
TestResults: sufficientTests,
183+
HasCandidateTierResults: false,
184+
},
185+
},
186+
wantBlockingErrors: 0,
187+
wantWarnings: 0,
188+
},
189+
{
190+
name: "candidate tier returned results with low pass rate - blocking error plus warning",
191+
featureGate: "TestFeature",
192+
testingResults: map[JobVariant]*TestingResults{
193+
{
194+
Cloud: "aws",
195+
Architecture: "amd64",
196+
Topology: "ha",
197+
}: {
198+
TestResults: []TestResults{
199+
{TestName: "test1", TotalRuns: 20, SuccessfulRuns: 18}, // 90%
200+
{TestName: "test2", TotalRuns: 15, SuccessfulRuns: 15},
201+
{TestName: "test3", TotalRuns: 15, SuccessfulRuns: 15},
202+
{TestName: "test4", TotalRuns: 15, SuccessfulRuns: 15},
203+
{TestName: "test5", TotalRuns: 15, SuccessfulRuns: 15},
204+
},
205+
HasCandidateTierResults: true,
206+
},
207+
},
208+
wantBlockingErrors: 1, // low pass rate is still blocking
209+
wantWarnings: 1, // component readiness warning
210+
},
211+
{
212+
name: "mix of variants - one with candidate results one without",
213+
featureGate: "TestFeature",
214+
testingResults: map[JobVariant]*TestingResults{
215+
{
216+
Cloud: "aws",
217+
Architecture: "amd64",
218+
Topology: "ha",
219+
}: {
220+
TestResults: sufficientTests,
221+
HasCandidateTierResults: false,
222+
},
223+
{
224+
Cloud: "gcp",
225+
Architecture: "amd64",
226+
Topology: "ha",
227+
}: {
228+
TestResults: sufficientTests,
229+
HasCandidateTierResults: true,
230+
},
231+
},
232+
wantBlockingErrors: 0,
233+
wantWarnings: 1, // only gcp variant has candidate warning
234+
},
235+
}
236+
237+
for _, tt := range tests {
238+
t.Run(tt.name, func(t *testing.T) {
239+
results := checkIfTestingIsSufficient(tt.featureGate, tt.testingResults)
240+
241+
blockingErrors := 0
242+
warnings := 0
243+
for _, result := range results {
244+
if result.IsWarning {
245+
warnings++
246+
} else {
247+
blockingErrors++
248+
}
249+
}
250+
251+
if blockingErrors != tt.wantBlockingErrors {
252+
t.Errorf("got %d blocking errors, want %d", blockingErrors, tt.wantBlockingErrors)
253+
for _, result := range results {
254+
if !result.IsWarning {
255+
t.Logf(" Blocking error: %v", result.Error)
256+
}
257+
}
258+
}
259+
if warnings != tt.wantWarnings {
260+
t.Errorf("got %d warnings, want %d", warnings, tt.wantWarnings)
261+
for _, result := range results {
262+
if result.IsWarning {
263+
t.Logf(" Warning: %v", result.Error)
264+
}
265+
}
266+
}
267+
})
268+
}
269+
}
270+
82271
func Test_checkIfTestingIsSufficient_OptionalVariants(t *testing.T) {
83272
tests := []struct {
84273
name string

tools/codegen/pkg/sippy/json_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
type SippyQueryStruct struct {
1313
Items []SippyQueryItem `json:"items"`
1414
LinkOperator string `json:"linkOperator"`
15+
TierName string `json:"-"` // not serialized, used to track which tier this query is for
1516
}
1617

1718
type SippyQueryItem struct {
@@ -138,6 +139,7 @@ func QueriesFor(cloud, architecture, topology, networkStack, os, jobTiers, testP
138139
queries = append(queries, &SippyQueryStruct{
139140
Items: items,
140141
LinkOperator: "and",
142+
TierName: jobTier,
141143
})
142144
}
143145

0 commit comments

Comments
 (0)