From 8e813902c553c7bff983cf3c601aac7b77055219 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Thu, 4 Sep 2025 22:46:22 +0200 Subject: [PATCH] feat: ability to view previous logs for Actions runs that have been retried (#9017) Adds a new dropdown to the job logs, visible only when a job has been retried at least once: ![action-with-dropdown](/attachments/9669b47e-2239-4f07-b823-2759dd99a4fb) When an older run attempt from the dropdown is selected, displays the older run's logs: ![historical-action-logs](/attachments/8b737386-63fb-4f3f-b5b5-ac38c62ed648) Context on implementation & design decisions: - It is important that when a URL from an Action's log is shared, the person on the other side sees the exact same logs that were being viewed. For this reason, all log views are automatically redirected to a fully-qualified URL (including the *run*, *job*, and *attempt*), so that when they are shared there is a guarantee of stability in the viewed logs. - Individual jobs can be rerun any number of times independent of other jobs in the same workflow. This means there isn't a "set" of related jobs that were executed at the same time, and this led me to remove the display of current status of jobs on the left-hand side of the view. There isn't a logical set of job statuses to display here. Fixes #1043. Based upon @gmem's original work in #1416. ## 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. - [ ] 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. - [x] 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 - User Interface features - [PR](https://codeberg.org/forgejo/forgejo/pulls/9017): ability to view previous logs for Actions runs that have been retried Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9017 Reviewed-by: Earl Warren Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- Makefile | 4 +- models/actions/main_test.go | 4 + models/actions/run_job.go | 25 +++ models/actions/run_job_test.go | 43 ++++ models/actions/task.go | 27 +++ models/actions/task_test.go | 48 +++++ models/fixtures/action_run_job.yml | 2 +- models/fixtures/action_task.yml | 40 ++++ options/locale_next/locale_en-US.json | 3 + routers/web/repo/actions/view.go | 56 +++++- routers/web/repo/actions/view_test.go | 129 +++++++++++- routers/web/web.go | 7 +- templates/repo/actions/view.tmpl | 4 + tests/integration/actions_route_test.go | 5 + tests/integration/actions_view_test.go | 30 +++ web_src/js/components/ActionRunStatus.vue | 11 +- web_src/js/components/RepoActionView.test.js | 201 +++++++++++++++++++ web_src/js/components/RepoActionView.vue | 164 +++++++++++++-- 18 files changed, 765 insertions(+), 38 deletions(-) create mode 100644 models/actions/task_test.go diff --git a/Makefile b/Makefile index f6b84c6b2c..e68b8b1127 100644 --- a/Makefile +++ b/Makefile @@ -527,7 +527,7 @@ test: test-frontend test-backend .PHONY: test-backend test-backend: @echo "Running go test with $(GOTESTFLAGS) -tags '$(TEST_TAGS)'..." - @$(GOTEST) $(GOTESTFLAGS) -tags='$(TEST_TAGS)' $(GO_TEST_PACKAGES) + @TZ=UTC $(GOTEST) $(GOTESTFLAGS) -tags='$(TEST_TAGS)' $(GO_TEST_PACKAGES) .PHONY: test-remote-cacher test-remote-cacher: @@ -557,7 +557,7 @@ test-check: .PHONY: test\#% test\#%: @echo "Running go test with $(GOTESTFLAGS) -tags '$(TEST_TAGS)'..." - @$(GOTEST) $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -run $(subst .,/,$*) $(GO_TEST_PACKAGES) + @TZ=UTC $(GOTEST) $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -run $(subst .,/,$*) $(GO_TEST_PACKAGES) coverage-merge: rm -fr coverage/merged ; mkdir -p coverage/merged diff --git a/models/actions/main_test.go b/models/actions/main_test.go index 2eb923d9d0..f551d39671 100644 --- a/models/actions/main_test.go +++ b/models/actions/main_test.go @@ -15,6 +15,10 @@ func TestMain(m *testing.M) { "action_runner.yml", "repository.yml", "action_runner_token.yml", + "user.yml", + "action_run.yml", + "action_run_job.yml", + "action_task.yml", }, }) } diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 1fadb4b7c7..c7ab93d2c6 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -44,6 +44,31 @@ func init() { db.RegisterModel(new(ActionRunJob)) } +func (job *ActionRunJob) HTMLURL(ctx context.Context) (string, error) { + if job.Run == nil || job.Run.Repo == nil { + return "", fmt.Errorf("action_run_job: load run and repo before accessing HTMLURL") + } + + // Find the "index" of the currently selected job... kinda ugly that the URL uses the index rather than some other + // unique identifier of the job which could actually be stored upon it. But hard to change that now. + allJobs, err := GetRunJobsByRunID(ctx, job.RunID) + if err != nil { + return "", err + } + jobIndex := -1 + for i, otherJob := range allJobs { + if job.ID == otherJob.ID { + jobIndex = i + break + } + } + if jobIndex == -1 { + return "", fmt.Errorf("action_run_job: unable to find job on run: %d", job.ID) + } + + return fmt.Sprintf("%s/actions/runs/%d/jobs/%d/attempt/%d", job.Run.Repo.HTMLURL(), job.Run.Index, jobIndex, job.Attempt), nil +} + func (job *ActionRunJob) Duration() time.Duration { return calculateDuration(job.Started, job.Stopped, job.Status) } diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index 50a4ba10d8..6abdb2bf5c 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -3,9 +3,14 @@ package actions import ( + "fmt" "testing" + "forgejo.org/models/db" + "forgejo.org/models/unittest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestActionRunJob_ItRunsOn(t *testing.T) { @@ -27,3 +32,41 @@ func TestActionRunJob_ItRunsOn(t *testing.T) { assert.False(t, actionJob.ItRunsOn(agentLabels)) } + +func TestActionRunJob_HTMLURL(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + tests := []struct { + id int64 + expected string + }{ + { + id: 192, + expected: "https://try.gitea.io/user5/repo4/actions/runs/187/jobs/0/attempt/1", + }, + { + id: 393, + expected: "https://try.gitea.io/user2/repo1/actions/runs/187/jobs/1/attempt/1", + }, + { + id: 394, + expected: "https://try.gitea.io/user2/repo1/actions/runs/187/jobs/2/attempt/2", + }, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("id=%d", tt.id), func(t *testing.T) { + var job ActionRunJob + has, err := db.GetEngine(t.Context()).Where("id=?", tt.id).Get(&job) + require.NoError(t, err) + require.True(t, has, "load ActionRunJob from fixture") + + err = job.LoadAttributes(t.Context()) + require.NoError(t, err) + + url, err := job.HTMLURL(t.Context()) + require.NoError(t, err) + assert.Equal(t, tt.expected, url) + }) + } +} diff --git a/models/actions/task.go b/models/actions/task.go index 88b30196e3..8a1c7d2f83 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -148,6 +148,21 @@ func (task *ActionTask) GenerateToken() (err error) { return err } +// Retrieve all the attempts from the same job as the target `ActionTask`. Limited fields are queried to avoid loading +// the LogIndexes blob when not needed. +func (task *ActionTask) GetAllAttempts(ctx context.Context) ([]*ActionTask, error) { + var attempts []*ActionTask + err := db.GetEngine(ctx). + Cols("id", "attempt", "status", "started"). + Where("job_id=?", task.JobID). + Desc("attempt"). + Find(&attempts) + if err != nil { + return nil, err + } + return attempts, nil +} + func GetTaskByID(ctx context.Context, id int64) (*ActionTask, error) { var task ActionTask has, err := db.GetEngine(ctx).Where("id=?", id).Get(&task) @@ -160,6 +175,18 @@ func GetTaskByID(ctx context.Context, id int64) (*ActionTask, error) { return &task, nil } +func GetTaskByJobAttempt(ctx context.Context, jobID, attempt int64) (*ActionTask, error) { + var task ActionTask + has, err := db.GetEngine(ctx).Where("job_id=?", jobID).Where("attempt=?", attempt).Get(&task) + if err != nil { + return nil, err + } else if !has { + return nil, fmt.Errorf("task with job_id %d and attempt %d: %w", jobID, attempt, util.ErrNotExist) + } + + return &task, nil +} + func GetRunningTaskByToken(ctx context.Context, token string) (*ActionTask, error) { errNotExist := fmt.Errorf("task with token %q: %w", token, util.ErrNotExist) if token == "" { diff --git a/models/actions/task_test.go b/models/actions/task_test.go new file mode 100644 index 0000000000..73aff17a85 --- /dev/null +++ b/models/actions/task_test.go @@ -0,0 +1,48 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + + "forgejo.org/models/db" + "forgejo.org/models/unittest" + "forgejo.org/modules/timeutil" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActionTask_GetAllAttempts(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + var task ActionTask + has, err := db.GetEngine(t.Context()).Where("id=?", 47).Get(&task) + require.NoError(t, err) + require.True(t, has, "load ActionTask from fixture") + + allAttempts, err := task.GetAllAttempts(t.Context()) + require.NoError(t, err) + require.Len(t, allAttempts, 3) + assert.EqualValues(t, 47, allAttempts[0].ID, "ordered by attempt, 1") + assert.EqualValues(t, 53, allAttempts[1].ID, "ordered by attempt, 2") + assert.EqualValues(t, 52, allAttempts[2].ID, "ordered by attempt, 3") + + // GetAllAttempts doesn't populate all fields; so check expected fields from one of the records + assert.EqualValues(t, 3, allAttempts[0].Attempt, "read Attempt field") + assert.Equal(t, StatusRunning, allAttempts[0].Status, "read Status field") + assert.Equal(t, timeutil.TimeStamp(1683636528), allAttempts[0].Started, "read Started field") +} + +func TestActionTask_GetTaskByJobAttempt(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + task, err := GetTaskByJobAttempt(t.Context(), 192, 2) + require.NoError(t, err) + assert.EqualValues(t, 192, task.JobID) + assert.EqualValues(t, 2, task.Attempt) + + _, err = GetTaskByJobAttempt(t.Context(), 192, 100) + assert.ErrorContains(t, err, "task with job_id 192 and attempt 100: resource does not exist") +} diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index 702c6bc832..9455ac3c41 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -106,7 +106,7 @@ commit_sha: 985f0301dba5e7b34be866819cd15ad3d8f508ee is_fork_pull_request: 0 name: job_2 - attempt: 1 + attempt: 2 job_id: job_2 task_id: 47 status: 5 diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index 506a47d8a0..e5fa35f0b3 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -117,3 +117,43 @@ log_length: 707 log_size: 90179 log_expired: 0 +- + id: 52 + job_id: 192 + attempt: 1 + runner_id: 1 + status: 1 # success + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784223 + token_salt: ffffffffff + token_last_eight: ffffffff + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 +- + id: 53 + job_id: 192 + attempt: 2 + runner_id: 1 + status: 1 # success + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784224 + token_salt: ffffffffff + token_last_eight: ffffffff + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index e520e657e1..41fd09be40 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -145,5 +145,8 @@ "migrate.pagure.token_label": "Token", "migrate.pagure.token_body_a": "Provide a Pagure API token with access to the private issues to create a repository with just the private issues in it", "migrate.pagure.token_body_b": "Be sure to set the private repo flag above if you want this repo to be private", + "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", "meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it." } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index a0b3607176..e2919bcb00 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -40,10 +40,31 @@ import ( "xorm.io/builder" ) +func RedirectToLatestAttempt(ctx *context_module.Context) { + runIndex := ctx.ParamsInt64("run") + jobIndex := ctx.ParamsInt64("job") + + job, _ := getRunJobs(ctx, runIndex, jobIndex) + if ctx.Written() { + return + } + + jobURL, err := job.HTMLURL(ctx) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + ctx.Redirect(jobURL, http.StatusTemporaryRedirect) +} + func View(ctx *context_module.Context) { ctx.Data["PageIsActions"] = true runIndex := ctx.ParamsInt64("run") jobIndex := ctx.ParamsInt64("job") + // note: this is `attemptNumber` not `attemptIndex` since this value has to matches the ActionTask's Attempt field + // which uses 1-based numbering... would be confusing as "Index" if it later can't be used to index an slice/array. + attemptNumber := ctx.ParamsInt64("attempt") job, _ := getRunJobs(ctx, runIndex, jobIndex) if ctx.Written() { @@ -56,6 +77,7 @@ func View(ctx *context_module.Context) { ctx.Data["RunID"] = job.Run.ID ctx.Data["JobIndex"] = jobIndex ctx.Data["ActionsURL"] = ctx.Repo.RepoLink + "/actions" + ctx.Data["AttemptNumber"] = attemptNumber ctx.Data["WorkflowName"] = workflowName ctx.Data["WorkflowURL"] = ctx.Repo.RepoLink + "/actions?workflow=" + workflowName @@ -136,9 +158,10 @@ type ViewRunInfo struct { } type ViewCurrentJob struct { - Title string `json:"title"` - Detail string `json:"detail"` - Steps []*ViewJobStep `json:"steps"` + Title string `json:"title"` + Detail string `json:"detail"` + Steps []*ViewJobStep `json:"steps"` + AllAttempts []*TaskAttempt `json:"allAttempts"` } type ViewLogs struct { @@ -193,10 +216,19 @@ type ViewStepLogLine struct { Timestamp float64 `json:"timestamp"` } +type TaskAttempt struct { + Number int64 `json:"number"` + Started template.HTML `json:"time_since_started_html"` + Status string `json:"status"` +} + func ViewPost(ctx *context_module.Context) { req := web.GetForm(ctx).(*ViewRequest) runIndex := ctx.ParamsInt64("run") jobIndex := ctx.ParamsInt64("job") + // note: this is `attemptNumber` not `attemptIndex` since this value has to matches the ActionTask's Attempt field + // which uses 1-based numbering... would be confusing as "Index" if it later can't be used to index an slice/array. + attemptNumber := ctx.ParamsInt64("attempt") current, jobs := getRunJobs(ctx, runIndex, jobIndex) if ctx.Written() { @@ -274,7 +306,7 @@ func ViewPost(ctx *context_module.Context) { var task *actions_model.ActionTask if current.TaskID > 0 { var err error - task, err = actions_model.GetTaskByID(ctx, current.TaskID) + task, err = actions_model.GetTaskByJobAttempt(ctx, current.ID, attemptNumber) if err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return @@ -294,8 +326,22 @@ func ViewPost(ctx *context_module.Context) { 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 if task != nil { - steps := actions.FullSteps(task) + taskAttempts, err := task.GetAllAttempts(ctx) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + allAttempts := make([]*TaskAttempt, len(taskAttempts)) + for i, actionTask := range taskAttempts { + allAttempts[i] = &TaskAttempt{ + Number: actionTask.Attempt, + Started: templates.TimeSince(actionTask.Started), + Status: actionTask.Status.String(), + } + } + resp.State.CurrentJob.AllAttempts = allAttempts + steps := actions.FullSteps(task) for _, v := range steps { resp.State.CurrentJob.Steps = append(resp.State.CurrentJob.Steps, &ViewJobStep{ Summary: v.Name, diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index 974906e6f3..68b981211e 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -141,7 +141,7 @@ func Test_artifactsFindByNameOrID(t *testing.T) { } } -func baseExpectedResponse() *ViewResponse { +func baseExpectedViewResponse() *ViewResponse { return &ViewResponse{ State: ViewState{ Run: ViewRunInfo{ @@ -193,6 +193,23 @@ func baseExpectedResponse() *ViewResponse { Status: "waiting", }, }, + AllAttempts: []*TaskAttempt{ + { + Number: 3, + Started: template.HTML("2023-05-09 12:48:48 +00:00"), + Status: "running", + }, + { + Number: 2, + Started: template.HTML("2023-05-09 12:48:48 +00:00"), + Status: "success", + }, + { + Number: 1, + Started: template.HTML("2023-05-09 12:48:48 +00:00"), + Status: "success", + }, + }, }, }, Logs: ViewLogs{ @@ -208,22 +225,27 @@ func TestActionsViewViewPost(t *testing.T) { name string runIndex int64 jobIndex int64 + attemptNumber int64 expected *ViewResponse expectedTweaks func(*ViewResponse) }{ { - name: "base case", - runIndex: 187, - jobIndex: 0, - expected: baseExpectedResponse(), + name: "base case", + runIndex: 187, + jobIndex: 0, + attemptNumber: 1, + expected: baseExpectedViewResponse(), expectedTweaks: func(resp *ViewResponse) { + resp.State.CurrentJob.Steps[0].Status = "success" + resp.State.CurrentJob.Steps[1].Status = "success" }, }, { - name: "run with waiting jobs", - runIndex: 189, - jobIndex: 0, - expected: baseExpectedResponse(), + name: "run with waiting jobs", + runIndex: 189, + jobIndex: 0, + attemptNumber: 1, + expected: baseExpectedViewResponse(), expectedTweaks: func(resp *ViewResponse) { // Variations from runIndex 187 -> runIndex 189 that are not the subject of this test... resp.State.Run.Link = "/user5/repo4/actions/runs/189" @@ -257,6 +279,13 @@ func TestActionsViewViewPost(t *testing.T) { Status: "success", }, } + resp.State.CurrentJob.AllAttempts = []*TaskAttempt{ + { + Number: 1, + Started: template.HTML("2023-05-09 12:48:48 +00:00"), + Status: "success", + }, + } // Under test in this case: verify that Done is set to false; in the fixture data, job.ID=195 is status // Success, but job.ID=196 is status Waiting, and so we expect to signal Done=false to indicate to the @@ -264,6 +293,17 @@ func TestActionsViewViewPost(t *testing.T) { resp.State.Run.Done = false }, }, + { + name: "attempt 3", + runIndex: 187, + jobIndex: 0, + attemptNumber: 3, + expected: baseExpectedViewResponse(), + expectedTweaks: func(resp *ViewResponse) { + resp.State.CurrentJob.Steps[0].Status = "running" + resp.State.CurrentJob.Steps[1].Status = "waiting" + }, + }, } for _, tt := range tests { @@ -273,10 +313,11 @@ func TestActionsViewViewPost(t *testing.T) { contexttest.LoadRepo(t, ctx, 4) ctx.SetParams(":run", fmt.Sprintf("%d", tt.runIndex)) ctx.SetParams(":job", fmt.Sprintf("%d", tt.jobIndex)) + ctx.SetParams(":attempt", fmt.Sprintf("%d", tt.attemptNumber)) web.SetForm(ctx, &ViewRequest{}) ViewPost(ctx) - require.Equal(t, http.StatusOK, resp.Result().StatusCode) + require.Equal(t, http.StatusOK, resp.Result().StatusCode, "failure in ViewPost(): %q", resp.Body.String()) var actual ViewResponse err := json.Unmarshal(resp.Body.Bytes(), &actual) @@ -301,3 +342,71 @@ func TestActionsViewViewPost(t *testing.T) { }) } } + +func TestActionsViewRedirectToLatestAttempt(t *testing.T) { + unittest.PrepareTestEnv(t) + + tests := []struct { + name string + runIndex int64 + jobIndex int64 + expectedCode int + expectedURL string + }{ + { + name: "no job index", + runIndex: 187, + jobIndex: -1, + expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/187/jobs/0/attempt/1", + }, + { + name: "job w/ 1 attempt", + runIndex: 187, + jobIndex: 0, + expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/187/jobs/0/attempt/1", + }, + { + name: "job w/ multiple attempts", + runIndex: 187, + jobIndex: 2, + expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/187/jobs/2/attempt/2", + }, + { + name: "run out-of-range", + runIndex: 5000, + jobIndex: -1, + expectedCode: http.StatusNotFound, + }, + // Odd behavior with an out-of-bound jobIndex -- defaults to the first job. This is existing behavior + // documented in the getRunJobs internal helper which... seems not perfect for the redirect... but it's high + // risk to change and it's an OK user outcome to be redirected to something valid in the requested run. + { + name: "job out-of-range", + runIndex: 187, + jobIndex: 500, + expectedURL: "https://try.gitea.io/user2/repo1/actions/runs/187/jobs/0/attempt/1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, resp := contexttest.MockContext(t, "user2/repo1/actions/runs/0") + contexttest.LoadUser(t, ctx, 2) + contexttest.LoadRepo(t, ctx, 1) + ctx.SetParams(":run", fmt.Sprintf("%d", tt.runIndex)) + if tt.jobIndex != -1 { + ctx.SetParams(":job", fmt.Sprintf("%d", tt.jobIndex)) + } + + RedirectToLatestAttempt(ctx) + if tt.expectedCode == 0 { + assert.Equal(t, http.StatusTemporaryRedirect, resp.Code) + url, err := resp.Result().Location() + require.NoError(t, err) + assert.Equal(t, tt.expectedURL, url.String()) + } else { + assert.Equal(t, tt.expectedCode, resp.Code) + } + }) + } +} diff --git a/routers/web/web.go b/routers/web/web.go index 43ce0dba6d..20d5376cfe 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1444,14 +1444,17 @@ func registerRoutes(m *web.Route) { m.Get("/latest", actions.ViewLatest) m.Group("/{run}", func() { m.Combo(""). - Get(actions.View). + Get(actions.RedirectToLatestAttempt). Post(web.Bind(actions.ViewRequest{}), actions.ViewPost) m.Group("/jobs/{job}", func() { m.Combo(""). - Get(actions.View). + Get(actions.RedirectToLatestAttempt). Post(web.Bind(actions.ViewRequest{}), actions.ViewPost) m.Post("/rerun", reqRepoActionsWriter, actions.Rerun) m.Get("/logs", actions.Logs) + m.Combo("/attempt/{attempt}"). + Get(actions.View). + Post(web.Bind(actions.ViewRequest{}), actions.ViewPost) }) m.Post("/cancel", reqRepoActionsWriter, actions.Cancel) m.Post("/approve", reqRepoActionsWriter, actions.Approve) diff --git a/templates/repo/actions/view.tmpl b/templates/repo/actions/view.tmpl index abd3c5c764..81ed5ec1c9 100644 --- a/templates/repo/actions/view.tmpl +++ b/templates/repo/actions/view.tmpl @@ -6,6 +6,7 @@ data-run-index="{{.RunIndex}}" data-run-id="{{.RunID}}" data-job-index="{{.JobIndex}}" + data-attempt-number="{{.AttemptNumber}}" data-actions-url="{{.ActionsURL}}" data-workflow-name="{{.WorkflowName}}" data-workflow-url="{{.WorkflowURL}}" @@ -27,6 +28,9 @@ data-locale-show-log-seconds="{{ctx.Locale.Tr "show_log_seconds"}}" data-locale-show-full-screen="{{ctx.Locale.Tr "show_full_screen"}}" data-locale-download-logs="{{ctx.Locale.Tr "download_logs"}}" + data-locale-run-attempt-label="{{ctx.Locale.Tr "actions.runs.run_attempt_label"}}" + data-locale-viewing-out-of-date-run="{{ctx.Locale.Tr "actions.runs.viewing_out_of_date_run"}}" + data-locale-view-most-recent-run="{{ctx.Locale.Tr "actions.runs.view_most_recent_run"}}" > diff --git a/tests/integration/actions_route_test.go b/tests/integration/actions_route_test.go index c058877806..930a917524 100644 --- a/tests/integration/actions_route_test.go +++ b/tests/integration/actions_route_test.go @@ -90,7 +90,12 @@ func TestActionsWebRouteLatestWorkflowRun(t *testing.T) { // Fetch the page that shows information about the run initiated by "workflow-1.yml". // routers/web/repo/actions/view.go: data-workflow-url is constructed using data-workflow-name. req := NewRequest(t, "GET", workflowOneURI) + intermediateRedirect := MakeRequest(t, req, http.StatusTemporaryRedirect) + + finalURL := intermediateRedirect.Result().Header.Get("Location") + req = NewRequest(t, "GET", finalURL) resp := MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) // Verify that URL of the workflow is shown correctly. diff --git a/tests/integration/actions_view_test.go b/tests/integration/actions_view_test.go index 14ef123674..a518f0796d 100644 --- a/tests/integration/actions_view_test.go +++ b/tests/integration/actions_view_test.go @@ -49,6 +49,10 @@ func TestActionsViewArtifactDeletion(t *testing.T) { // Visit it's web view req := NewRequest(t, "GET", run.HTMLURL()) + intermediateRedirect := MakeRequest(t, req, http.StatusTemporaryRedirect) + + finalURL := intermediateRedirect.Result().Header.Get("Location") + req = NewRequest(t, "GET", finalURL) resp := MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) @@ -78,6 +82,10 @@ func TestActionViewsArtifactDownload(t *testing.T) { assert.JSONEq(t, `{"artifacts":[{"name":"multi-file-download","size":2048,"status":"completed"}]}`, strings.TrimSuffix(resp.Body.String(), "\n")) req = NewRequest(t, "GET", fmt.Sprintf("/user5/repo4/actions/runs/%d", runIndex)) + intermediateRedirect := MakeRequest(t, req, http.StatusTemporaryRedirect) + + finalURL := intermediateRedirect.Result().Header.Get("Location") + req = NewRequest(t, "GET", finalURL) resp = MakeRequest(t, req, http.StatusOK) assertDataAttrs(t, resp.Body, runID) @@ -95,6 +103,10 @@ func TestActionViewsArtifactDownload(t *testing.T) { assert.JSONEq(t, `{"artifacts":[{"name":"artifact-v4-download","size":1024,"status":"completed"}]}`, strings.TrimSuffix(resp.Body.String(), "\n")) req = NewRequest(t, "GET", fmt.Sprintf("/user5/repo4/actions/runs/%d", runIndex)) + intermediateRedirect := MakeRequest(t, req, http.StatusTemporaryRedirect) + + finalURL := intermediateRedirect.Result().Header.Get("Location") + req = NewRequest(t, "GET", finalURL) resp = MakeRequest(t, req, http.StatusOK) assertDataAttrs(t, resp.Body, runID) @@ -112,3 +124,21 @@ func TestActionViewsArtifactDownload(t *testing.T) { assert.Equal(t, strings.Repeat("D", 100), resp.Body.String()) }) } + +func TestActionViewsView(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + req := NewRequest(t, "GET", "/user2/repo1/actions/runs/187") + intermediateRedirect := MakeRequest(t, req, http.StatusTemporaryRedirect) + + finalURL := intermediateRedirect.Result().Header.Get("Location") + req = NewRequest(t, "GET", finalURL) + resp := MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + selector := "#repo-action-view" + // Verify key properties going into the `repo-action-view` to initialize the Vue component. + htmlDoc.AssertAttrEqual(t, selector, "data-run-index", "187") + htmlDoc.AssertAttrEqual(t, selector, "data-job-index", "0") + htmlDoc.AssertAttrEqual(t, selector, "data-attempt-number", "1") +} diff --git a/web_src/js/components/ActionRunStatus.vue b/web_src/js/components/ActionRunStatus.vue index db380f0038..25d86f1522 100644 --- a/web_src/js/components/ActionRunStatus.vue +++ b/web_src/js/components/ActionRunStatus.vue @@ -24,11 +24,20 @@ export default { type: String, default: '', }, + inline: { + type: Boolean, + default: false, + }, + }, + computed: { + containerClasses() { + return this.inline ? 'tw-inline' : 'tw-flex tw-items-center'; + }, }, };