diff options
| -rw-r--r-- | models/actions/run_job.go | 15 | ||||
| -rw-r--r-- | models/actions/run_job_status_test.go | 25 |
2 files changed, 31 insertions, 9 deletions
diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 8c131351d5..de4b6aab66 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -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 } diff --git a/models/actions/run_job_status_test.go b/models/actions/run_job_status_test.go index bac480a96b..04fd9ceba7 100644 --- a/models/actions/run_job_status_test.go +++ b/models/actions/run_job_status_test.go @@ -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}, |