From d28a4843b8de5d5e01ef3d7b2ad25f22853247ad 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 --- models/actions/run_job.go | 43 ++++++++------- models/actions/run_job_status_test.go | 64 +++++++++++++++++++++++ templates/repo/actions/status.tmpl | 18 +++---- web_src/js/components/ActionRunStatus.vue | 13 ++--- web_src/js/components/RepoActionView.vue | 4 +- web_src/js/svg.ts | 2 + 6 files changed, 106 insertions(+), 38 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..64c2543302 100644 --- a/templates/repo/actions/status.tmpl +++ b/templates/repo/actions/status.tmpl @@ -2,28 +2,22 @@ Please also update the vue file above if this template is modified. action status accepted: success, skipped, waiting, blocked, running, failure, cancelled, unknown --> -{{- $size := 16 -}} -{{- if .size -}} -{{- $size = .size -}} -{{- end -}} - -{{- $className := "" -}} -{{- if .className -}} -{{- $className = .className -}} -{{- end -}} - - +{{- $size := Iif .size .size 16 -}} +{{- $className := Iif .className .className "" -}} + {{if eq .status "success"}} {{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 558b881dfe..deab5f6469 100644 --- a/web_src/js/components/ActionRunStatus.vue +++ b/web_src/js/components/ActionRunStatus.vue @@ -6,24 +6,25 @@ import {SvgIcon} from '../svg.ts'; withDefaults(defineProps<{ - status: '', - size?: number, - className?: string, + status: 'success' | 'skipped' | 'waiting' | 'blocked' | 'running' | 'failure' | 'cancelled' | 'unknown', + size: number, + className: string, localeStatus?: string, }>(), { size: 16, - className: undefined, + className: '', localeStatus: undefined, }); diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index eece2efaf8..cb65a98edd 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -551,11 +551,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.ts b/web_src/js/svg.ts index 3a0f2ed53c..90b12fa87d 100644 --- a/web_src/js/svg.ts +++ b/web_src/js/svg.ts @@ -65,6 +65,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'; @@ -140,6 +141,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,