mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-02-03 20:51:07 -05:00
feat: display detailed action run diagnostics (#9966)
Forgejo Actions allows variables in `jobs.<job_id>.runs-on`. However, the action list [skips checking whether a suitable runner](c3412d0579/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:
How it looks after updating Forgejo without reloading the window:
How it looks afterwards with a single label:
How it looks afterwards with multiple labels:
## 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/<pull request number>.md` to be be used for the release notes instead of the title.
<!--start release-notes-assistant-->
## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Features
- [PR](https://codeberg.org/forgejo/forgejo/pulls/9966): <!--number 9966 --><!--line 0 --><!--description ZGlzcGxheSBkZXRhaWxlZCBhY3Rpb24gcnVuIGRpYWdub3N0aWNz-->display detailed action run diagnostics<!--description-->
<!--end release-notes-assistant-->
Co-authored-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9966
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
Co-committed-by: Andreas Ahlenstorf <andreas@ahlenstorf.ch>
This commit is contained in:
Andreas Ahlenstorf
Mathieu Fenniak
parent
b3f1f115f7
commit
72b35c5a73
8 changed files with 128 additions and 19 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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...
|
||||
|
|
|
|||
|
|
@ -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
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -110,7 +110,7 @@ export default {
|
|||
},
|
||||
currentJob: {
|
||||
title: '',
|
||||
detail: '',
|
||||
details: [],
|
||||
steps: [
|
||||
// {
|
||||
// summary: '',
|
||||
|
|
@ -615,9 +615,11 @@ export default {
|
|||
<h3 class="job-info-header-title gt-ellipsis">
|
||||
{{ currentJob.title }}
|
||||
</h3>
|
||||
<p class="job-info-header-detail">
|
||||
{{ currentJob.detail }}
|
||||
</p>
|
||||
<ul class="job-info-header-detail">
|
||||
<li v-for="detail in currentJob.details" :key="detail">
|
||||
{{ detail }}
|
||||
</li>
|
||||
</ul>
|
||||
</div>
|
||||
<div class="job-info-header-right job-attempt-dropdown tw-mr-8" v-if="shouldShowAttemptDropdown" v-cloak>
|
||||
<div class="ui dropdown selection" @click.stop="toggleAttemptDropdown()">
|
||||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Reference in a new issue