From 90b65da7e48d2aa548092a7e3149f65ba39f1b02 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 16 Dec 2024 11:18:00 +0800 Subject: [PATCH] Fix incomplete Actions status aggregations (#32859) fix #32857 (cherry picked from commit d28a4843b8de5d5e01ef3d7b2ad25f22853247ad) Conflicts: web_src/js/components/ActionRunStatus.vue remove the refactoring, keep the additional cancelled status --- models/actions/run_job.go | 43 ++++++++------- models/actions/run_job_status_test.go | 64 +++++++++++++++++++++++ templates/repo/actions/status.tmpl | 4 +- web_src/js/components/ActionRunStatus.vue | 5 +- web_src/js/components/RepoActionView.vue | 4 +- web_src/js/svg.js | 2 + 6 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 models/actions/run_job_status_test.go diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 2319af8e08..8c131351d5 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -153,28 +153,33 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col } func AggregateJobStatus(jobs []*ActionRunJob) Status { - allDone := true - allWaiting := true - hasFailure := false + allSuccessOrSkipped := true + var hasFailure, hasCancelled, hasSkipped, hasWaiting, hasRunning, hasBlocked bool for _, job := range jobs { - if !job.Status.IsDone() { - allDone = false - } - if job.Status != StatusWaiting && !job.Status.IsDone() { - allWaiting = false - } - if job.Status == StatusFailure || job.Status == StatusCancelled { - hasFailure = true - } + allSuccessOrSkipped = allSuccessOrSkipped && (job.Status == StatusSuccess || 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 } - if allDone { - if hasFailure { - return StatusFailure - } + switch { + case allSuccessOrSkipped: return StatusSuccess - } - if allWaiting { + case hasFailure: + return StatusFailure + case hasRunning: + return StatusRunning + case hasWaiting: return StatusWaiting + case hasBlocked: + return StatusBlocked + case hasCancelled: + return StatusCancelled + case hasSkipped: + return StatusSkipped + default: + return StatusUnknown // it shouldn't happen } - return StatusRunning } diff --git a/models/actions/run_job_status_test.go b/models/actions/run_job_status_test.go new file mode 100644 index 0000000000..bac480a96b --- /dev/null +++ b/models/actions/run_job_status_test.go @@ -0,0 +1,64 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAggregateJobStatus(t *testing.T) { + testStatuses := func(expected Status, statuses []Status) { + var jobs []*ActionRunJob + for _, v := range statuses { + jobs = append(jobs, &ActionRunJob{Status: v}) + } + actual := AggregateJobStatus(jobs) + if !assert.Equal(t, expected, actual) { + var statusStrings []string + for _, s := range statuses { + statusStrings = append(statusStrings, s.String()) + } + t.Errorf("AggregateJobStatus(%v) = %v, want %v", statusStrings, statusNames[actual], statusNames[expected]) + } + } + + cases := []struct { + statuses []Status + expected Status + }{ + // success with other status + {[]Status{StatusSuccess}, StatusSuccess}, + {[]Status{StatusSuccess, StatusSkipped}, StatusSuccess}, // skipped doesn't affect success + {[]Status{StatusSuccess, StatusFailure}, StatusFailure}, + {[]Status{StatusSuccess, StatusCancelled}, StatusCancelled}, + {[]Status{StatusSuccess, StatusWaiting}, StatusWaiting}, + {[]Status{StatusSuccess, StatusRunning}, StatusRunning}, + {[]Status{StatusSuccess, StatusBlocked}, StatusBlocked}, + + // 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, StatusWaiting}, StatusFailure}, + {[]Status{StatusFailure, StatusRunning}, StatusFailure}, + {[]Status{StatusFailure, StatusBlocked}, StatusFailure}, + + // skipped with other status + {[]Status{StatusSkipped}, StatusSuccess}, + {[]Status{StatusSkipped, StatusSuccess}, StatusSuccess}, + {[]Status{StatusSkipped, StatusFailure}, StatusFailure}, + {[]Status{StatusSkipped, StatusCancelled}, StatusCancelled}, + {[]Status{StatusSkipped, StatusWaiting}, StatusWaiting}, + {[]Status{StatusSkipped, StatusRunning}, StatusRunning}, + {[]Status{StatusSkipped, StatusBlocked}, StatusBlocked}, + } + + for _, c := range cases { + testStatuses(c.expected, c.statuses) + } +} diff --git a/templates/repo/actions/status.tmpl b/templates/repo/actions/status.tmpl index a0e02cf8d7..99fa74ac17 100644 --- a/templates/repo/actions/status.tmpl +++ b/templates/repo/actions/status.tmpl @@ -17,13 +17,15 @@ {{svg "octicon-check-circle-fill" $size (printf "text green %s" $className)}} {{else if eq .status "skipped"}} {{svg "octicon-skip" $size (printf "text grey %s" $className)}} +{{else if eq .status "cancelled"}} + {{svg "octicon-stop" $size (printf "text grey %s" $className)}} {{else if eq .status "waiting"}} {{svg "octicon-clock" $size (printf "text yellow %s" $className)}} {{else if eq .status "blocked"}} {{svg "octicon-blocked" $size (printf "text yellow %s" $className)}} {{else if eq .status "running"}} {{svg "octicon-meter" $size (printf "text yellow job-status-rotate %s" $className)}} -{{else if or (eq .status "failure") or (eq .status "cancelled") or (eq .status "unknown")}} +{{else}}{{/*failure, unknown*/}} {{svg "octicon-x-circle-fill" $size (printf "text red %s" $className)}} {{end}} diff --git a/web_src/js/components/ActionRunStatus.vue b/web_src/js/components/ActionRunStatus.vue index 7ada543fea..eb7f780fbe 100644 --- a/web_src/js/components/ActionRunStatus.vue +++ b/web_src/js/components/ActionRunStatus.vue @@ -28,12 +28,13 @@ export default { }; diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 13ac9427f3..136ad3693d 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -570,11 +570,13 @@ export function initRepositoryActionView() { .action-info-summary-title { display: flex; + align-items: center; + gap: 0.5em; } .action-info-summary-title-text { font-size: 20px; - margin: 0 0 0 8px; + margin: 0; flex: 1; overflow-wrap: anywhere; } diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 9ef5f28e2f..76df7ff211 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -64,6 +64,7 @@ import octiconSidebarCollapse from '../../public/assets/img/svg/octicon-sidebar- import octiconSidebarExpand from '../../public/assets/img/svg/octicon-sidebar-expand.svg'; import octiconSkip from '../../public/assets/img/svg/octicon-skip.svg'; import octiconStar from '../../public/assets/img/svg/octicon-star.svg'; +import octiconStop from '../../public/assets/img/svg/octicon-stop.svg'; import octiconStrikethrough from '../../public/assets/img/svg/octicon-strikethrough.svg'; import octiconSync from '../../public/assets/img/svg/octicon-sync.svg'; import octiconTable from '../../public/assets/img/svg/octicon-table.svg'; @@ -138,6 +139,7 @@ const svgs = { 'octicon-sidebar-expand': octiconSidebarExpand, 'octicon-skip': octiconSkip, 'octicon-star': octiconStar, + 'octicon-stop': octiconStop, 'octicon-strikethrough': octiconStrikethrough, 'octicon-sync': octiconSync, 'octicon-table': octiconTable,