Improve Actions status aggregations (#32860)

Make the result the same as GitHub:

* all skipped, then result is skipped
* any cancelled, then result cancelled
This commit is contained in:
wxiaoguang 2024-12-16 21:49:53 +08:00 committed by GitHub
parent d28a4843b8
commit 22c4599542
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 31 additions and 9 deletions

View File

@ -153,20 +153,25 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col
}
func AggregateJobStatus(jobs []*ActionRunJob) Status {
allSuccessOrSkipped := true
var hasFailure, hasCancelled, hasSkipped, hasWaiting, hasRunning, hasBlocked bool
allSuccessOrSkipped := len(jobs) != 0
allSkipped := len(jobs) != 0
var hasFailure, hasCancelled, hasWaiting, hasRunning, hasBlocked bool
for _, job := range jobs {
allSuccessOrSkipped = allSuccessOrSkipped && (job.Status == StatusSuccess || job.Status == StatusSkipped)
allSkipped = allSkipped && job.Status == StatusSkipped
hasFailure = hasFailure || job.Status == StatusFailure
hasCancelled = hasCancelled || job.Status == StatusCancelled
hasSkipped = hasSkipped || job.Status == StatusSkipped
hasWaiting = hasWaiting || job.Status == StatusWaiting
hasRunning = hasRunning || job.Status == StatusRunning
hasBlocked = hasBlocked || job.Status == StatusBlocked
}
switch {
case allSkipped:
return StatusSkipped
case allSuccessOrSkipped:
return StatusSuccess
case hasCancelled:
return StatusCancelled
case hasFailure:
return StatusFailure
case hasRunning:
@ -175,10 +180,6 @@ func AggregateJobStatus(jobs []*ActionRunJob) Status {
return StatusWaiting
case hasBlocked:
return StatusBlocked
case hasCancelled:
return StatusCancelled
case hasSkipped:
return StatusSkipped
default:
return StatusUnknown // it shouldn't happen
}

View File

@ -11,6 +11,7 @@ import (
func TestAggregateJobStatus(t *testing.T) {
testStatuses := func(expected Status, statuses []Status) {
t.Helper()
var jobs []*ActionRunJob
for _, v := range statuses {
jobs = append(jobs, &ActionRunJob{Status: v})
@ -29,6 +30,16 @@ func TestAggregateJobStatus(t *testing.T) {
statuses []Status
expected Status
}{
// unknown cases, maybe it shouldn't happen in real world
{[]Status{}, StatusUnknown},
{[]Status{StatusUnknown, StatusSuccess}, StatusUnknown},
{[]Status{StatusUnknown, StatusSkipped}, StatusUnknown},
{[]Status{StatusUnknown, StatusFailure}, StatusFailure},
{[]Status{StatusUnknown, StatusCancelled}, StatusCancelled},
{[]Status{StatusUnknown, StatusWaiting}, StatusWaiting},
{[]Status{StatusUnknown, StatusRunning}, StatusRunning},
{[]Status{StatusUnknown, StatusBlocked}, StatusBlocked},
// success with other status
{[]Status{StatusSuccess}, StatusSuccess},
{[]Status{StatusSuccess, StatusSkipped}, StatusSuccess}, // skipped doesn't affect success
@ -38,18 +49,28 @@ func TestAggregateJobStatus(t *testing.T) {
{[]Status{StatusSuccess, StatusRunning}, StatusRunning},
{[]Status{StatusSuccess, StatusBlocked}, StatusBlocked},
// any cancelled, then cancelled
{[]Status{StatusCancelled}, StatusCancelled},
{[]Status{StatusCancelled, StatusSuccess}, StatusCancelled},
{[]Status{StatusCancelled, StatusSkipped}, StatusCancelled},
{[]Status{StatusCancelled, StatusFailure}, StatusCancelled},
{[]Status{StatusCancelled, StatusWaiting}, StatusCancelled},
{[]Status{StatusCancelled, StatusRunning}, StatusCancelled},
{[]Status{StatusCancelled, StatusBlocked}, StatusCancelled},
// failure with other status, fail fast
// Should "running" win? Maybe no: old code does make "running" win, but GitHub does fail fast.
{[]Status{StatusFailure}, StatusFailure},
{[]Status{StatusFailure, StatusSuccess}, StatusFailure},
{[]Status{StatusFailure, StatusSkipped}, StatusFailure},
{[]Status{StatusFailure, StatusCancelled}, StatusFailure},
{[]Status{StatusFailure, StatusCancelled}, StatusCancelled},
{[]Status{StatusFailure, StatusWaiting}, StatusFailure},
{[]Status{StatusFailure, StatusRunning}, StatusFailure},
{[]Status{StatusFailure, StatusBlocked}, StatusFailure},
// skipped with other status
{[]Status{StatusSkipped}, StatusSuccess},
// TODO: need to clarify whether a PR with "skipped" job status is considered as "mergeable" or not.
{[]Status{StatusSkipped}, StatusSkipped},
{[]Status{StatusSkipped, StatusSuccess}, StatusSuccess},
{[]Status{StatusSkipped, StatusFailure}, StatusFailure},
{[]Status{StatusSkipped, StatusCancelled}, StatusCancelled},