diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index b0d9346247..a0b3607176 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -112,29 +112,37 @@ type ViewRequest struct { } type ViewResponse struct { - State struct { - Run struct { - Link string `json:"link"` - Title string `json:"title"` - TitleHTML template.HTML `json:"titleHTML"` - Status string `json:"status"` - CanCancel bool `json:"canCancel"` - CanApprove bool `json:"canApprove"` // the run needs an approval and the doer has permission to approve - CanRerun bool `json:"canRerun"` - CanDeleteArtifact bool `json:"canDeleteArtifact"` - Done bool `json:"done"` - Jobs []*ViewJob `json:"jobs"` - Commit ViewCommit `json:"commit"` - } `json:"run"` - CurrentJob struct { - Title string `json:"title"` - Detail string `json:"detail"` - Steps []*ViewJobStep `json:"steps"` - } `json:"currentJob"` - } `json:"state"` - Logs struct { - StepsLog []*ViewStepLog `json:"stepsLog"` - } `json:"logs"` + State ViewState `json:"state"` + Logs ViewLogs `json:"logs"` +} + +type ViewState struct { + Run ViewRunInfo `json:"run"` + CurrentJob ViewCurrentJob `json:"currentJob"` +} + +type ViewRunInfo struct { + Link string `json:"link"` + Title string `json:"title"` + TitleHTML template.HTML `json:"titleHTML"` + Status string `json:"status"` + CanCancel bool `json:"canCancel"` + CanApprove bool `json:"canApprove"` // the run needs an approval and the doer has permission to approve + CanRerun bool `json:"canRerun"` + CanDeleteArtifact bool `json:"canDeleteArtifact"` + Done bool `json:"done"` + Jobs []*ViewJob `json:"jobs"` + Commit ViewCommit `json:"commit"` +} + +type ViewCurrentJob struct { + Title string `json:"title"` + Detail string `json:"detail"` + Steps []*ViewJobStep `json:"steps"` +} + +type ViewLogs struct { + StepsLog []*ViewStepLog `json:"stepsLog"` } type ViewJob struct { @@ -211,10 +219,20 @@ func ViewPost(ctx *context_module.Context) { resp.State.Run.CanApprove = run.NeedApproval && ctx.Repo.CanWrite(unit.TypeActions) resp.State.Run.CanRerun = run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions) resp.State.Run.CanDeleteArtifact = run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions) - resp.State.Run.Done = run.Status.IsDone() resp.State.Run.Jobs = make([]*ViewJob, 0, len(jobs)) // marshal to '[]' instead of 'null' in json resp.State.Run.Status = run.Status.String() + + // It's possible for the run to be marked with a finalized status (eg. failure) because of a single job within the + // run; eg. one job fails, the run fails. But other jobs can still be running. The frontend RepoActionView uses the + // `done` flag to indicate whether to stop querying the run's status -- so even though the run has reached a final + // state, it may not be time to stop polling for updates. + done := run.Status.IsDone() + for _, v := range jobs { + if !v.Status.IsDone() { + // Ah, another job is still running. Keep the frontend polling enabled then. + done = false + } resp.State.Run.Jobs = append(resp.State.Run.Jobs, &ViewJob{ ID: v.ID, Name: v.Name, @@ -223,6 +241,7 @@ func ViewPost(ctx *context_module.Context) { Duration: v.Duration().String(), }) } + resp.State.Run.Done = done pusher := ViewUser{ DisplayName: run.TriggerUser.GetDisplayName(), diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index dae1b47e6e..974906e6f3 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -5,14 +5,19 @@ package actions import ( "fmt" + "html/template" + "net/http" "testing" actions_model "forgejo.org/models/actions" repo_model "forgejo.org/models/repo" unittest "forgejo.org/models/unittest" + "forgejo.org/modules/json" + "forgejo.org/modules/web" "forgejo.org/services/contexttest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_getRunByID(t *testing.T) { @@ -135,3 +140,164 @@ func Test_artifactsFindByNameOrID(t *testing.T) { }) } } + +func baseExpectedResponse() *ViewResponse { + return &ViewResponse{ + State: ViewState{ + Run: ViewRunInfo{ + Link: "/user5/repo4/actions/runs/187", + Title: "update actions", + TitleHTML: template.HTML("update actions"), + Status: "success", + CanCancel: false, + CanApprove: false, + CanRerun: false, + CanDeleteArtifact: false, + Done: true, + Jobs: []*ViewJob{ + { + ID: 192, + Name: "job_2", + Status: "success", + CanRerun: false, + Duration: "1m38s", + }, + }, + Commit: ViewCommit{ + LocaleCommit: "actions.runs.commit", + LocalePushedBy: "actions.runs.pushed_by", + LocaleWorkflow: "actions.runs.workflow", + ShortSha: "c2d72f5484", + Link: "/user5/repo4/commit/c2d72f548424103f01ee1dc02889c1e2bff816b0", + Pusher: ViewUser{ + DisplayName: "user1", + Link: "/user1", + }, + Branch: ViewBranch{ + Name: "master", + Link: "/user5/repo4/src/branch/master", + IsDeleted: false, + }, + }, + }, + CurrentJob: ViewCurrentJob{ + Title: "job_2", + Detail: "actions.status.success", + Steps: []*ViewJobStep{ + { + Summary: "Set up job", + Status: "running", + }, + { + Summary: "Complete job", + Status: "waiting", + }, + }, + }, + }, + Logs: ViewLogs{ + StepsLog: []*ViewStepLog{}, + }, + } +} + +func TestActionsViewViewPost(t *testing.T) { + unittest.PrepareTestEnv(t) + + tests := []struct { + name string + runIndex int64 + jobIndex int64 + expected *ViewResponse + expectedTweaks func(*ViewResponse) + }{ + { + name: "base case", + runIndex: 187, + jobIndex: 0, + expected: baseExpectedResponse(), + expectedTweaks: func(resp *ViewResponse) { + }, + }, + { + name: "run with waiting jobs", + runIndex: 189, + jobIndex: 0, + expected: baseExpectedResponse(), + 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" + resp.State.Run.Title = "job output" + resp.State.Run.TitleHTML = "job output" + resp.State.Run.Jobs = []*ViewJob{ + { + ID: 194, + Name: "job1 (1)", + Status: "success", + }, + { + ID: 195, + Name: "job1 (2)", + Status: "success", + }, + { + ID: 196, + Name: "job2", + Status: "waiting", + }, + } + resp.State.CurrentJob.Title = "job1 (1)" + resp.State.CurrentJob.Steps = []*ViewJobStep{ + { + Summary: "Set up job", + Status: "success", + }, + { + Summary: "Complete job", + 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 + // UI to continue refreshing the page. + resp.State.Run.Done = false + }, + }, + } + + 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, 4) + ctx.SetParams(":run", fmt.Sprintf("%d", tt.runIndex)) + ctx.SetParams(":job", fmt.Sprintf("%d", tt.jobIndex)) + web.SetForm(ctx, &ViewRequest{}) + + ViewPost(ctx) + require.Equal(t, http.StatusOK, resp.Result().StatusCode) + + var actual ViewResponse + err := json.Unmarshal(resp.Body.Bytes(), &actual) + require.NoError(t, err) + + // `Duration` field is dynamic based upon current time, so eliminate it from comparison -- but check that it + // has the right format at least. + zeroDurations := func(vr *ViewResponse) { + for _, job := range vr.State.Run.Jobs { + assert.Regexp(t, `^(\d+[hms]){1,3}$`, job.Duration) + job.Duration = "" + } + for _, step := range vr.State.CurrentJob.Steps { + step.Duration = "" + } + } + zeroDurations(&actual) + zeroDurations(tt.expected) + tt.expectedTweaks(tt.expected) + + assert.Equal(t, *tt.expected, actual) + }) + } +}