From 72b35c5a731b5236a3e79964161cc9e33720f68e Mon Sep 17 00:00:00 2001 From: Andreas Ahlenstorf Date: Tue, 11 Nov 2025 04:39:02 +0100 Subject: [PATCH] feat: display detailed action run diagnostics (#9966) Forgejo Actions allows variables in `jobs..runs-on`. However, the action list [skips checking whether a suitable runner](https://codeberg.org/forgejo/forgejo/src/commit/c3412d0579acfc702224f1785fc44e5695daee90/routers/web/repo/actions/actions.go#L114-L148) is available if an expression contains variables. That hampers a user's ability to figure out whether an expression was evaluated correctly and why a job might not be picked up by an available runner. This PR adds the ability to surface more complex and additional diagnostic information on the action view screen. Previously, only a job's status (waiting, running, ...) was displayed. Now, extended messages like "Waiting for a runner with the following labels: docker, trixie" are displayed with the possibility to show multiple messages simultaneously. How it looked before: ![old](/attachments/019e4e83-d44e-4143-8df0-7fceb611a3bd) How it looks after updating Forgejo without reloading the window: ![old-after-update](/attachments/e909af81-cf9e-4f44-a011-75585a2d1950) How it looks afterwards with a single label: ![new-single-label](/attachments/72f4a862-e23d-4ab5-9f96-09545954f982) How it looks afterwards with multiple labels: ![new-multiple-labels](/attachments/53036d7b-3589-4eeb-bad1-4da4cd5ff4b5) ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [x] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/9966): display detailed action run diagnostics Co-authored-by: Mathieu Fenniak Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9966 Reviewed-by: Mathieu Fenniak Co-authored-by: Andreas Ahlenstorf Co-committed-by: Andreas Ahlenstorf --- models/actions/run_job.go | 24 ++++++ models/actions/run_job_test.go | 78 ++++++++++++++++++++ options/locale_next/locale_en-US.json | 4 + routers/web/repo/actions/view.go | 14 ++-- routers/web/repo/actions/view_test.go | 6 +- tests/integration/actions_view_test.go | 4 +- web_src/js/components/RepoActionView.test.js | 4 +- web_src/js/components/RepoActionView.vue | 13 +++- 8 files changed, 128 insertions(+), 19 deletions(-) diff --git a/models/actions/run_job.go b/models/actions/run_job.go index edd1170072..78e7ac192d 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -6,12 +6,15 @@ package actions import ( "context" "fmt" + "html/template" "slices" + "strings" "time" "forgejo.org/models/db" "forgejo.org/modules/container" "forgejo.org/modules/timeutil" + "forgejo.org/modules/translation" "forgejo.org/modules/util" "xorm.io/builder" @@ -229,3 +232,24 @@ func AggregateJobStatus(jobs []*ActionRunJob) Status { return StatusUnknown // it shouldn't happen } } + +// StatusDiagnostics returns optional diagnostic information to display to the user derived from +// ActionRunJob's current status. It should help the user understand in which state the +// ActionRunJob is and why. +func (job *ActionRunJob) StatusDiagnostics(lang translation.Locale) []template.HTML { + diagnostics := []template.HTML{} + + switch job.Status { + case StatusWaiting: + joinedLabels := strings.Join(job.RunsOn, ", ") + diagnostics = append(diagnostics, lang.TrPluralString(len(job.RunsOn), "actions.status.diagnostics.waiting", joinedLabels)) + default: + diagnostics = append(diagnostics, template.HTML(job.Status.LocaleString(lang))) + } + + if job.Run.NeedApproval { + diagnostics = append(diagnostics, template.HTML(lang.TrString("actions.need_approval_desc"))) + } + + return diagnostics +} diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index 6abdb2bf5c..8f9e4b0ba2 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -4,10 +4,12 @@ package actions import ( "fmt" + "html/template" "testing" "forgejo.org/models/db" "forgejo.org/models/unittest" + "forgejo.org/modules/translation" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -70,3 +72,79 @@ func TestActionRunJob_HTMLURL(t *testing.T) { }) } } + +func TestActionRunJob_StatusDiagnostics(t *testing.T) { + translation.InitLocales(t.Context()) + english := translation.NewLocale("en-US") + + tests := []struct { + name string + job ActionRunJob + expected []template.HTML + }{ + { + name: "Unknown status", + job: ActionRunJob{RunsOn: []string{"windows"}, Status: StatusUnknown, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Unknown"}, + }, + { + name: "Waiting without labels", + job: ActionRunJob{RunsOn: []string{}, Status: StatusWaiting, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Waiting for a runner with the following labels: "}, + }, + { + name: "Waiting with one label", + job: ActionRunJob{RunsOn: []string{"freebsd"}, Status: StatusWaiting, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Waiting for a runner with the following label: freebsd"}, + }, + { + name: "Waiting with labels, no approval", + job: ActionRunJob{RunsOn: []string{"docker", "ubuntu"}, Status: StatusWaiting, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Waiting for a runner with the following labels: docker, ubuntu"}, + }, + { + name: "Waiting with labels, approval", + job: ActionRunJob{RunsOn: []string{"docker", "ubuntu"}, Status: StatusWaiting, Run: &ActionRun{NeedApproval: true}}, + expected: []template.HTML{ + "Waiting for a runner with the following labels: docker, ubuntu", + "Need approval to run workflows for fork pull request.", + }, + }, + { + name: "Running", + job: ActionRunJob{RunsOn: []string{"debian"}, Status: StatusRunning, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Running"}, + }, + { + name: "Success", + job: ActionRunJob{RunsOn: []string{"debian"}, Status: StatusSuccess, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Success"}, + }, + { + name: "Failure", + job: ActionRunJob{RunsOn: []string{"debian"}, Status: StatusFailure, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Failure"}, + }, + { + name: "Cancelled", + job: ActionRunJob{RunsOn: []string{"debian"}, Status: StatusCancelled, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Canceled"}, + }, + { + name: "Skipped", + job: ActionRunJob{RunsOn: []string{"debian"}, Status: StatusSkipped, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Skipped"}, + }, + { + name: "Blocked", + job: ActionRunJob{RunsOn: []string{"debian"}, Status: StatusBlocked, Run: &ActionRun{NeedApproval: false}}, + expected: []template.HTML{"Blocked"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.job.StatusDiagnostics(english)) + }) + } +} diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index edca1072a8..77aa288f48 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -180,6 +180,10 @@ "one": "%s download", "other": "%s downloads" }, + "actions.status.diagnostics.waiting": { + "one": "Waiting for a runner with the following label: %s", + "other": "Waiting for a runner with the following labels: %s" + }, "actions.runs.run_attempt_label": "Run attempt #%[1]s (%[2]s)", "actions.runs.viewing_out_of_date_run": "You are viewing an out-of-date run of this job that was executed %[1]s.", "actions.runs.view_most_recent_run": "View most recent run", diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 55999702d2..d56d993ae5 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -182,10 +182,10 @@ type ViewRunInfo struct { } type ViewCurrentJob struct { - Title string `json:"title"` - Detail string `json:"detail"` - Steps []*ViewJobStep `json:"steps"` - AllAttempts []*TaskAttempt `json:"allAttempts"` + Title string `json:"title"` + Details []template.HTML `json:"details"` + Steps []*ViewJobStep `json:"steps"` + AllAttempts []*TaskAttempt `json:"allAttempts"` } type ViewLogs struct { @@ -358,10 +358,8 @@ func getViewResponse(ctx *context_module.Context, req *ViewRequest, runIndex, jo } resp.State.CurrentJob.Title = current.Name - resp.State.CurrentJob.Detail = current.Status.LocaleString(ctx.Locale) - if run.NeedApproval { - resp.State.CurrentJob.Detail = ctx.Locale.TrString("actions.need_approval_desc") - } + resp.State.CurrentJob.Details = current.StatusDiagnostics(ctx.Locale) + resp.State.CurrentJob.Steps = make([]*ViewJobStep, 0) // marshal to '[]' instead of 'null' in json resp.Logs.StepsLog = make([]*ViewStepLog, 0) // marshal to '[]' instead of 'null' in json // As noted above with TaskID; task will be nil when the job hasn't be picked yet... diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index e3f72e9e37..4e2dab676c 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -181,8 +181,8 @@ func baseExpectedViewResponse() *ViewResponse { }, }, CurrentJob: ViewCurrentJob{ - Title: "job_2", - Detail: "actions.status.success", + Title: "job_2", + Details: []template.HTML{"actions.status.success"}, Steps: []*ViewJobStep{ { Summary: "Set up job", @@ -333,7 +333,7 @@ func TestActionsViewViewPost(t *testing.T) { // Expected blank data in the response because this job isn't picked by a runner yet. Keep details here // in-sync with the RepoActionView 'view non-picked action run job' test. - resp.State.CurrentJob.Detail = "actions.status.waiting" + resp.State.CurrentJob.Details = []template.HTML{"actions.status.diagnostics.waiting"} resp.State.CurrentJob.Steps = []*ViewJobStep{} resp.State.CurrentJob.AllAttempts = nil }, diff --git a/tests/integration/actions_view_test.go b/tests/integration/actions_view_test.go index 8cd643ca4b..8ccaed8f7b 100644 --- a/tests/integration/actions_view_test.go +++ b/tests/integration/actions_view_test.go @@ -153,7 +153,7 @@ func TestActionViewsView(t *testing.T) { re = regexp.MustCompile(pattern) actualClean = re.ReplaceAllString(actualClean, `"time_since_started_html":"_time_"`) - return assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"master\",\"link\":\"/user5/repo4/src/branch/master\",\"isDeleted\":false}}},\"currentJob\":{\"title\":\"job_2\",\"detail\":\"Success\",\"steps\":[{\"summary\":\"Set up job\",\"duration\":\"_duration_\",\"status\":\"success\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"success\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\"},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\"},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\"}]}},\"logs\":{\"stepsLog\":[]}}\n", actualClean) + return assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"master\",\"link\":\"/user5/repo4/src/branch/master\",\"isDeleted\":false}}},\"currentJob\":{\"title\":\"job_2\",\"details\":[\"Success\"],\"steps\":[{\"summary\":\"Set up job\",\"duration\":\"_duration_\",\"status\":\"success\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"success\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\"},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\"},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\"}]}},\"logs\":{\"stepsLog\":[]}}\n", actualClean) }) htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[{\"name\":\"multi-file-download\",\"size\":2048,\"status\":\"completed\"}]}\n") } @@ -185,7 +185,7 @@ func TestActionViewsViewAttemptOutOfRange(t *testing.T) { re = regexp.MustCompile(pattern) actualClean = re.ReplaceAllString(actualClean, `"time_since_started_html":"_time_"`) - return assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"test\",\"link\":\"/user5/repo4/src/branch/test\",\"isDeleted\":true}}},\"currentJob\":{\"title\":\"job_2\",\"detail\":\"Waiting\",\"steps\":[],\"allAttempts\":null}},\"logs\":{\"stepsLog\":[]}}\n", actualClean) + return assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeCommit\":\"Commit\",\"localePushedBy\":\"pushed by\",\"localeWorkflow\":\"Workflow\",\"shortSHA\":\"c2d72f5484\",\"link\":\"/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0\",\"pusher\":{\"displayName\":\"user1\",\"link\":\"/user1\"},\"branch\":{\"name\":\"test\",\"link\":\"/user5/repo4/src/branch/test\",\"isDeleted\":true}}},\"currentJob\":{\"title\":\"job_2\",\"details\":[\"Waiting for a runner with the following label: fedora\"],\"steps\":[],\"allAttempts\":null}},\"logs\":{\"stepsLog\":[]}}\n", actualClean) }) htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[]}\n") } diff --git a/web_src/js/components/RepoActionView.test.js b/web_src/js/components/RepoActionView.test.js index b06751b8cf..71ea39a9c2 100644 --- a/web_src/js/components/RepoActionView.test.js +++ b/web_src/js/components/RepoActionView.test.js @@ -614,7 +614,7 @@ test('view non-picked action run job', async () => { }, currentJob: { title: 'check-1', - detail: 'waiting (locale)', // locale-specific, not exact match to backend test + details: ['waiting (locale)'], // locale-specific, not exact match to backend test steps: [], allAttempts: null, }, @@ -624,7 +624,7 @@ test('view non-picked action run job', async () => { }); await flushPromises(); - expect(wrapper.get('.job-info-header-detail').text()).toEqual('waiting (locale)'); + expect(wrapper.get('.job-info-header-detail li:first-child').text()).toEqual('waiting (locale)'); expect(wrapper.get('.job-brief-list .job-brief-item:nth-of-type(1) .job-brief-name').text()).toEqual('check-1'); expect(wrapper.get('.job-brief-list .job-brief-item:nth-of-type(2) .job-brief-name').text()).toEqual('check-2'); expect(wrapper.get('.job-brief-list .job-brief-item:nth-of-type(3) .job-brief-name').text()).toEqual('check-3'); diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index e8259abd86..88db677243 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -110,7 +110,7 @@ export default { }, currentJob: { title: '', - detail: '', + details: [], steps: [ // { // summary: '', @@ -615,9 +615,11 @@ export default {

{{ currentJob.title }}

-

- {{ currentJob.detail }} -

+
    +
  • + {{ detail }} +
    • +
@@ -932,6 +934,9 @@ export default { .job-info-header .job-info-header-detail { color: var(--color-console-fg-subtle); font-size: 12px; + list-style: none; + padding: 0; + margin: 0; } .job-info-header-left {