diff --git a/models/actions/run_job.go b/models/actions/run_job.go index e64b62805c..1ec917dafc 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -153,6 +153,21 @@ func (job *ActionRunJob) CanBeRerun(ctx context.Context) (bool, error) { return job.Status.IsDone(), nil } +// GetAllAttempts retrieve all the attempts of this job. Limited fields are queried to avoid loading the LogIndexes blob +// when not needed. +func (job *ActionRunJob) GetAllAttempts(ctx context.Context) ([]*ActionTask, error) { + var attempts []*ActionTask + err := db.GetEngine(ctx). + Cols("id", "attempt", "status", "started"). + Where("job_id=?", job.ID). + Desc("attempt"). + Find(&attempts) + if err != nil { + return nil, err + } + return attempts, nil +} + func GetRunJobByID(ctx context.Context, id int64) (*ActionRunJob, error) { var job ActionRunJob has, err := db.GetEngine(ctx).Where("id=?", id).Get(&job) diff --git a/models/actions/run_job_test.go b/models/actions/run_job_test.go index d9275a81ec..9cd2a2804b 100644 --- a/models/actions/run_job_test.go +++ b/models/actions/run_job_test.go @@ -45,7 +45,7 @@ func TestActionRunJob_HTMLURL(t *testing.T) { }{ { id: 192, - expected: "https://try.gitea.io/user5/repo4/actions/runs/187/jobs/0/attempt/1", + expected: "https://try.gitea.io/user5/repo4/actions/runs/187/jobs/0/attempt/3", }, { id: 393, @@ -502,3 +502,22 @@ func TestActionRunJob_CanBeRerun(t *testing.T) { }) } } + +func TestActionTask_GetAllAttempts(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + job2 := unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: 192}) + + allAttempts, err := job2.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") +} diff --git a/models/actions/task.go b/models/actions/task.go index d056c786c4..d7e2b0ef87 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -161,21 +161,6 @@ func (task *ActionTask) UpdateToken(ctx context.Context) error { return UpdateTask(ctx, task, "token_hash", "token_salt", "token_last_eight") } -// 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) diff --git a/models/actions/task_test.go b/models/actions/task_test.go index 7969f52e73..ab88723fe1 100644 --- a/models/actions/task_test.go +++ b/models/actions/task_test.go @@ -6,35 +6,12 @@ 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()) @@ -58,7 +35,7 @@ func TestActionTask_CreatePlaceholderTask(t *testing.T) { assert.NotEqualValues(t, 0, task.ID) assert.Equal(t, job.ID, task.JobID) - assert.EqualValues(t, 0, task.Attempt) + assert.EqualValues(t, 1, task.Attempt) assert.NotEqualValues(t, 0, task.Started) assert.NotEqualValues(t, 0, task.Stopped) assert.Equal(t, job.Status, task.Status) diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index a3afaa8461..f8478d882f 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -6,7 +6,7 @@ commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: false name: job_2 - attempt: 1 + attempt: 3 job_id: job_2 task_id: 47 status: 1 @@ -199,7 +199,7 @@ commit_sha: 985f0301dba5e7b34be866819cd15ad3d8f508ee is_fork_pull_request: false name: job_2 - attempt: 0 + attempt: 1 job_id: job_2 task_id: null status: 5 diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 1f01726e3a..c264c382bd 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -848,6 +848,7 @@ "actions.runs.delete.button": "Delete run", "actions.runs.delete.error": "Could not delete the workflow run.", "actions.runs.delete.confirm_action": "Do you really want to delete this workflow run?", + "actions.jobs.not_started": "not started", "actions.workflow.job_parsing_error": "Unable to parse jobs in workflow: %v", "actions.workflow.event_detection_error": "Unable to parse supported events in workflow: %v", "actions.workflow.persistent_incomplete_matrix": "Unable to evaluate `strategy.matrix` of job %[1]s due to a `needs` expression that was invalid. It may reference a job that is not in it's 'needs' list (%[2]s), or an output that doesn't exist on one of those jobs.", diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index e953947c35..ea33ecbd88 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -349,6 +349,39 @@ func getViewResponse(ctx *app_context.Context, req *ViewRequest, runIndex, jobIn Branch: branch, } + taskAttempts, err := current.GetAllAttempts(ctx) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return nil + } + + var allAttempts []*TaskAttempt + // If the latest attempt has not been taken up yet by a runner, there is no task that could be displayed. As a + // stopgap, inject a phantom task that provides the necessary information until a real tasks is created, if ever. + if len(taskAttempts) == 0 || taskAttempts[0].Attempt != current.Attempt { + taskAttempt := &TaskAttempt{ + Number: current.Attempt, + Status: current.Status.String(), + Started: template.HTML(ctx.Locale.TrString("actions.jobs.not_started")), + StatusDiagnostics: statusDiagnostics(current.Status, current, ctx.Locale), + } + allAttempts = append(allAttempts, taskAttempt) + } + for _, actionTask := range taskAttempts { + taskAttempt := &TaskAttempt{ + Number: actionTask.Attempt, + Started: templates.TimeSince(actionTask.Started), + Status: actionTask.Status.String(), + StatusDiagnostics: statusDiagnostics(actionTask.Status, current, ctx.Locale), + } + allAttempts = append(allAttempts, taskAttempt) + } + + resp.State.CurrentJob.Title = current.Name + resp.State.CurrentJob.Details = statusDiagnostics(current.Status, current, ctx.Locale) + resp.State.CurrentJob.Steps = make([]*ViewJobStep, 0) // marshal to '[]' instead of 'null' in json + resp.State.CurrentJob.AllAttempts = allAttempts + var task *actions_model.ActionTask // TaskID will be set only when the ActionRunJob has been picked by a runner, resulting in an ActionTask being // created representing the specific task. If current.TaskID is not set, then the user is attempting to view a job @@ -369,29 +402,9 @@ func getViewResponse(ctx *app_context.Context, req *ViewRequest, runIndex, jobIn } } - resp.State.CurrentJob.Title = current.Name - resp.State.CurrentJob.Details = statusDiagnostics(current.Status, current, 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 + 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... if task != nil { - taskAttempts, err := task.GetAllAttempts(ctx) - if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) - return nil - } - 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(), - StatusDiagnostics: statusDiagnostics(actionTask.Status, task.Job, ctx.Locale), - } - } - resp.State.CurrentJob.AllAttempts = allAttempts - steps := actions.FullSteps(task) for _, v := range steps { resp.State.CurrentJob.Steps = append(resp.State.CurrentJob.Steps, &ViewJobStep{ diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index 0b66b7f7b8..38e23383bb 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -340,7 +340,14 @@ func TestActionsViewViewPost(t *testing.T) { // in-sync with the RepoActionView 'view non-picked action run job' test. resp.State.CurrentJob.Details = []template.HTML{"actions.status.diagnostics.waiting"} resp.State.CurrentJob.Steps = []*ViewJobStep{} - resp.State.CurrentJob.AllAttempts = nil + resp.State.CurrentJob.AllAttempts = []*TaskAttempt{ + { + Number: 1, + Started: template.HTML("actions.jobs.not_started"), + Status: "waiting", + StatusDiagnostics: []template.HTML{"actions.status.diagnostics.waiting"}, + }, + } }, }, } diff --git a/tests/integration/actions_view_test.go b/tests/integration/actions_view_test.go index 73c07f3bb5..a7efd69277 100644 --- a/tests/integration/actions_view_test.go +++ b/tests/integration/actions_view_test.go @@ -147,8 +147,8 @@ func TestActionViewsView(t *testing.T) { url: "/user5/repo4/actions/runs/187", runIndex: 187, jobIndex: 0, - attempt: 1, - expectedJSON: "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit c2d72f5484 pushed by user1\",\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"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\",\"status_diagnostics\":[\"Running\"]},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]}]}},\"logs\":{\"stepsLog\":[]}}\n", + attempt: 3, + expectedJSON: "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/187\",\"title\":\"update actions\",\"titleHTML\":\"update actions\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit c2d72f5484 pushed by user1\",\"done\":true,\"jobs\":[{\"id\":192,\"name\":\"job_2\",\"status\":\"success\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"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\":\"running\"},{\"summary\":\"Complete job\",\"duration\":\"_duration_\",\"status\":\"waiting\"}],\"allAttempts\":[{\"number\":3,\"time_since_started_html\":\"_time_\",\"status\":\"running\",\"status_diagnostics\":[\"Running\"]},{\"number\":2,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]},{\"number\":1,\"time_since_started_html\":\"_time_\",\"status\":\"success\",\"status_diagnostics\":[\"Success\"]}]}},\"logs\":{\"stepsLog\":[]}}\n", expectedArtifacts: "{\"artifacts\":[{\"name\":\"multi-file-download\",\"size\":2048,\"status\":\"completed\"}]}\n", }, { @@ -230,7 +230,7 @@ func TestActionViewsViewAttemptOutOfRange(t *testing.T) { re = regexp.MustCompile(pattern) actualClean = re.ReplaceAllString(actualClean, `"time_since_started_html":"_time_"`) - assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit c2d72f5484 pushed by user1\",\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"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) + assert.JSONEq(t, "{\"state\":{\"run\":{\"preExecutionError\":\"\",\"link\":\"/user5/repo4/actions/runs/190\",\"title\":\"job output\",\"titleHTML\":\"job output\",\"status\":\"success\",\"canCancel\":false,\"canDelete\":false,\"canApprove\":false,\"canRerun\":false,\"canDeleteArtifact\":false,\"description\":\"Commit c2d72f5484 pushed by user1\",\"done\":false,\"jobs\":[{\"id\":396,\"name\":\"job_2\",\"status\":\"waiting\",\"canRerun\":false,\"duration\":\"_duration_\"}],\"commit\":{\"localeWorkflow\":\"Workflow\",\"localeAllRuns\":\"all runs\",\"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\":[{\"number\":1,\"status\":\"waiting\",\"status_diagnostics\":[\"Waiting for a runner with the following label: fedora\"],\"time_since_started_html\":\"not started\"}]}},\"logs\":{\"stepsLog\":[]}}\n", actualClean) }) htmlDoc.AssertAttrEqual(t, selector, "data-initial-artifacts-response", "{\"artifacts\":[]}\n") }