diff --git a/models/actions/run.go b/models/actions/run.go index 733d21b0cfa..be291715012 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/url" + "slices" "strconv" "time" @@ -78,6 +79,11 @@ func init() { db.RegisterModel(new(ActionRunIndex)) } +// IsAwaitingApproval reports whether approval can still release the run +func (run *ActionRun) IsAwaitingApproval() bool { + return run.NeedApproval && !run.Status.IsDone() +} + func (run *ActionRun) HTMLURL(ctxOpt ...context.Context) string { if run.Repo == nil { return "" @@ -395,6 +401,7 @@ func CancelPreviousJobsByRunConcurrency(ctx context.Context, attempt *ActionRunA if err != nil { return nil, fmt.Errorf("find concurrent runs and jobs: %w", err) } + jobs = slices.DeleteFunc(jobs, func(job *ActionRunJob) bool { return job.RunID == attempt.RunID }) jobsToCancel = append(jobsToCancel, jobs...) // cancel runs in the same concurrency group diff --git a/routers/api/v1/repo/actions_run.go b/routers/api/v1/repo/actions_run.go index 1fd9e387db1..90c14d03a1d 100644 --- a/routers/api/v1/repo/actions_run.go +++ b/routers/api/v1/repo/actions_run.go @@ -196,11 +196,11 @@ func ApproveWorkflowRun(ctx *context.APIContext) { return } - if !run.NeedApproval { + if !run.IsAwaitingApproval() { // Approving twice is idempotent, but a run that never awaited approval gets 409 rather // than GitHub's 403, which would be indistinguishable from a permission denial. if run.ApprovedBy == 0 { - ctx.APIError(http.StatusConflict, "run does not require approval") + ctx.APIError(http.StatusConflict, "run is not waiting for approval") return } respondRepoActionWorkflowRun(ctx, run) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 47ceceddf68..af8676602ca 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -594,7 +594,7 @@ func fillViewRunResponseSummary(ctx *context_module.Context, resp *ViewResponse, // Hide the Cancel button once a cancel is already in cancelling progress resp.State.Run.CanCancel = isLatestAttempt && !resp.State.Run.Done && !effectiveStatus.IsCancelling() && ctx.Repo.Permission.CanWrite(unit.TypeActions) - resp.State.Run.CanApprove = isLatestAttempt && run.NeedApproval && ctx.Repo.Permission.CanWrite(unit.TypeActions) + resp.State.Run.CanApprove = isLatestAttempt && run.IsAwaitingApproval() && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanRerun = isLatestAttempt && resp.State.Run.Done && len(jobs) > 0 && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanDeleteArtifact = resp.State.Run.Done && ctx.Repo.Permission.CanWrite(unit.TypeActions) if resp.State.Run.CanRerun { @@ -748,7 +748,7 @@ func fillViewRunResponseCurrentJob(ctx *context_module.Context, resp *ViewRespon resp.State.CurrentJob.Title = current.Name resp.State.CurrentJob.Detail = current.Status.LocaleString(ctx.Locale) - if run.NeedApproval { + if run.IsAwaitingApproval() { resp.State.CurrentJob.Detail = ctx.Locale.TrString("actions.need_approval_desc") } else if detail := describePendingJobDetail(ctx, current, jobs); detail != "" { resp.State.CurrentJob.Detail = detail @@ -1320,7 +1320,7 @@ func ApproveAllChecks(ctx *context_module.Context) { runIDs := make([]int64, 0, len(runs)) for _, run := range runs { - if run.NeedApproval { + if run.IsAwaitingApproval() { runIDs = append(runIDs, run.ID) } } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0605d5210e0..e99667a0d14 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -443,7 +443,7 @@ func (prInfo *pullRequestViewInfo) prepareMergeBoxStatusCheckData(ctx *context.C log.Error("GetRunsFromCommitStatuses: %v", err) } for _, run := range runs { - if run.NeedApproval { + if run.IsAwaitingApproval() { statusCheckData.RequireApprovalRunCount++ } } diff --git a/services/actions/approve.go b/services/actions/approve.go index 1c58b908fa1..327072aa062 100644 --- a/services/actions/approve.go +++ b/services/actions/approve.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "slices" actions_model "gitea.dev/models/actions" "gitea.dev/models/db" @@ -15,13 +16,15 @@ import ( "gitea.dev/modules/container" "gitea.dev/modules/log" "gitea.dev/modules/util" + + "xorm.io/builder" ) // ApproveRuns returns the approved runs in the same order as runIDs. func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, runIDs []int64) ([]*actions_model.ActionRun, error) { updatedJobs := make([]*actions_model.ActionRunJob, 0) cancelledConcurrencyJobs := make([]*actions_model.ActionRunJob, 0) - runIDsToEmit := make(container.Set[int64]) + approvedRunIDs := make(container.Set[int64]) err := db.WithTx(ctx, func(ctx context.Context) (err error) { for _, runID := range runIDs { @@ -29,7 +32,7 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo if err != nil { return err } - if !run.NeedApproval { + if !run.IsAwaitingApproval() { continue } run.NeedApproval = false @@ -37,10 +40,25 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo if err := actions_model.UpdateRun(ctx, run, "need_approval", "approved_by"); err != nil { return err } + approvedRunIDs.Add(run.ID) jobs, err := actions_model.GetLatestAttemptJobsByRun(ctx, run) if err != nil { return err } + attempt, hasAttempt, err := run.GetLatestAttempt(ctx) + if err != nil { + return fmt.Errorf("get latest attempt of run %d: %w", run.ID, err) + } + if hasAttempt && attempt.ConcurrencyGroup != "" { + status, jobsToCancel, err := PrepareToStartRunWithConcurrency(ctx, attempt) + if err != nil { + return err + } + cancelledConcurrencyJobs = append(cancelledConcurrencyJobs, jobsToCancel...) + if status == actions_model.StatusBlocked { + continue + } + } vars, err := actions_model.GetVariablesOfRun(ctx, run) if err != nil { @@ -54,35 +72,31 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo } for _, job := range jobs { - // Skip jobs with `needs`: they stay blocked until their dependencies finish, - // at which point job_emitter will evaluate and start them. - if len(job.Needs) > 0 { + // Only approval-blocked jobs are released here, job_emitter starts those with `needs` + if job.Status != actions_model.StatusBlocked || len(job.Needs) > 0 { continue } - // Only a job this approval unblocks competes for a slot, one that is already - // active was counted by the seeding loop above and must not take a second. - isUnblocking := job.Status == actions_model.StatusBlocked + if slices.ContainsFunc(cancelledConcurrencyJobs, func(cancelled *actions_model.ActionRunJob) bool { return cancelled.ID == job.ID }) { + continue // cancelled by a sibling's concurrency in this loop + } // a skipped job must neither cancel its group peers nor take a slot - if isUnblocking { - shouldStart, err := evaluateJobIf(ctx, run, nil, job, vars, true) + shouldStart, err := evaluateJobIf(ctx, run, nil, job, vars, true) + if err != nil { + return fmt.Errorf("evaluate job %d if on approval: %w", job.ID, err) + } + if !shouldStart { + job.Status = actions_model.StatusSkipped + n, err := actions_model.UpdateRunJob(ctx, job, nil, "status") if err != nil { - return fmt.Errorf("evaluate job %d if on approval: %w", job.ID, err) + return err } - if !shouldStart { - job.Status = actions_model.StatusSkipped - n, err := actions_model.UpdateRunJob(ctx, job, nil, "status") - if err != nil { - return err - } - if n > 0 { - updatedJobs = append(updatedJobs, job) - runIDsToEmit.Add(run.ID) - } - continue + if n > 0 { + updatedJobs = append(updatedJobs, job) } + continue } // A slot-starved job cannot start, skip the following checks. - if isUnblocking && !slots.available(job) { + if !slots.available(job) { continue } var jobsToCancel []*actions_model.ActionRunJob @@ -91,13 +105,11 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo return err } cancelledConcurrencyJobs = append(cancelledConcurrencyJobs, jobsToCancel...) - if isUnblocking { - applyMaxParallel(job, slots) - } + applyMaxParallel(job, slots) if job.Status != actions_model.StatusWaiting { continue } - n, err := actions_model.UpdateRunJob(ctx, job, nil, "status") + n, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": actions_model.StatusBlocked}, "status") if err != nil { return err } @@ -108,17 +120,12 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo // A top-level reusable caller was just unblocked by approval, expand it if job.IsReusableCaller && !job.IsExpanded { - attempt, has, err := run.GetLatestAttempt(ctx) - if err != nil { - return fmt.Errorf("get latest attempt of run %d: %w", run.ID, err) - } - if !has { + if !hasAttempt { return errors.New("run has no attempt") } if err := expandInlineReusableCaller(ctx, run, attempt, job, vars); err != nil { return err } - runIDsToEmit.Add(run.ID) } } } @@ -128,8 +135,8 @@ func ApproveRuns(ctx context.Context, repo *repo_model.Repository, doer *user_mo return nil, err } - // Re-emit AFTER the tx commits so callee rows and dependents of skipped jobs get resolved. - for runID := range runIDsToEmit { + // The emitter skipped these runs while they awaited approval + for runID := range approvedRunIDs { if err := EmitJobsIfReadyByRun(runID); err != nil { log.Error("emit run %d after approval: %v", runID, err) } diff --git a/services/actions/approve_test.go b/services/actions/approve_test.go index 6f611559a17..eda11b2a779 100644 --- a/services/actions/approve_test.go +++ b/services/actions/approve_test.go @@ -22,6 +22,11 @@ func TestApproveRuns(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + var emittedRunIDs []int64 + defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(runID int64) error { + emittedRunIDs = append(emittedRunIDs, runID) + return nil + })() insertRun := func(index int64, status actions_model.Status, needApproval bool, approvedBy int64) *actions_model.ActionRun { run := &actions_model.ActionRun{ @@ -59,7 +64,7 @@ func TestApproveRuns(t *testing.T) { t.Run("approve skips a job whose if is false", func(t *testing.T) { defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(int64) error { return nil })() - run := insertRun(1006, actions_model.StatusBlocked, true, 0) + run := insertRun(1009, actions_model.StatusBlocked, true, 0) job := insertJob(run, actions_model.StatusBlocked) job.WorkflowPayload = []byte("jobs:\n job1:\n if: false\n") _, err := actions_model.UpdateRunJob(t.Context(), job, nil, "workflow_payload") @@ -83,6 +88,80 @@ func TestApproveRuns(t *testing.T) { assert.Equal(t, actions_model.StatusBlocked, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: job.ID}).Status) }) + t.Run("approval never revives cancelled runs or jobs and re-emits approved runs", func(t *testing.T) { + cancelledRun := insertRun(1006, actions_model.StatusCancelled, true, 0) + cancelledRunJob := insertJob(cancelledRun, actions_model.StatusCancelled) + blockedRun := insertRun(1007, actions_model.StatusBlocked, true, 0) + cancelledJob := insertJob(blockedRun, actions_model.StatusCancelled) + emittedRunIDs = nil + + approved, err := ApproveRuns(t.Context(), repo, doer, []int64{cancelledRun.ID, blockedRun.ID}) + require.NoError(t, err) + require.Len(t, approved, 2) + assert.True(t, approved[0].NeedApproval) + assert.Zero(t, approved[0].ApprovedBy) + assert.False(t, approved[1].NeedApproval) + assert.Equal(t, []int64{blockedRun.ID}, emittedRunIDs) + + assert.Equal(t, actions_model.StatusCancelled, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: cancelledRunJob.ID}).Status) + assert.Equal(t, actions_model.StatusCancelled, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: cancelledJob.ID}).Status) + }) + + t.Run("approval starts one of two blocked jobs sharing a concurrency group", func(t *testing.T) { + run := insertRun(1008, actions_model.StatusBlocked, true, 0) + jobs := []*actions_model.ActionRunJob{insertJob(run, actions_model.StatusBlocked), insertJob(run, actions_model.StatusBlocked)} + for _, job := range jobs { + job.RawConcurrency, job.ConcurrencyGroup, job.IsConcurrencyEvaluated = "group: siblings", "siblings", true + _, err := actions_model.UpdateRunJob(t.Context(), job, nil, "raw_concurrency", "concurrency_group", "is_concurrency_evaluated") + require.NoError(t, err) + } + + _, err := ApproveRuns(t.Context(), repo, doer, []int64{run.ID}) + require.NoError(t, err) + assert.ElementsMatch(t, []actions_model.Status{actions_model.StatusWaiting, actions_model.StatusCancelled}, []actions_model.Status{ + unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: jobs[0].ID}).Status, + unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: jobs[1].ID}).Status, + }) + }) + + t.Run("workflow concurrency cancels peers on approval, not on insertion", func(t *testing.T) { + previousRun := &actions_model.ActionRun{RepoID: 4, OwnerID: 1, Index: 9001, TriggerUserID: 1, TriggerEvent: "push", EventPayload: "{}", Status: actions_model.StatusWaiting} + require.NoError(t, db.Insert(t.Context(), previousRun)) + previousAttempt := &actions_model.ActionRunAttempt{RepoID: 4, RunID: previousRun.ID, Attempt: 1, Status: actions_model.StatusWaiting, ConcurrencyGroup: "shared", ConcurrencyCancel: true} + require.NoError(t, db.Insert(t.Context(), previousAttempt)) + previousJob := &actions_model.ActionRunJob{RunID: previousRun.ID, RunAttemptID: previousAttempt.ID, RepoID: 4, OwnerID: 1, JobID: "deploy", AttemptJobID: 1, Status: actions_model.StatusWaiting} + require.NoError(t, db.Insert(t.Context(), previousJob)) + + content := []byte(`on: pull_request +concurrency: + group: shared + cancel-in-progress: true +jobs: + deploy: + runs-on: ubuntu-latest + concurrency: + group: shared + cancel-in-progress: true + steps: + - run: echo hi +`) + run := &actions_model.ActionRun{ + RepoID: 4, OwnerID: 1, WorkflowID: "test.yaml", TriggerUserID: 2, + Ref: "refs/pull/1/head", CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "pull_request", TriggerEvent: "pull_request", EventPayload: "{}", NeedApproval: true, + WorkflowRepoID: 4, WorkflowCommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + } + require.NoError(t, PrepareRunAndInsert(t.Context(), content, run, nil)) + assert.Equal(t, actions_model.StatusWaiting, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: previousJob.ID}).Status) + + _, err := ApproveRuns(t.Context(), unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}), doer, []int64{run.ID}) + require.NoError(t, err) + assert.Equal(t, actions_model.StatusCancelled, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: previousJob.ID}).Status) + jobs := runJobs(t, run.ID, run.LatestAttemptID) + require.Len(t, jobs, 1) + assert.Equal(t, actions_model.StatusWaiting, jobs[0].Status) + }) + t.Run("re-approving an approved run is a no-op", func(t *testing.T) { run := insertRun(1005, actions_model.StatusRunning, false, 4) job := insertJob(run, actions_model.StatusRunning) diff --git a/services/actions/max_parallel_test.go b/services/actions/max_parallel_test.go index a90b6834aaf..7d054c97dd9 100644 --- a/services/actions/max_parallel_test.go +++ b/services/actions/max_parallel_test.go @@ -11,6 +11,7 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -149,6 +150,7 @@ jobs: // An approval-gated run inserts every job as Blocked, so the cap has to be applied on approval. func TestApproveRuns_MaxParallel(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(int64) error { return nil })() run := insertMaxParallelRun(t, maxParallelWorkflow, true) assert.Equal(t, map[actions_model.Status]int{actions_model.StatusBlocked: 5}, statusCounts(runJobs(t, run.ID, run.LatestAttemptID))) @@ -199,6 +201,7 @@ func Test_jobStatusResolver_MaxParallelStarvedSkipsConcurrency(t *testing.T) { func TestApproveRuns_MaxParallelStarvedSkipsConcurrency(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(int64) error { return nil })() holder := insertConcurrencyHolder(t, 9704, "cluster-b") run := insertMaxParallelRun(t, maxParallelConcurrencyWorkflow, true) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index c67b6fb1026..6b8ac63814e 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -394,11 +394,20 @@ func buildApproveAndInsertRun( IsScopedRun: isScopedRun, } - need, err := ifNeedApproval(ctx, run, input.Repo, input.Doer) + approvalUsers, err := getApprovalUsers(ctx, input, isForkPullRequest) if err != nil { - return fmt.Errorf("check if need approval for user %d: %w", input.Doer.ID, err) + return err + } + for _, user := range approvalUsers { + need, err := ifNeedApproval(ctx, run, input.Repo, user) + if err != nil { + return fmt.Errorf("check if need approval for user %d: %w", user.ID, err) + } + if need { + run.NeedApproval = true + break + } } - run.NeedApproval = need if err := PrepareRunAndInsert(ctx, dwf.Content, run, nil); err != nil { return fmt.Errorf("PrepareRunAndInsert: %w", err) @@ -481,6 +490,23 @@ func notifyPackage(ctx context.Context, sender *user_model.User, pd *packages_mo Notify(ctx) } +// getApprovalUsers returns the event actor, plus the fork PR author when the workflow comes from the PR +func getApprovalUsers(ctx context.Context, input *notifyInput, isForkPullRequest bool) ([]*user_model.User, error) { + if !isForkPullRequest || input.PullRequest == nil || actions_module.IsDefaultBranchWorkflow(input.Event) { + return []*user_model.User{input.Doer}, nil + } + if err := input.PullRequest.LoadIssue(ctx); err != nil { + return nil, fmt.Errorf("load pull request issue: %w", err) + } + if err := input.PullRequest.Issue.LoadPoster(ctx); err != nil { + return nil, fmt.Errorf("load pull request author: %w", err) + } + if input.PullRequest.Issue.PosterID == input.Doer.ID { + return []*user_model.User{input.Doer}, nil + } + return []*user_model.User{input.Doer, input.PullRequest.Issue.Poster}, nil +} + func ifNeedApproval(ctx context.Context, run *actions_model.ActionRun, repo *repo_model.Repository, user *user_model.User) (bool, error) { canWrite := func(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) { perm, err := access_model.GetDoerRepoPermission(ctx, repo, user) diff --git a/services/actions/notifier_helper_test.go b/services/actions/notifier_helper_test.go index 47b21aed5b5..0cb7a213a07 100644 --- a/services/actions/notifier_helper_test.go +++ b/services/actions/notifier_helper_test.go @@ -11,9 +11,11 @@ import ( actions_model "gitea.dev/models/actions" issues_model "gitea.dev/models/issues" repo_model "gitea.dev/models/repo" + "gitea.dev/models/unittest" user_model "gitea.dev/models/user" actions_module "gitea.dev/modules/actions" "gitea.dev/modules/actions/jobparser" + webhook_module "gitea.dev/modules/webhook" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -103,6 +105,22 @@ func TestIfNeedApproval(t *testing.T) { }) } +func TestGetApprovalUsersAddsForkPullRequestAuthorUnlessDefaultBranchWorkflow(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + approvalUsers, err := getApprovalUsers(t.Context(), ¬ifyInput{Doer: doer, PullRequest: pr, Event: webhook_module.HookEventPullRequest}, true) + require.NoError(t, err) + require.Len(t, approvalUsers, 2) + assert.Equal(t, []int64{doer.ID, pr.Issue.PosterID}, []int64{approvalUsers[0].ID, approvalUsers[1].ID}) + + approvalUsers, err = getApprovalUsers(t.Context(), ¬ifyInput{Doer: doer, PullRequest: pr, Event: webhook_module.HookEventIssueComment}, true) + require.NoError(t, err) + assert.Equal(t, []*user_model.User{doer}, approvalUsers) +} + func TestFilteredWorkflowCommitStatusForForkPullRequest(t *testing.T) { forkPR := &issues_model.PullRequest{ Flow: issues_model.PullRequestFlowGithub, diff --git a/services/actions/rerun.go b/services/actions/rerun.go index 0efe7d09355..f516e67cafd 100644 --- a/services/actions/rerun.go +++ b/services/actions/rerun.go @@ -240,7 +240,10 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR } plan.run.LatestAttemptID = newAttempt.ID - if err := actions_model.UpdateRun(ctx, plan.run, "latest_attempt_id"); err != nil { + if plan.run.NeedApproval { // rerunning is an explicit approval + plan.run.NeedApproval, plan.run.ApprovedBy = false, plan.triggerUser.ID + } + if err := actions_model.UpdateRun(ctx, plan.run, "latest_attempt_id", "need_approval", "approved_by"); err != nil { return err } diff --git a/services/actions/run.go b/services/actions/run.go index 4fc70788880..a589ccdf9fa 100644 --- a/services/actions/run.go +++ b/services/actions/run.go @@ -100,12 +100,14 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte return fmt.Errorf("EvaluateRunConcurrencyFillModel: %w", err) } // check run (workflow-level) concurrency - var jobsToCancel []*actions_model.ActionRunJob - runAttempt.Status, jobsToCancel, err = PrepareToStartRunWithConcurrency(ctx, runAttempt) - if err != nil { - return err + if !run.NeedApproval { // deferred to ApproveRuns + var jobsToCancel []*actions_model.ActionRunJob + runAttempt.Status, jobsToCancel, err = PrepareToStartRunWithConcurrency(ctx, runAttempt) + if err != nil { + return err + } + cancelledConcurrencyJobs = append(cancelledConcurrencyJobs, jobsToCancel...) } - cancelledConcurrencyJobs = append(cancelledConcurrencyJobs, jobsToCancel...) } if err := db.Insert(ctx, runAttempt); err != nil { diff --git a/tests/integration/actions_approve_test.go b/tests/integration/actions_approve_test.go index 6d4e4d7f8ac..32c7104eac3 100644 --- a/tests/integration/actions_approve_test.go +++ b/tests/integration/actions_approve_test.go @@ -124,19 +124,34 @@ jobs: dataURL, ) + req = NewRequest(t, "POST", fmt.Sprintf("%s/actions/runs/%d/cancel", baseRepo.Link(), run1.ID)) + user2Session.MakeRequest(t, req, http.StatusOK) + // user2 approves all runs req = NewRequest(t, "POST", dataURL) user2Session.MakeRequest(t, req, http.StatusOK) // check runs run1 = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run1.ID}) - assert.False(t, run1.NeedApproval) - assert.Equal(t, user2.ID, run1.ApprovedBy) - assert.Equal(t, actions_model.StatusWaiting, run1.Status) + assert.True(t, run1.NeedApproval) + assert.Equal(t, actions_model.StatusCancelled, run1.Status) run2 = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run2.ID}) assert.False(t, run2.NeedApproval) assert.Equal(t, user2.ID, run2.ApprovedBy) assert.Equal(t, actions_model.StatusWaiting, run2.Status) + + req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", baseRepo.OwnerName, baseRepo.Name, apiPull.Index)) + resp = user2Session.MakeRequest(t, req, http.StatusOK) + assert.Zero(t, NewHTMLParser(t, resp.Body).doc.Find("#approve-status-checks").Length()) + + req = NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/actions/runs/%d/approve", baseRepo.OwnerName, baseRepo.Name, run1.ID)).AddTokenAuth(user2Token) + MakeRequest(t, req, http.StatusConflict) + + req = NewRequest(t, "POST", fmt.Sprintf("%s/actions/runs/%d/rerun", baseRepo.Link(), run1.ID)) + user2Session.MakeRequest(t, req, http.StatusOK) + run1 = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run1.ID}) + assert.False(t, run1.NeedApproval) + assert.Equal(t, user2.ID, run1.ApprovedBy) }) }