Fix incomplete Actions status aggregations (#32859)

fix #32857

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
+	allSuccessOrSkipped := true
-	allWaiting := true
+	var hasFailure, hasCancelled, hasSkipped, hasWaiting, hasRunning, hasBlocked bool
-	hasFailure := false
 	for _, job := range jobs {
-		if !job.Status.IsDone() {
+		allSuccessOrSkipped = allSuccessOrSkipped && (job.Status == StatusSuccess || job.Status == StatusSkipped)
-			allDone = false
+		hasFailure = hasFailure || job.Status == StatusFailure
-		}
+		hasCancelled = hasCancelled || job.Status == StatusCancelled
-		if job.Status != StatusWaiting && !job.Status.IsDone() {
+		hasSkipped = hasSkipped || job.Status == StatusSkipped
-			allWaiting = false
+		hasWaiting = hasWaiting || job.Status == StatusWaiting
-		}
+		hasRunning = hasRunning || job.Status == StatusRunning
-		if job.Status == StatusFailure || job.Status == StatusCancelled {
+		hasBlocked = hasBlocked || job.Status == StatusBlocked
-			hasFailure = true
-		}
 	}
-	if allDone {
+	switch {
-		if hasFailure {
+	case allSuccessOrSkipped:
-			return StatusFailure
-		}
 		return StatusSuccess
-	}
+	case hasFailure:
-	if allWaiting {
+		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
 }

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)
+	}
+}

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 -}}
+{{- $size := Iif .size .size 16 -}}
-{{- if .size -}}
+{{- $className := Iif .className .className "" -}}
-{{- $size = .size -}}
+<span data-tooltip-content="{{ctx.Locale.Tr (printf "actions.status.%s" .status)}}">
-{{- end -}}
-
-{{- $className := "" -}}
-{{- if .className -}}
-{{- $className = .className -}}
-{{- end -}}
-
-<span class="tw-flex tw-items-center" data-tooltip-content="{{ctx.Locale.Tr (printf "actions.status.%s" .status)}}">
 {{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}}
 </span>

web_src/js/components/ActionRunStatus.vue

@@ -6,24 +6,25 @@
 import {SvgIcon} from '../svg.ts';
 
 withDefaults(defineProps<{
-  status: '',
+  status: 'success' | 'skipped' | 'waiting' | 'blocked' | 'running' | 'failure' | 'cancelled' | 'unknown',
-  size?: number,
+  size: number,
-  className?: string,
+  className: string,
   localeStatus?: string,
 }>(), {
   size: 16,
-  className: undefined,
+  className: '',
   localeStatus: undefined,
 });
 </script>
 
 <template>
-  <span class="tw-flex tw-items-center" :data-tooltip-content="localeStatus" v-if="status">
+  <span :data-tooltip-content="localeStatus ?? status" v-if="status">
     <SvgIcon name="octicon-check-circle-fill" class="text green" :size="size" :class-name="className" v-if="status === 'success'"/>
     <SvgIcon name="octicon-skip" class="text grey" :size="size" :class-name="className" v-else-if="status === 'skipped'"/>
+    <SvgIcon name="octicon-stop" class="text yellow" :size="size" :class-name="className" v-else-if="status === 'cancelled'"/>
     <SvgIcon name="octicon-clock" class="text yellow" :size="size" :class-name="className" v-else-if="status === 'waiting'"/>
     <SvgIcon name="octicon-blocked" class="text yellow" :size="size" :class-name="className" v-else-if="status === 'blocked'"/>
     <SvgIcon name="octicon-meter" class="text yellow" :size="size" :class-name="'job-status-rotate ' + className" v-else-if="status === 'running'"/>
-    <SvgIcon name="octicon-x-circle-fill" class="text red" :size="size" v-else-if="['failure', 'cancelled', 'unknown'].includes(status)"/>
+    <SvgIcon name="octicon-x-circle-fill" class="text red" :size="size" v-else/><!-- failure, unknown -->
   </span>
 </template>

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;
 }

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,