Skip to content

Commit dd83ccf

Browse files
silverwindclaude
andcommitted
Replace integration test with unit test for resolver guard
Per @wxiaoguang's feedback on #37461: the run-level concurrency guard in checkJobsOfCurrentRunAttempt is function-level logic and is better covered by a unit test. The unit test sets up a Running holder attempt and a Blocked sibling attempt in the same concurrency group directly in the DB, calls checkJobsOfCurrentRunAttempt, and asserts the blocked job stays Blocked. ~0.3s vs ~3.7s for the integration version, and no API/git-hook overhead. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
1 parent 2ee2d35 commit dd83ccf

2 files changed

Lines changed: 65 additions & 65 deletions

File tree

services/actions/job_emitter_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,68 @@ func Test_checkRunConcurrency_NoDuplicateConcurrencyGroupCheck(t *testing.T) {
228228
assert.Equal(t, jobBBlocked.ID, jobs[0].ID)
229229
}
230230
}
231+
232+
// Test_checkJobsOfCurrentRunAttempt_RunLevelConcurrencyKeepsJobsBlocked verifies that
233+
// the resolver does not transition a job out of Blocked while another run still holds
234+
// the workflow-level concurrency group. Regression for #37446.
235+
func Test_checkJobsOfCurrentRunAttempt_RunLevelConcurrencyKeepsJobsBlocked(t *testing.T) {
236+
assert.NoError(t, unittest.PrepareTestDatabase())
237+
ctx := t.Context()
238+
239+
const group = "test-run-level-concurrency-keeps-blocked"
240+
241+
// Holder run: Running attempt in the concurrency group.
242+
holderRun := &actions_model.ActionRun{
243+
RepoID: 4, OwnerID: 1, TriggerUserID: 1,
244+
WorkflowID: "test.yml", Index: 9911, Ref: "refs/heads/main",
245+
Status: actions_model.StatusRunning,
246+
}
247+
assert.NoError(t, db.Insert(ctx, holderRun))
248+
holderAttempt := &actions_model.ActionRunAttempt{
249+
RepoID: 4, RunID: holderRun.ID, Attempt: 1,
250+
Status: actions_model.StatusRunning, ConcurrencyGroup: group,
251+
}
252+
assert.NoError(t, db.Insert(ctx, holderAttempt))
253+
_, err := db.Exec(ctx, "UPDATE `action_run` SET latest_attempt_id = ? WHERE id = ?", holderAttempt.ID, holderRun.ID)
254+
assert.NoError(t, err)
255+
256+
// Blocked run: Blocked attempt in the same group, with one Blocked job that has
257+
// no needs and no job-level concurrency. Without the run-level guard in
258+
// checkJobsOfCurrentRunAttempt, the resolver would transition this job to Waiting.
259+
blockedRun := &actions_model.ActionRun{
260+
RepoID: 4, OwnerID: 1, TriggerUserID: 1,
261+
WorkflowID: "test.yml", Index: 9912, Ref: "refs/heads/main",
262+
Status: actions_model.StatusBlocked,
263+
}
264+
assert.NoError(t, db.Insert(ctx, blockedRun))
265+
blockedAttempt := &actions_model.ActionRunAttempt{
266+
RepoID: 4, RunID: blockedRun.ID, Attempt: 1,
267+
Status: actions_model.StatusBlocked, ConcurrencyGroup: group,
268+
}
269+
assert.NoError(t, db.Insert(ctx, blockedAttempt))
270+
_, err = db.Exec(ctx, "UPDATE `action_run` SET latest_attempt_id = ? WHERE id = ?", blockedAttempt.ID, blockedRun.ID)
271+
assert.NoError(t, err)
272+
blockedRun.LatestAttemptID = blockedAttempt.ID
273+
blockedJob := &actions_model.ActionRunJob{
274+
RunID: blockedRun.ID, RunAttemptID: blockedAttempt.ID, AttemptJobID: 1,
275+
RepoID: 4, OwnerID: 1, JobID: "job1", Name: "job1",
276+
Status: actions_model.StatusBlocked,
277+
WorkflowPayload: []byte(`
278+
name: test
279+
on: push
280+
jobs:
281+
job1:
282+
runs-on: ubuntu-latest
283+
steps:
284+
- run: echo
285+
`),
286+
}
287+
assert.NoError(t, db.Insert(ctx, blockedJob))
288+
289+
_, updated, _, err := checkJobsOfCurrentRunAttempt(ctx, blockedRun)
290+
assert.NoError(t, err)
291+
assert.Empty(t, updated)
292+
293+
refreshed := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: blockedJob.ID})
294+
assert.Equal(t, actions_model.StatusBlocked, refreshed.Status)
295+
}

tests/integration/actions_concurrency_test.go

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
repo_model "code.gitea.io/gitea/models/repo"
1818
"code.gitea.io/gitea/models/unittest"
1919
user_model "code.gitea.io/gitea/models/user"
20-
"code.gitea.io/gitea/modules/queue"
2120
"code.gitea.io/gitea/modules/setting"
2221
api "code.gitea.io/gitea/modules/structs"
2322
"code.gitea.io/gitea/modules/timeutil"
@@ -1220,70 +1219,6 @@ jobs:
12201219
})
12211220
}
12221221

1223-
// TestScheduleConcurrencyBlockedRunStaysBlocked verifies that a run blocked solely by
1224-
// run-level concurrency is not unblocked by the job-status resolver while another run
1225-
// still holds the same concurrency group.
1226-
func TestScheduleConcurrencyBlockedRunStaysBlocked(t *testing.T) {
1227-
onGiteaRun(t, func(t *testing.T, u *url.URL) {
1228-
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
1229-
session := loginUser(t, user2.Name)
1230-
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
1231-
1232-
apiRepo := createActionsTestRepo(t, token, "actions-concurrency", false)
1233-
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID})
1234-
httpContext := NewAPITestContext(t, user2.Name, repo.Name, auth_model.AccessTokenScopeWriteRepository)
1235-
defer doAPIDeleteRepository(httpContext)(t)
1236-
1237-
runner := newMockRunner()
1238-
runner.registerAsRepoRunner(t, user2.Name, repo.Name, "mock-runner", []string{"ubuntu-latest"}, false)
1239-
1240-
wfTreePath := ".gitea/workflows/schedule-concurrency.yml"
1241-
wfFileContent := `name: schedule-concurrency
1242-
on:
1243-
push:
1244-
schedule:
1245-
- cron: '@every 1m'
1246-
concurrency:
1247-
group: schedule-concurrency
1248-
cancel-in-progress: ${{ gitea.event_name == 'push' }}
1249-
jobs:
1250-
job:
1251-
runs-on: ubuntu-latest
1252-
steps:
1253-
- run: echo 'schedule workflow'
1254-
`
1255-
opts := getWorkflowCreateFileOptions(user2, repo.DefaultBranch, "create "+wfTreePath, wfFileContent)
1256-
createWorkflowFile(t, token, user2.Name, repo.Name, wfTreePath, opts)
1257-
1258-
// Push run picks up and holds the "schedule-concurrency" group while it runs.
1259-
_ = runner.fetchTask(t)
1260-
1261-
fireSchedule := func() {
1262-
t.Helper()
1263-
spec := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionScheduleSpec{RepoID: repo.ID})
1264-
spec.Next = timeutil.TimeStampNow()
1265-
require.NoError(t, actions_model.UpdateScheduleSpec(t.Context(), spec, "next"))
1266-
require.NoError(t, actions_service.StartScheduleTasks(t.Context()))
1267-
}
1268-
1269-
// Two schedule triggers: the first creates a Blocked run; the second creates another
1270-
// Blocked run and cancels the first (cancel-in-progress is false for schedule, so
1271-
// only Waiting/Blocked are cancelled). The cancellation queues EmitJobsIfReadyByRun
1272-
// and drives the resolver against the surviving Blocked run.
1273-
fireSchedule()
1274-
fireSchedule()
1275-
require.NoError(t, queue.GetManager().FlushAll(t.Context(), 15*time.Second))
1276-
1277-
blockedRuns, err := db.Find[actions_model.ActionRun](t.Context(), actions_model.FindRunOptions{
1278-
RepoID: repo.ID,
1279-
TriggerEvent: webhook_module.HookEventSchedule,
1280-
Status: []actions_model.Status{actions_model.StatusBlocked},
1281-
})
1282-
require.NoError(t, err)
1283-
require.Len(t, blockedRuns, 1)
1284-
})
1285-
}
1286-
12871222
func TestWorkflowAndJobConcurrency(t *testing.T) {
12881223
onGiteaRun(t, func(t *testing.T, u *url.URL) {
12891224
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})

0 commit comments

Comments
 (0)