Skip to content

Commit 7c34c85

Browse files
FloThinksPiFloThinksPi
committed
Thread waiting statuses as missing results
1 parent 4e9e9fe commit 7c34c85

File tree

5 files changed

+241
-50
lines changed

5 files changed

+241
-50
lines changed

‎policy/predicate/status_check.go‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c
6363
allowedStatuses = []string{"completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting", "error", "success"}
6464
}
6565

66+
// checkStatuses that are in a wating state so should be considdered as missing results
67+
waitingStatuses := []string{"expected", "in_progress", "pending", "queued", "requested", "waiting"}
68+
6669
checkStatuses, err := prctx.LatestCheckStatuses()
6770
if err != nil {
6871
return nil, errors.Wrap(err, "failed to list commit statuses")
@@ -92,7 +95,9 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c
9295
checkResult := checkStatuses[checkResultName]
9396
isValidStatus := slices.Contains(allowedStatuses, *checkResult.Status)
9497
isValidConclusion := checkResult.Conclusion != nil && slices.Contains(allowedConclusions, *checkResult.Conclusion)
95-
if (checkResult.Status == nil || !isValidStatus) ||
98+
if slices.Contains(waitingStatuses, *checkResult.Status) && !isValidStatus {
99+
missingResults[checkResultName] = checkResultName
100+
} else if (checkResult.Status == nil || !isValidStatus) ||
96101
(*checkResult.Status == "completed" && !isValidConclusion) {
97102
failingStatuses[checkResultName] = checkResultName
98103
}
@@ -103,7 +108,9 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c
103108
matched = true
104109
allChecks[repoStatusName] = repoStatusName
105110
repoStatusResult := repoStatuses[repoStatusName]
106-
if repoStatusResult == nil || repoStatusResult.State == nil || !slices.Contains(allowedStatuses, *repoStatusResult.State) {
111+
if *repoStatusResult.State == "pending" && !slices.Contains(allowedStatuses, "pending") {
112+
missingResults[repoStatusName] = repoStatusName
113+
} else if !slices.Contains(allowedStatuses, *repoStatusResult.State) {
107114
failingStatuses[repoStatusName] = repoStatusName
108115
}
109116
}
@@ -124,18 +131,28 @@ func (pred HasStatusCheck) Evaluate(ctx context.Context, prctx pull.Context) (*c
124131
),
125132
}
126133

127-
if len(missingResults) > 0 {
128-
predicateResult.Values = slices.Sorted(maps.Keys(missingResults))
129-
predicateResult.Description = fmt.Sprintf("One or more status checks or repo statuses are missing: %s", predicateResult.Values)
130-
predicateResult.Satisfied = false
131-
return &predicateResult, nil
134+
checkOrder := []struct {
135+
condition bool
136+
results map[string]string
137+
message string
138+
}{
139+
{pred.noRegex, missingResults, "One or more status checks or repo statuses are missing: %s"},
140+
{pred.noRegex, failingStatuses, "One or more status checks or repo statuses have not concluded with %s: %s"},
141+
{!pred.noRegex, failingStatuses, "One or more status checks or repo statuses have not concluded with %s: %s"},
142+
{!pred.noRegex, missingResults, "One or more status checks or repo statuses are missing: %s"},
132143
}
133144

134-
if len(failingStatuses) > 0 {
135-
predicateResult.Values = slices.Sorted(maps.Keys(failingStatuses))
136-
predicateResult.Description = fmt.Sprintf("One or more status checks or repo statuses have not concluded with %s: %s", joinElementsWithOr(allowedConclusions), failingStatuses)
137-
predicateResult.Satisfied = false
138-
return &predicateResult, nil
145+
for _, check := range checkOrder {
146+
if check.condition && len(check.results) > 0 {
147+
predicateResult.Values = slices.Sorted(maps.Keys(check.results))
148+
if strings.Contains(check.message, "%s: %s") {
149+
predicateResult.Description = fmt.Sprintf(check.message, joinElementsWithOr(allowedConclusions), check.results)
150+
} else {
151+
predicateResult.Description = fmt.Sprintf(check.message, predicateResult.Values)
152+
}
153+
predicateResult.Satisfied = false
154+
return &predicateResult, nil
155+
}
139156
}
140157

141158
predicateResult.Values = allChecksList

‎policy/predicate/status_check_test.go‎

Lines changed: 147 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func TestHasStatusCheck(t *testing.T) {
209209
},
210210
},
211211
{
212-
name: "a status check has non completed status",
212+
name: "a status check has non valid statuses and non ones in non accepted states, it should fail not reporting missing results",
213213
LatestCheckStatusesValue: map[string]*github.CheckRun{
214214
"test": mockCheckRun("completed", "success"),
215215
"test1": mockCheckRun("expected", ""),
@@ -227,7 +227,26 @@ func TestHasStatusCheck(t *testing.T) {
227227
},
228228
ExpectedPredicateResult: &common.PredicateResult{
229229
Satisfied: false,
230-
Values: []string{"test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8", "test9"},
230+
Values: []string{"test2", "test7", "test9"},
231+
},
232+
},
233+
{
234+
name: "a status check has non non completed ones so they shall be considered as missing results",
235+
LatestCheckStatusesValue: map[string]*github.CheckRun{
236+
"test": mockCheckRun("completed", "success"),
237+
"test1": mockCheckRun("expected", ""),
238+
"test3": mockCheckRun("in_progress", ""),
239+
"test4": mockCheckRun("pending", ""),
240+
"test5": mockCheckRun("queued", ""),
241+
"test6": mockCheckRun("requested", ""),
242+
"test8": mockCheckRun("waiting", ""),
243+
},
244+
predicate: HasStatusCheck{
245+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
246+
},
247+
ExpectedPredicateResult: &common.PredicateResult{
248+
Satisfied: false,
249+
Values: []string{"test1", "test3", "test4", "test5", "test6", "test8"},
231250
},
232251
},
233252
{
@@ -372,7 +391,7 @@ func TestHasStatusCheck(t *testing.T) {
372391
},
373392
ExpectedPredicateResult: &common.PredicateResult{
374393
Satisfied: false,
375-
Values: []string{"test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8"},
394+
Values: []string{"test2", "test7"},
376395
},
377396
},
378397
{
@@ -384,7 +403,7 @@ func TestHasStatusCheck(t *testing.T) {
384403
},
385404
ExpectedPredicateResult: &common.PredicateResult{
386405
Satisfied: false,
387-
Values: []string{"test", "test2", "test3", "test4", "test5", "test6", "test7", "test8"},
406+
Values: []string{"test", "test2", "test7"},
388407
},
389408
},
390409
{
@@ -396,7 +415,7 @@ func TestHasStatusCheck(t *testing.T) {
396415
},
397416
ExpectedPredicateResult: &common.PredicateResult{
398417
Satisfied: false,
399-
Values: []string{"test", "test1", "test3", "test4", "test5", "test6", "test7", "test8"},
418+
Values: []string{"test", "test7"},
400419
},
401420
},
402421
{
@@ -408,7 +427,7 @@ func TestHasStatusCheck(t *testing.T) {
408427
},
409428
ExpectedPredicateResult: &common.PredicateResult{
410429
Satisfied: false,
411-
Values: []string{"test", "test1", "test2", "test4", "test5", "test6", "test7", "test8"},
430+
Values: []string{"test", "test2", "test7"},
412431
},
413432
},
414433
{
@@ -420,7 +439,7 @@ func TestHasStatusCheck(t *testing.T) {
420439
},
421440
ExpectedPredicateResult: &common.PredicateResult{
422441
Satisfied: false,
423-
Values: []string{"test", "test1", "test2", "test3", "test5", "test6", "test7", "test8"},
442+
Values: []string{"test", "test2", "test7"},
424443
},
425444
},
426445
{
@@ -432,7 +451,7 @@ func TestHasStatusCheck(t *testing.T) {
432451
},
433452
ExpectedPredicateResult: &common.PredicateResult{
434453
Satisfied: false,
435-
Values: []string{"test", "test1", "test2", "test3", "test4", "test6", "test7", "test8"},
454+
Values: []string{"test", "test2", "test7"},
436455
},
437456
},
438457
{
@@ -444,7 +463,7 @@ func TestHasStatusCheck(t *testing.T) {
444463
},
445464
ExpectedPredicateResult: &common.PredicateResult{
446465
Satisfied: false,
447-
Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test7", "test8"},
466+
Values: []string{"test", "test2", "test7"},
448467
},
449468
},
450469
{
@@ -456,7 +475,7 @@ func TestHasStatusCheck(t *testing.T) {
456475
},
457476
ExpectedPredicateResult: &common.PredicateResult{
458477
Satisfied: false,
459-
Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test6", "test8"},
478+
Values: []string{"test", "test2"},
460479
},
461480
},
462481
{
@@ -468,7 +487,118 @@ func TestHasStatusCheck(t *testing.T) {
468487
},
469488
ExpectedPredicateResult: &common.PredicateResult{
470489
Satisfied: false,
471-
Values: []string{"test", "test1", "test2", "test3", "test4", "test5", "test6", "test7"},
490+
Values: []string{"test", "test2", "test7"},
491+
},
492+
},
493+
}
494+
495+
customStatusCheckTestCasesPositiv := []StatusCheckTestCase{
496+
{
497+
name: "status checks exist in status completed, conclusion success and status completed is allowed",
498+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("completed", "success")},
499+
predicate: HasStatusCheck{
500+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
501+
Statuses: []string{"completed"},
502+
},
503+
ExpectedPredicateResult: &common.PredicateResult{
504+
Satisfied: true,
505+
Values: []string{"test"},
506+
},
507+
},
508+
{
509+
name: "status checks exist with status expected and status expected is allowed",
510+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("expected", "")},
511+
predicate: HasStatusCheck{
512+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
513+
Statuses: []string{"expected"},
514+
},
515+
ExpectedPredicateResult: &common.PredicateResult{
516+
Satisfied: true,
517+
Values: []string{"test"},
518+
},
519+
},
520+
{
521+
name: "status checks exist with status failure and status failure is allowed",
522+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("failure", "")},
523+
predicate: HasStatusCheck{
524+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
525+
Statuses: []string{"failure"},
526+
},
527+
ExpectedPredicateResult: &common.PredicateResult{
528+
Satisfied: true,
529+
Values: []string{"test"},
530+
},
531+
},
532+
{
533+
name: "status checks exist with status in_progress and status in_progress is allowed",
534+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("in_progress", "")},
535+
predicate: HasStatusCheck{
536+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
537+
Statuses: []string{"in_progress"},
538+
},
539+
ExpectedPredicateResult: &common.PredicateResult{
540+
Satisfied: true,
541+
Values: []string{"test"},
542+
},
543+
},
544+
{
545+
name: "status checks exist with status pending and status pending is allowed",
546+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("pending", "")},
547+
predicate: HasStatusCheck{
548+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
549+
Statuses: []string{"pending"},
550+
},
551+
ExpectedPredicateResult: &common.PredicateResult{
552+
Satisfied: true,
553+
Values: []string{"test"},
554+
},
555+
},
556+
{
557+
name: "status checks exist with status queued and status queued is allowed",
558+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("queued", "")},
559+
predicate: HasStatusCheck{
560+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
561+
Statuses: []string{"queued"},
562+
},
563+
ExpectedPredicateResult: &common.PredicateResult{
564+
Satisfied: true,
565+
Values: []string{"test"},
566+
},
567+
},
568+
{
569+
name: "status checks exist with status requested and status requested is allowed",
570+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("requested", "")},
571+
predicate: HasStatusCheck{
572+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
573+
Statuses: []string{"requested"},
574+
},
575+
ExpectedPredicateResult: &common.PredicateResult{
576+
Satisfied: true,
577+
Values: []string{"test"},
578+
},
579+
},
580+
{
581+
name: "status checks exist with status startup_failure and status startup_failure is allowed",
582+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("startup_failure", "")},
583+
predicate: HasStatusCheck{
584+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
585+
Statuses: []string{"startup_failure"},
586+
},
587+
ExpectedPredicateResult: &common.PredicateResult{
588+
Satisfied: true,
589+
Values: []string{"test"},
590+
},
591+
},
592+
{
593+
name: "status checks exist with status waiting and status waiting is allowed",
594+
LatestCheckStatusesValue: map[string]*github.CheckRun{"test": mockCheckRun("waiting", "")},
595+
predicate: HasStatusCheck{
596+
Checks: []common.Regexp{common.NewMustCompileRegexp(".*")},
597+
Statuses: []string{"waiting"},
598+
},
599+
ExpectedPredicateResult: &common.PredicateResult{
600+
Satisfied: true,
601+
Values: []string{"test"},
472602
},
473603
},
474604
}
@@ -580,7 +710,7 @@ func TestHasStatusCheck(t *testing.T) {
580710
},
581711
ExpectedPredicateResult: &common.PredicateResult{
582712
Satisfied: false,
583-
Values: []string{"test2", "test3", "test4"},
713+
Values: []string{"test2", "test4"},
584714
},
585715
},
586716
}
@@ -595,7 +725,7 @@ func TestHasStatusCheck(t *testing.T) {
595725
},
596726
ExpectedPredicateResult: &common.PredicateResult{
597727
Satisfied: false,
598-
Values: []string{"test2", "test3", "test4"},
728+
Values: []string{"test2", "test4"},
599729
},
600730
},
601731
{
@@ -607,7 +737,7 @@ func TestHasStatusCheck(t *testing.T) {
607737
},
608738
ExpectedPredicateResult: &common.PredicateResult{
609739
Satisfied: false,
610-
Values: []string{"test1", "test3", "test4"},
740+
Values: []string{"test1", "test4"},
611741
},
612742
},
613743
{
@@ -631,7 +761,7 @@ func TestHasStatusCheck(t *testing.T) {
631761
},
632762
ExpectedPredicateResult: &common.PredicateResult{
633763
Satisfied: false,
634-
Values: []string{"test1", "test2", "test3"},
764+
Values: []string{"test1", "test2"},
635765
},
636766
},
637767
}
@@ -716,13 +846,14 @@ func TestHasStatusCheck(t *testing.T) {
716846
},
717847
ExpectedPredicateResult: &common.PredicateResult{
718848
Satisfied: false,
719-
Values: []string{"abc", "test2", "test3", "test4"},
849+
Values: []string{"abc", "test2", "test4"},
720850
},
721851
},
722852
}
723853

724854
runStatusCheckTestCase(t, defaultChecksTestCases)
725855
runStatusCheckTestCase(t, customConclusionChecksTestCases)
856+
runStatusCheckTestCase(t, customStatusCheckTestCasesPositiv)
726857
runStatusCheckTestCase(t, customStatusCheckTestCases)
727858
runStatusCheckTestCase(t, defaultRepoStatusTestCases)
728859
runStatusCheckTestCase(t, customStatusRepoStatusTestCases)

0 commit comments

Comments
 (0)