Skip to content

Commit d28a484

Browse files
wxiaoguangwxiaoguang
authored
Fix incomplete Actions status aggregations (#32859)
fix #32857
1 parent 276f433 commit d28a484

File tree

6 files changed

+106
-38
lines changed

6 files changed

+106
-38
lines changed

‎models/actions/run_job.go

+24-19
Original file line numberDiff line numberDiff line change
@@ -153,28 +153,33 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col
153153
}
154154

155155
func AggregateJobStatus(jobs []*ActionRunJob) Status {
156-
allDone := true
157-
allWaiting := true
158-
hasFailure := false
156+
allSuccessOrSkipped := true
157+
var hasFailure, hasCancelled, hasSkipped, hasWaiting, hasRunning, hasBlocked bool
159158
for _, job := range jobs {
160-
if !job.Status.IsDone() {
161-
allDone = false
162-
}
163-
if job.Status != StatusWaiting && !job.Status.IsDone() {
164-
allWaiting = false
165-
}
166-
if job.Status == StatusFailure || job.Status == StatusCancelled {
167-
hasFailure = true
168-
}
159+
allSuccessOrSkipped = allSuccessOrSkipped && (job.Status == StatusSuccess || job.Status == StatusSkipped)
160+
hasFailure = hasFailure || job.Status == StatusFailure
161+
hasCancelled = hasCancelled || job.Status == StatusCancelled
162+
hasSkipped = hasSkipped || job.Status == StatusSkipped
163+
hasWaiting = hasWaiting || job.Status == StatusWaiting
164+
hasRunning = hasRunning || job.Status == StatusRunning
165+
hasBlocked = hasBlocked || job.Status == StatusBlocked
169166
}
170-
if allDone {
171-
if hasFailure {
172-
return StatusFailure
173-
}
167+
switch {
168+
case allSuccessOrSkipped:
174169
return StatusSuccess
175-
}
176-
if allWaiting {
170+
case hasFailure:
171+
return StatusFailure
172+
case hasRunning:
173+
return StatusRunning
174+
case hasWaiting:
177175
return StatusWaiting
176+
case hasBlocked:
177+
return StatusBlocked
178+
case hasCancelled:
179+
return StatusCancelled
180+
case hasSkipped:
181+
return StatusSkipped
182+
default:
183+
return StatusUnknown // it shouldn't happen
178184
}
179-
return StatusRunning
180185
}

‎models/actions/run_job_status_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package actions
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestAggregateJobStatus(t *testing.T) {
13+
testStatuses := func(expected Status, statuses []Status) {
14+
var jobs []*ActionRunJob
15+
for _, v := range statuses {
16+
jobs = append(jobs, &ActionRunJob{Status: v})
17+
}
18+
actual := AggregateJobStatus(jobs)
19+
if !assert.Equal(t, expected, actual) {
20+
var statusStrings []string
21+
for _, s := range statuses {
22+
statusStrings = append(statusStrings, s.String())
23+
}
24+
t.Errorf("AggregateJobStatus(%v) = %v, want %v", statusStrings, statusNames[actual], statusNames[expected])
25+
}
26+
}
27+
28+
cases := []struct {
29+
statuses []Status
30+
expected Status
31+
}{
32+
// success with other status
33+
{[]Status{StatusSuccess}, StatusSuccess},
34+
{[]Status{StatusSuccess, StatusSkipped}, StatusSuccess}, // skipped doesn't affect success
35+
{[]Status{StatusSuccess, StatusFailure}, StatusFailure},
36+
{[]Status{StatusSuccess, StatusCancelled}, StatusCancelled},
37+
{[]Status{StatusSuccess, StatusWaiting}, StatusWaiting},
38+
{[]Status{StatusSuccess, StatusRunning}, StatusRunning},
39+
{[]Status{StatusSuccess, StatusBlocked}, StatusBlocked},
40+
41+
// failure with other status, fail fast
42+
// Should "running" win? Maybe no: old code does make "running" win, but GitHub does fail fast.
43+
{[]Status{StatusFailure}, StatusFailure},
44+
{[]Status{StatusFailure, StatusSuccess}, StatusFailure},
45+
{[]Status{StatusFailure, StatusSkipped}, StatusFailure},
46+
{[]Status{StatusFailure, StatusCancelled}, StatusFailure},
47+
{[]Status{StatusFailure, StatusWaiting}, StatusFailure},
48+
{[]Status{StatusFailure, StatusRunning}, StatusFailure},
49+
{[]Status{StatusFailure, StatusBlocked}, StatusFailure},
50+
51+
// skipped with other status
52+
{[]Status{StatusSkipped}, StatusSuccess},
53+
{[]Status{StatusSkipped, StatusSuccess}, StatusSuccess},
54+
{[]Status{StatusSkipped, StatusFailure}, StatusFailure},
55+
{[]Status{StatusSkipped, StatusCancelled}, StatusCancelled},
56+
{[]Status{StatusSkipped, StatusWaiting}, StatusWaiting},
57+
{[]Status{StatusSkipped, StatusRunning}, StatusRunning},
58+
{[]Status{StatusSkipped, StatusBlocked}, StatusBlocked},
59+
}
60+
61+
for _, c := range cases {
62+
testStatuses(c.expected, c.statuses)
63+
}
64+
}

‎templates/repo/actions/status.tmpl

+6-12
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,22 @@
22
Please also update the vue file above if this template is modified.
33
action status accepted: success, skipped, waiting, blocked, running, failure, cancelled, unknown
44
-->
5-
{{- $size := 16 -}}
6-
{{- if .size -}}
7-
{{- $size = .size -}}
8-
{{- end -}}
9-
10-
{{- $className := "" -}}
11-
{{- if .className -}}
12-
{{- $className = .className -}}
13-
{{- end -}}
14-
15-
<span class="tw-flex tw-items-center" data-tooltip-content="{{ctx.Locale.Tr (printf "actions.status.%s" .status)}}">
5+
{{- $size := Iif .size .size 16 -}}
6+
{{- $className := Iif .className .className "" -}}
7+
<span data-tooltip-content="{{ctx.Locale.Tr (printf "actions.status.%s" .status)}}">
168
{{if eq .status "success"}}
179
{{svg "octicon-check-circle-fill" $size (printf "text green %s" $className)}}
1810
{{else if eq .status "skipped"}}
1911
{{svg "octicon-skip" $size (printf "text grey %s" $className)}}
12+
{{else if eq .status "cancelled"}}
13+
{{svg "octicon-stop" $size (printf "text grey %s" $className)}}
2014
{{else if eq .status "waiting"}}
2115
{{svg "octicon-clock" $size (printf "text yellow %s" $className)}}
2216
{{else if eq .status "blocked"}}
2317
{{svg "octicon-blocked" $size (printf "text yellow %s" $className)}}
2418
{{else if eq .status "running"}}
2519
{{svg "octicon-meter" $size (printf "text yellow job-status-rotate %s" $className)}}
26-
{{else if or (eq .status "failure") or (eq .status "cancelled") or (eq .status "unknown")}}
20+
{{else}}{{/*failure, unknown*/}}
2721
{{svg "octicon-x-circle-fill" $size (printf "text red %s" $className)}}
2822
{{end}}
2923
</span>

‎web_src/js/components/ActionRunStatus.vue

+7-6
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,25 @@
66
import {SvgIcon} from '../svg.ts';
77

88
withDefaults(defineProps<{
9-
status: '',
10-
size?: number,
11-
className?: string,
9+
status: 'success' | 'skipped' | 'waiting' | 'blocked' | 'running' | 'failure' | 'cancelled' | 'unknown',
10+
size: number,
11+
className: string,
1212
localeStatus?: string,
1313
}>(), {
1414
size: 16,
15-
className: undefined,
15+
className: '',
1616
localeStatus: undefined,
1717
});
1818
</script>
1919

2020
<template>
21-
<span class="tw-flex tw-items-center" :data-tooltip-content="localeStatus" v-if="status">
21+
<span :data-tooltip-content="localeStatus ?? status" v-if="status">
2222
<SvgIcon name="octicon-check-circle-fill" class="text green" :size="size" :class-name="className" v-if="status === 'success'"/>
2323
<SvgIcon name="octicon-skip" class="text grey" :size="size" :class-name="className" v-else-if="status === 'skipped'"/>
24+
<SvgIcon name="octicon-stop" class="text yellow" :size="size" :class-name="className" v-else-if="status === 'cancelled'"/>
2425
<SvgIcon name="octicon-clock" class="text yellow" :size="size" :class-name="className" v-else-if="status === 'waiting'"/>
2526
<SvgIcon name="octicon-blocked" class="text yellow" :size="size" :class-name="className" v-else-if="status === 'blocked'"/>
2627
<SvgIcon name="octicon-meter" class="text yellow" :size="size" :class-name="'job-status-rotate ' + className" v-else-if="status === 'running'"/>
27-
<SvgIcon name="octicon-x-circle-fill" class="text red" :size="size" v-else-if="['failure', 'cancelled', 'unknown'].includes(status)"/>
28+
<SvgIcon name="octicon-x-circle-fill" class="text red" :size="size" v-else/><!-- failure, unknown -->
2829
</span>
2930
</template>

‎web_src/js/components/RepoActionView.vue

+3-1
Original file line numberDiff line numberDiff line change
@@ -551,11 +551,13 @@ export function initRepositoryActionView() {
551551
552552
.action-info-summary-title {
553553
display: flex;
554+
align-items: center;
555+
gap: 0.5em;
554556
}
555557
556558
.action-info-summary-title-text {
557559
font-size: 20px;
558-
margin: 0 0 0 8px;
560+
margin: 0;
559561
flex: 1;
560562
overflow-wrap: anywhere;
561563
}

‎web_src/js/svg.ts

+2
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ import octiconSidebarCollapse from '../../public/assets/img/svg/octicon-sidebar-
6565
import octiconSidebarExpand from '../../public/assets/img/svg/octicon-sidebar-expand.svg';
6666
import octiconSkip from '../../public/assets/img/svg/octicon-skip.svg';
6767
import octiconStar from '../../public/assets/img/svg/octicon-star.svg';
68+
import octiconStop from '../../public/assets/img/svg/octicon-stop.svg';
6869
import octiconStrikethrough from '../../public/assets/img/svg/octicon-strikethrough.svg';
6970
import octiconSync from '../../public/assets/img/svg/octicon-sync.svg';
7071
import octiconTable from '../../public/assets/img/svg/octicon-table.svg';
@@ -140,6 +141,7 @@ const svgs = {
140141
'octicon-sidebar-expand': octiconSidebarExpand,
141142
'octicon-skip': octiconSkip,
142143
'octicon-star': octiconStar,
144+
'octicon-stop': octiconStop,
143145
'octicon-strikethrough': octiconStrikethrough,
144146
'octicon-sync': octiconSync,
145147
'octicon-table': octiconTable,

0 commit comments

Comments
 (0)