test(mr): cover failing checks with failing_checks_block off, the pending auto-merge index, and stop relying on event order
- commit
- be277d4b1716e727e95a9ed710787775794916ab
- author
- brice · · 2026-10-20 17:15 UTC
- parent
- 84db480
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +141 -28
M internal/app/automerge_e2e_test.go +11 -5
| @@ -45,11 +45,17 @@ jobs: | |||
| 45 | 45 | require.NoError(t, err) | |
| 46 | 46 | require.NotNil(t, enabled.AutoMerge) | |
| 47 | 47 | assert.Equal(t, head, enabled.AutoMerge.HeadID.String()) | |
| 48 | - | select { | |
| 49 | - | case ev := <-events: | |
| 50 | - | require.IsType(t, domain.MergeRequestAutoMergeEnabled{}, ev) | |
| 51 | - | case <-time.After(5 * time.Second): | |
| 52 | - | t.Fatal("no enabled event") | |
| 48 | + | enabledDeadline := time.After(5 * time.Second) | |
| 49 | + | waitEnabled: | |
| 50 | + | for { | |
| 51 | + | select { | |
| 52 | + | case ev := <-events: | |
| 53 | + | if enabled, ok := ev.(domain.MergeRequestAutoMergeEnabled); ok && enabled.MergeRequest.Repo == repo && enabled.MergeRequest.Number == number { | |
| 54 | + | break waitEnabled | |
| 55 | + | } | |
| 56 | + | case <-enabledDeadline: | |
| 57 | + | t.Fatal("no enabled event") | |
| 58 | + | } | |
| 53 | 59 | } | |
| 54 | 60 | c.finish(runner) | |
| 55 | 61 | // A successful push check alone must not merge while the pull run is queued. | |
| @@ -45,11 +45,17 @@ jobs: | |||||
| 45 | require.NoError(t, err) | 45 | require.NoError(t, err) | ||
| 46 | require.NotNil(t, enabled.AutoMerge) | 46 | require.NotNil(t, enabled.AutoMerge) | ||
| 47 | assert.Equal(t, head, enabled.AutoMerge.HeadID.String()) | 47 | assert.Equal(t, head, enabled.AutoMerge.HeadID.String()) | ||
| 48 | - | select { | 48 | + | enabledDeadline := time.After(5 * time.Second) |
| 49 | - | case ev := <-events: | 49 | + | waitEnabled: |
| 50 | - | require.IsType(t, domain.MergeRequestAutoMergeEnabled{}, ev) | 50 | + | for { |
| 51 | - | case <-time.After(5 * time.Second): | 51 | + | select { |
| 52 | - | t.Fatal("no enabled event") | 52 | + | case ev := <-events: |
| 53 | + | if enabled, ok := ev.(domain.MergeRequestAutoMergeEnabled); ok && enabled.MergeRequest.Repo == repo && enabled.MergeRequest.Number == number { | |||
| 54 | + | break waitEnabled | |||
| 55 | + | } | |||
| 56 | + | case <-enabledDeadline: | |||
| 57 | + | t.Fatal("no enabled event") | |||
| 58 | + | } | |||
| 53 | } | 59 | } | ||
| 54 | c.finish(runner) | 60 | c.finish(runner) | ||
| 55 | // A successful push check alone must not merge while the pull run is queued. | 61 | // A successful push check alone must not merge while the pull run is queued. | ||
A internal/usecase/automerge_index_test.go +97 -0
| @@ -0,0 +1,97 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package usecase | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "testing" | |
| 8 | + | "time" | |
| 9 | + | ||
| 10 | + | "github.com/stretchr/testify/mock" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ||
| 13 | + | "bnema.dev/ermite/internal/domain" | |
| 14 | + | "bnema.dev/ermite/internal/ports/mocks" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | func TestAutoMergePendingIndex(t *testing.T) { | |
| 18 | + | ctx := context.Background() | |
| 19 | + | repo, err := domain.ParseRepoName("proj") | |
| 20 | + | require.NoError(t, err) | |
| 21 | + | store := mocks.NewMergeRequestStore(t) | |
| 22 | + | meta := mocks.NewRepoMetaStore(t) | |
| 23 | + | a := NewAutoMerge(ctx, AutoMergeConfig{Store: store, Meta: meta}) | |
| 24 | + | defer a.Close(ctx) | |
| 25 | + | // No expectations: an unindexed kick must not read either store. | |
| 26 | + | a.OnRepoChanged(ctx, repo) | |
| 27 | + | _, found := a.next() | |
| 28 | + | require.False(t, found) | |
| 29 | + | a.OnAutoMergeEnabled(ctx, repo) | |
| 30 | + | a.OnRepoChanged(ctx, repo) | |
| 31 | + | got, found := a.next() | |
| 32 | + | require.True(t, found) | |
| 33 | + | require.Equal(t, repo, got) | |
| 34 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil).Once() | |
| 35 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return(nil, nil).Once() | |
| 36 | + | require.True(t, a.evaluate(repo)) | |
| 37 | + | require.NotContains(t, a.pending, repo) | |
| 38 | + | a.mu.Lock() | |
| 39 | + | delete(a.running, repo) | |
| 40 | + | a.mu.Unlock() | |
| 41 | + | a.OnRepoChanged(ctx, repo) | |
| 42 | + | _, found = a.next() | |
| 43 | + | require.False(t, found) | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func TestAutoMergeEnableDuringEmptyScanKeepsPending(t *testing.T) { | |
| 47 | + | ctx := context.Background() | |
| 48 | + | repo, err := domain.ParseRepoName("proj") | |
| 49 | + | require.NoError(t, err) | |
| 50 | + | store := mocks.NewMergeRequestStore(t) | |
| 51 | + | meta := mocks.NewRepoMetaStore(t) | |
| 52 | + | a := NewAutoMerge(ctx, AutoMergeConfig{Store: store, Meta: meta}) | |
| 53 | + | defer a.Close(ctx) | |
| 54 | + | a.OnAutoMergeEnabled(ctx, repo) | |
| 55 | + | _, found := a.next() | |
| 56 | + | require.True(t, found) | |
| 57 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil).Once() | |
| 58 | + | store.EXPECT().ListOpen(mock.Anything, repo).Run(func(context.Context, domain.RepoName) { | |
| 59 | + | // The returned empty snapshot predates this newly enabled intent. | |
| 60 | + | a.OnAutoMergeEnabled(ctx, repo) | |
| 61 | + | }).Return(nil, nil).Once() | |
| 62 | + | require.True(t, a.evaluate(repo)) | |
| 63 | + | require.True(t, a.pending[repo]) | |
| 64 | + | require.True(t, a.again[repo]) | |
| 65 | + | } | |
| 66 | + | ||
| 67 | + | func TestAutoMergeStartupFillsPendingIndex(t *testing.T) { | |
| 68 | + | ctx := context.Background() | |
| 69 | + | repo, err := domain.ParseRepoName("proj") | |
| 70 | + | require.NoError(t, err) | |
| 71 | + | store := mocks.NewMergeRequestStore(t) | |
| 72 | + | meta := mocks.NewRepoMetaStore(t) | |
| 73 | + | store.EXPECT().ListOpenAll(mock.Anything).Return([]domain.MergeRequest{{Repo: repo, AutoMerge: &domain.AutoMerge{}}}, nil).Once() | |
| 74 | + | scanned := make(chan struct{}) | |
| 75 | + | meta.EXPECT().Get(mock.Anything, repo).Run(func(context.Context, domain.RepoName) { close(scanned) }).Return(domain.Repo{}, domain.ErrNotFound).Once() | |
| 76 | + | a := NewAutoMerge(ctx, AutoMergeConfig{Store: store, Meta: meta}) | |
| 77 | + | a.Start() | |
| 78 | + | defer a.Close(ctx) | |
| 79 | + | select { | |
| 80 | + | case <-scanned: | |
| 81 | + | case <-time.After(5 * time.Second): | |
| 82 | + | t.Fatal("startup did not scan the pending repository") | |
| 83 | + | } | |
| 84 | + | a.mu.Lock() | |
| 85 | + | pending := a.pending[repo] | |
| 86 | + | a.mu.Unlock() | |
| 87 | + | require.True(t, pending) | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | func TestManualPendingChecksMessage(t *testing.T) { | |
| 91 | + | err := checksPolicy([]domain.CICheck{{Name: "build", Status: domain.CIQueued}}) | |
| 92 | + | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |
| 93 | + | require.EqualError(t, err, "failed precondition: checks have not finished: build") | |
| 94 | + | require.NotContains(t, err.Error(), "auto-merge") | |
| 95 | + | require.NotErrorIs(t, err, errAutoMergeWait) | |
| 96 | + | require.NoError(t, checksPolicy(nil)) | |
| 97 | + | } | |
| @@ -0,0 +1,97 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package usecase | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "testing" | |||
| 8 | + | "time" | |||
| 9 | + | ||||
| 10 | + | "github.com/stretchr/testify/mock" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ||||
| 13 | + | "bnema.dev/ermite/internal/domain" | |||
| 14 | + | "bnema.dev/ermite/internal/ports/mocks" | |||
| 15 | + | ) | |||
| 16 | + | ||||
| 17 | + | func TestAutoMergePendingIndex(t *testing.T) { | |||
| 18 | + | ctx := context.Background() | |||
| 19 | + | repo, err := domain.ParseRepoName("proj") | |||
| 20 | + | require.NoError(t, err) | |||
| 21 | + | store := mocks.NewMergeRequestStore(t) | |||
| 22 | + | meta := mocks.NewRepoMetaStore(t) | |||
| 23 | + | a := NewAutoMerge(ctx, AutoMergeConfig{Store: store, Meta: meta}) | |||
| 24 | + | defer a.Close(ctx) | |||
| 25 | + | // No expectations: an unindexed kick must not read either store. | |||
| 26 | + | a.OnRepoChanged(ctx, repo) | |||
| 27 | + | _, found := a.next() | |||
| 28 | + | require.False(t, found) | |||
| 29 | + | a.OnAutoMergeEnabled(ctx, repo) | |||
| 30 | + | a.OnRepoChanged(ctx, repo) | |||
| 31 | + | got, found := a.next() | |||
| 32 | + | require.True(t, found) | |||
| 33 | + | require.Equal(t, repo, got) | |||
| 34 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil).Once() | |||
| 35 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return(nil, nil).Once() | |||
| 36 | + | require.True(t, a.evaluate(repo)) | |||
| 37 | + | require.NotContains(t, a.pending, repo) | |||
| 38 | + | a.mu.Lock() | |||
| 39 | + | delete(a.running, repo) | |||
| 40 | + | a.mu.Unlock() | |||
| 41 | + | a.OnRepoChanged(ctx, repo) | |||
| 42 | + | _, found = a.next() | |||
| 43 | + | require.False(t, found) | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func TestAutoMergeEnableDuringEmptyScanKeepsPending(t *testing.T) { | |||
| 47 | + | ctx := context.Background() | |||
| 48 | + | repo, err := domain.ParseRepoName("proj") | |||
| 49 | + | require.NoError(t, err) | |||
| 50 | + | store := mocks.NewMergeRequestStore(t) | |||
| 51 | + | meta := mocks.NewRepoMetaStore(t) | |||
| 52 | + | a := NewAutoMerge(ctx, AutoMergeConfig{Store: store, Meta: meta}) | |||
| 53 | + | defer a.Close(ctx) | |||
| 54 | + | a.OnAutoMergeEnabled(ctx, repo) | |||
| 55 | + | _, found := a.next() | |||
| 56 | + | require.True(t, found) | |||
| 57 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo}, nil).Once() | |||
| 58 | + | store.EXPECT().ListOpen(mock.Anything, repo).Run(func(context.Context, domain.RepoName) { | |||
| 59 | + | // The returned empty snapshot predates this newly enabled intent. | |||
| 60 | + | a.OnAutoMergeEnabled(ctx, repo) | |||
| 61 | + | }).Return(nil, nil).Once() | |||
| 62 | + | require.True(t, a.evaluate(repo)) | |||
| 63 | + | require.True(t, a.pending[repo]) | |||
| 64 | + | require.True(t, a.again[repo]) | |||
| 65 | + | } | |||
| 66 | + | ||||
| 67 | + | func TestAutoMergeStartupFillsPendingIndex(t *testing.T) { | |||
| 68 | + | ctx := context.Background() | |||
| 69 | + | repo, err := domain.ParseRepoName("proj") | |||
| 70 | + | require.NoError(t, err) | |||
| 71 | + | store := mocks.NewMergeRequestStore(t) | |||
| 72 | + | meta := mocks.NewRepoMetaStore(t) | |||
| 73 | + | store.EXPECT().ListOpenAll(mock.Anything).Return([]domain.MergeRequest{{Repo: repo, AutoMerge: &domain.AutoMerge{}}}, nil).Once() | |||
| 74 | + | scanned := make(chan struct{}) | |||
| 75 | + | meta.EXPECT().Get(mock.Anything, repo).Run(func(context.Context, domain.RepoName) { close(scanned) }).Return(domain.Repo{}, domain.ErrNotFound).Once() | |||
| 76 | + | a := NewAutoMerge(ctx, AutoMergeConfig{Store: store, Meta: meta}) | |||
| 77 | + | a.Start() | |||
| 78 | + | defer a.Close(ctx) | |||
| 79 | + | select { | |||
| 80 | + | case <-scanned: | |||
| 81 | + | case <-time.After(5 * time.Second): | |||
| 82 | + | t.Fatal("startup did not scan the pending repository") | |||
| 83 | + | } | |||
| 84 | + | a.mu.Lock() | |||
| 85 | + | pending := a.pending[repo] | |||
| 86 | + | a.mu.Unlock() | |||
| 87 | + | require.True(t, pending) | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | func TestManualPendingChecksMessage(t *testing.T) { | |||
| 91 | + | err := checksPolicy([]domain.CICheck{{Name: "build", Status: domain.CIQueued}}) | |||
| 92 | + | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |||
| 93 | + | require.EqualError(t, err, "failed precondition: checks have not finished: build") | |||
| 94 | + | require.NotContains(t, err.Error(), "auto-merge") | |||
| 95 | + | require.NotErrorIs(t, err, errAutoMergeWait) | |||
| 96 | + | require.NoError(t, checksPolicy(nil)) | |||
| 97 | + | } | |||
M internal/usecase/automerge_test.go +28 -18
| @@ -32,7 +32,7 @@ func autoRig(t *testing.T) (*mrRig, domain.MergeRequest, *mocks.CIChecksReader) | |||
| 32 | 32 | } | |
| 33 | 33 | ||
| 34 | 34 | func autoEvaluator(r *mrRig, checks ports.CIChecksReader, activity ports.CIActivity, tokens ports.TokenStore) *usecase.AutoMerge { | |
| 35 | - | return usecase.NewAutoMerge(r.ctx, usecase.AutoMergeConfig{MRs: r.mrs, Store: r.store.MergeRequests(), Meta: r.store, Git: r.git, Checks: checks, Activity: activity, Tokens: tokens, Bus: r.bus, Clock: fixedClock{}}) | |
| 35 | + | return usecase.NewAutoMerge(r.ctx, usecase.AutoMergeConfig{MRs: r.mrs, Store: r.store.MergeRequests(), Meta: r.store, Git: r.git, Checks: checks, Activity: activity, Tokens: tokens, Clock: fixedClock{}}) | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | 38 | func enableAuto(t *testing.T, r *mrRig, mr domain.MergeRequest) domain.MergeRequest { | |
| @@ -87,7 +87,7 @@ func TestEnableAutoMergeNeedsSettingScopeAndRef(t *testing.T) { | |||
| 87 | 87 | func TestEnableAutoMergeRecordsEnablerModeHeadAndPublishes(t *testing.T) { | |
| 88 | 88 | r, mr, _ := autoRig(t) | |
| 89 | 89 | trigger := mocks.NewAutoMergeTrigger(t) | |
| 90 | - | trigger.EXPECT().OnRepoChanged(mock.Anything, r.repo).Return().Twice() | |
| 90 | + | trigger.EXPECT().OnAutoMergeEnabled(mock.Anything, r.repo).Return().Twice() | |
| 91 | 91 | r.mrs.WithAutoMerge(trigger) | |
| 92 | 92 | r.drain() | |
| 93 | 93 | got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: r.head()}) | |
| @@ -121,8 +121,8 @@ func TestAutoMergeWaitsWhileCINotSettledOrChecksPending(t *testing.T) { | |||
| 121 | 121 | r, mr, checks := autoRig(t) | |
| 122 | 122 | enableAuto(t, r, mr) | |
| 123 | 123 | activity := mocks.NewCIActivity(t) | |
| 124 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(false).Once() | |
| 125 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(true).Twice() | |
| 124 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(false).Once() | |
| 125 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(true).Twice() | |
| 126 | 126 | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}, {Status: domain.CIQueued, Name: "pending"}}, nil) | |
| 127 | 127 | a := autoEvaluator(r, checks, activity, nil) | |
| 128 | 128 | defer a.Close(r.ctx) | |
| @@ -138,7 +138,8 @@ func TestAutoMergeMergesAsEnablerWhenAChecksPassed(t *testing.T) { | |||
| 138 | 138 | trigger.EXPECT().OnMerge(mock.Anything, r.repo, mock.Anything, root).Return().Once() | |
| 139 | 139 | r.mrs.SetWorkflowTrigger(trigger) | |
| 140 | 140 | enableAuto(t, r, mr) | |
| 141 | - | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil) | |
| 141 | + | // One pre-lock fast-path read and exactly one under-lock snapshot. | |
| 142 | + | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil).Twice() | |
| 142 | 143 | a := autoEvaluator(r, checks, nil, nil) | |
| 143 | 144 | defer a.Close(r.ctx) | |
| 144 | 145 | r.drain() | |
| @@ -159,17 +160,26 @@ func TestAutoMergeMergesAsEnablerWhenAChecksPassed(t *testing.T) { | |||
| 159 | 160 | } | |
| 160 | 161 | ||
| 161 | 162 | func TestAutoMergeIgnoresFailingChecksBlockOff(t *testing.T) { | |
| 162 | - | r, mr, checks := autoRig(t) | |
| 163 | - | enableAuto(t, r, mr) | |
| 164 | - | _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{FailingChecksBlock: ptr(false)}) | |
| 165 | - | require.NoError(t, err) | |
| 166 | - | // No successful check: an unsuccessful head waits rather than merging. | |
| 167 | - | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CIFailure}}, nil) | |
| 168 | - | a := autoEvaluator(r, checks, nil, nil) | |
| 169 | - | defer a.Close(r.ctx) | |
| 170 | - | require.True(t, a.Evaluate(r.repo)) | |
| 171 | - | assert.True(t, storedAuto(t, r, mr).IsOpen()) | |
| 172 | - | assert.NotNil(t, storedAuto(t, r, mr).AutoMerge) | |
| 163 | + | for _, tc := range []struct { | |
| 164 | + | name string | |
| 165 | + | check domain.CICheck | |
| 166 | + | }{ | |
| 167 | + | {"failure", domain.CICheck{Status: domain.CICompleted, Conclusion: domain.CIFailure}}, | |
| 168 | + | {"pending", domain.CICheck{Status: domain.CIQueued}}, | |
| 169 | + | } { | |
| 170 | + | t.Run(tc.name, func(t *testing.T) { | |
| 171 | + | r, mr, checks := autoRig(t) | |
| 172 | + | enableAuto(t, r, mr) | |
| 173 | + | _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{FailingChecksBlock: ptr(false)}) | |
| 174 | + | require.NoError(t, err) | |
| 175 | + | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}, tc.check}, nil) | |
| 176 | + | a := autoEvaluator(r, checks, nil, nil) | |
| 177 | + | defer a.Close(r.ctx) | |
| 178 | + | require.True(t, a.Evaluate(r.repo)) | |
| 179 | + | assert.True(t, storedAuto(t, r, mr).IsOpen()) | |
| 180 | + | assert.NotNil(t, storedAuto(t, r, mr).AutoMerge) | |
| 181 | + | }) | |
| 182 | + | } | |
| 173 | 183 | } | |
| 174 | 184 | ||
| 175 | 185 | func TestAutoMergeCancelsOnNewHead(t *testing.T) { testAutoCancellation(t, "head") } | |
| @@ -432,8 +442,8 @@ func TestAutoMergeRechecksSettledUnderMergeGate(t *testing.T) { | |||
| 432 | 442 | r, mr, checks := autoRig(t) | |
| 433 | 443 | enableAuto(t, r, mr) | |
| 434 | 444 | activity := mocks.NewCIActivity(t) | |
| 435 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(true).Once() | |
| 436 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(false).Once() | |
| 445 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(true).Once() | |
| 446 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(false).Once() | |
| 437 | 447 | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil).Once() | |
| 438 | 448 | a := autoEvaluator(r, checks, activity, nil) | |
| 439 | 449 | defer a.Close(r.ctx) | |
| @@ -32,7 +32,7 @@ func autoRig(t *testing.T) (*mrRig, domain.MergeRequest, *mocks.CIChecksReader) | |||||
| 32 | } | 32 | } | ||
| 33 | 33 | ||||
| 34 | func autoEvaluator(r *mrRig, checks ports.CIChecksReader, activity ports.CIActivity, tokens ports.TokenStore) *usecase.AutoMerge { | 34 | func autoEvaluator(r *mrRig, checks ports.CIChecksReader, activity ports.CIActivity, tokens ports.TokenStore) *usecase.AutoMerge { | ||
| 35 | - | return usecase.NewAutoMerge(r.ctx, usecase.AutoMergeConfig{MRs: r.mrs, Store: r.store.MergeRequests(), Meta: r.store, Git: r.git, Checks: checks, Activity: activity, Tokens: tokens, Bus: r.bus, Clock: fixedClock{}}) | 35 | + | return usecase.NewAutoMerge(r.ctx, usecase.AutoMergeConfig{MRs: r.mrs, Store: r.store.MergeRequests(), Meta: r.store, Git: r.git, Checks: checks, Activity: activity, Tokens: tokens, Clock: fixedClock{}}) |
| 36 | } | 36 | } | ||
| 37 | 37 | ||||
| 38 | func enableAuto(t *testing.T, r *mrRig, mr domain.MergeRequest) domain.MergeRequest { | 38 | func enableAuto(t *testing.T, r *mrRig, mr domain.MergeRequest) domain.MergeRequest { | ||
| @@ -87,7 +87,7 @@ func TestEnableAutoMergeNeedsSettingScopeAndRef(t *testing.T) { | |||||
| 87 | func TestEnableAutoMergeRecordsEnablerModeHeadAndPublishes(t *testing.T) { | 87 | func TestEnableAutoMergeRecordsEnablerModeHeadAndPublishes(t *testing.T) { | ||
| 88 | r, mr, _ := autoRig(t) | 88 | r, mr, _ := autoRig(t) | ||
| 89 | trigger := mocks.NewAutoMergeTrigger(t) | 89 | trigger := mocks.NewAutoMergeTrigger(t) | ||
| 90 | - | trigger.EXPECT().OnRepoChanged(mock.Anything, r.repo).Return().Twice() | 90 | + | trigger.EXPECT().OnAutoMergeEnabled(mock.Anything, r.repo).Return().Twice() |
| 91 | r.mrs.WithAutoMerge(trigger) | 91 | r.mrs.WithAutoMerge(trigger) | ||
| 92 | r.drain() | 92 | r.drain() | ||
| 93 | got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: r.head()}) | 93 | got, err := r.mrs.EnableAutoMerge(r.ctx, domain.Owner(), r.repo, mr.Number, ports.EnableAutoMergeParams{Mode: domain.MergeModeSquash, HeadID: r.head()}) | ||
| @@ -121,8 +121,8 @@ func TestAutoMergeWaitsWhileCINotSettledOrChecksPending(t *testing.T) { | |||||
| 121 | r, mr, checks := autoRig(t) | 121 | r, mr, checks := autoRig(t) | ||
| 122 | enableAuto(t, r, mr) | 122 | enableAuto(t, r, mr) | ||
| 123 | activity := mocks.NewCIActivity(t) | 123 | activity := mocks.NewCIActivity(t) | ||
| 124 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(false).Once() | 124 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(false).Once() |
| 125 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(true).Twice() | 125 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(true).Twice() |
| 126 | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}, {Status: domain.CIQueued, Name: "pending"}}, nil) | 126 | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}, {Status: domain.CIQueued, Name: "pending"}}, nil) | ||
| 127 | a := autoEvaluator(r, checks, activity, nil) | 127 | a := autoEvaluator(r, checks, activity, nil) | ||
| 128 | defer a.Close(r.ctx) | 128 | defer a.Close(r.ctx) | ||
| @@ -138,7 +138,8 @@ func TestAutoMergeMergesAsEnablerWhenAChecksPassed(t *testing.T) { | |||||
| 138 | trigger.EXPECT().OnMerge(mock.Anything, r.repo, mock.Anything, root).Return().Once() | 138 | trigger.EXPECT().OnMerge(mock.Anything, r.repo, mock.Anything, root).Return().Once() | ||
| 139 | r.mrs.SetWorkflowTrigger(trigger) | 139 | r.mrs.SetWorkflowTrigger(trigger) | ||
| 140 | enableAuto(t, r, mr) | 140 | enableAuto(t, r, mr) | ||
| 141 | - | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil) | 141 | + | // One pre-lock fast-path read and exactly one under-lock snapshot. |
| 142 | + | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil).Twice() | |||
| 142 | a := autoEvaluator(r, checks, nil, nil) | 143 | a := autoEvaluator(r, checks, nil, nil) | ||
| 143 | defer a.Close(r.ctx) | 144 | defer a.Close(r.ctx) | ||
| 144 | r.drain() | 145 | r.drain() | ||
| @@ -159,17 +160,26 @@ func TestAutoMergeMergesAsEnablerWhenAChecksPassed(t *testing.T) { | |||||
| 159 | } | 160 | } | ||
| 160 | 161 | ||||
| 161 | func TestAutoMergeIgnoresFailingChecksBlockOff(t *testing.T) { | 162 | func TestAutoMergeIgnoresFailingChecksBlockOff(t *testing.T) { | ||
| 162 | - | r, mr, checks := autoRig(t) | 163 | + | for _, tc := range []struct { |
| 163 | - | enableAuto(t, r, mr) | 164 | + | name string |
| 164 | - | _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{FailingChecksBlock: ptr(false)}) | 165 | + | check domain.CICheck |
| 165 | - | require.NoError(t, err) | 166 | + | }{ |
| 166 | - | // No successful check: an unsuccessful head waits rather than merging. | 167 | + | {"failure", domain.CICheck{Status: domain.CICompleted, Conclusion: domain.CIFailure}}, |
| 167 | - | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CIFailure}}, nil) | 168 | + | {"pending", domain.CICheck{Status: domain.CIQueued}}, |
| 168 | - | a := autoEvaluator(r, checks, nil, nil) | 169 | + | } { |
| 169 | - | defer a.Close(r.ctx) | 170 | + | t.Run(tc.name, func(t *testing.T) { |
| 170 | - | require.True(t, a.Evaluate(r.repo)) | 171 | + | r, mr, checks := autoRig(t) |
| 171 | - | assert.True(t, storedAuto(t, r, mr).IsOpen()) | 172 | + | enableAuto(t, r, mr) |
| 172 | - | assert.NotNil(t, storedAuto(t, r, mr).AutoMerge) | 173 | + | _, err := r.repos.Update(r.ctx, root, r.repo, ports.UpdateRepoParams{FailingChecksBlock: ptr(false)}) |
| 174 | + | require.NoError(t, err) | |||
| 175 | + | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}, tc.check}, nil) | |||
| 176 | + | a := autoEvaluator(r, checks, nil, nil) | |||
| 177 | + | defer a.Close(r.ctx) | |||
| 178 | + | require.True(t, a.Evaluate(r.repo)) | |||
| 179 | + | assert.True(t, storedAuto(t, r, mr).IsOpen()) | |||
| 180 | + | assert.NotNil(t, storedAuto(t, r, mr).AutoMerge) | |||
| 181 | + | }) | |||
| 182 | + | } | |||
| 173 | } | 183 | } | ||
| 174 | 184 | ||||
| 175 | func TestAutoMergeCancelsOnNewHead(t *testing.T) { testAutoCancellation(t, "head") } | 185 | func TestAutoMergeCancelsOnNewHead(t *testing.T) { testAutoCancellation(t, "head") } | ||
| @@ -432,8 +442,8 @@ func TestAutoMergeRechecksSettledUnderMergeGate(t *testing.T) { | |||||
| 432 | r, mr, checks := autoRig(t) | 442 | r, mr, checks := autoRig(t) | ||
| 433 | enableAuto(t, r, mr) | 443 | enableAuto(t, r, mr) | ||
| 434 | activity := mocks.NewCIActivity(t) | 444 | activity := mocks.NewCIActivity(t) | ||
| 435 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(true).Once() | 445 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(true).Once() |
| 436 | - | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number, mr.Source, r.head()).Return(false).Once() | 446 | + | activity.EXPECT().Settled(mock.Anything, r.repo, mr.Number).Return(false).Once() |
| 437 | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil).Once() | 447 | checks.EXPECT().CommitChecks(mock.Anything, r.repo, r.head().String()).Return([]domain.CICheck{{Status: domain.CICompleted, Conclusion: domain.CISuccess}}, nil).Once() | ||
| 438 | a := autoEvaluator(r, checks, activity, nil) | 448 | a := autoEvaluator(r, checks, activity, nil) | ||
| 439 | defer a.Close(r.ctx) | 449 | defer a.Close(r.ctx) | ||
M internal/usecase/ci_activity_test.go +5 -5
| @@ -21,7 +21,7 @@ func TestSettledCountsQueuedPushesAndPullEvents(t *testing.T) { | |||
| 21 | 21 | trigger.EXPECT().OnRepoChanged(mock.Anything, repo).Return().Times(3) | |
| 22 | 22 | u.store.EXPECT().ListRunners(mock.Anything).Return(nil, nil).Once() | |
| 23 | 23 | ctx := context.Background() | |
| 24 | - | settled := func(n int64) bool { return u.ci.Settled(ctx, repo, n, "feat", domain.ObjectID{}) } | |
| 24 | + | settled := func(n int64) bool { return u.ci.Settled(ctx, repo, n) } | |
| 25 | 25 | assert.True(t, settled(1)) | |
| 26 | 26 | u.ci.QueuePush(ctx, repo) | |
| 27 | 27 | u.ci.QueuePush(ctx, repo) // dropped: queue capacity is one | |
| @@ -55,10 +55,10 @@ func TestSettledWaitsForPushDiscoveryAndReleasesOnError(t *testing.T) { | |||
| 55 | 55 | }() | |
| 56 | 56 | <-entered | |
| 57 | 57 | u.ci.PlanQueuedPush() | |
| 58 | - | assert.False(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | |
| 58 | + | assert.False(t, u.ci.Settled(context.Background(), repo, 1)) | |
| 59 | 59 | close(release) | |
| 60 | 60 | <-done | |
| 61 | - | assert.True(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | |
| 61 | + | assert.True(t, u.ci.Settled(context.Background(), repo, 1)) | |
| 62 | 62 | } | |
| 63 | 63 | ||
| 64 | 64 | func TestSettledReleasesQueuedPushOnShutdown(t *testing.T) { | |
| @@ -66,7 +66,7 @@ func TestSettledReleasesQueuedPushOnShutdown(t *testing.T) { | |||
| 66 | 66 | repo := name(t, "proj") | |
| 67 | 67 | u.ci.QueuePush(context.Background(), repo) | |
| 68 | 68 | u.ci.Close(context.Background()) | |
| 69 | - | assert.True(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | |
| 69 | + | assert.True(t, u.ci.Settled(context.Background(), repo, 1)) | |
| 70 | 70 | u.ci.QueuePush(context.Background(), repo) | |
| 71 | - | assert.True(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | |
| 71 | + | assert.True(t, u.ci.Settled(context.Background(), repo, 1)) | |
| 72 | 72 | } | |
| @@ -21,7 +21,7 @@ func TestSettledCountsQueuedPushesAndPullEvents(t *testing.T) { | |||||
| 21 | trigger.EXPECT().OnRepoChanged(mock.Anything, repo).Return().Times(3) | 21 | trigger.EXPECT().OnRepoChanged(mock.Anything, repo).Return().Times(3) | ||
| 22 | u.store.EXPECT().ListRunners(mock.Anything).Return(nil, nil).Once() | 22 | u.store.EXPECT().ListRunners(mock.Anything).Return(nil, nil).Once() | ||
| 23 | ctx := context.Background() | 23 | ctx := context.Background() | ||
| 24 | - | settled := func(n int64) bool { return u.ci.Settled(ctx, repo, n, "feat", domain.ObjectID{}) } | 24 | + | settled := func(n int64) bool { return u.ci.Settled(ctx, repo, n) } |
| 25 | assert.True(t, settled(1)) | 25 | assert.True(t, settled(1)) | ||
| 26 | u.ci.QueuePush(ctx, repo) | 26 | u.ci.QueuePush(ctx, repo) | ||
| 27 | u.ci.QueuePush(ctx, repo) // dropped: queue capacity is one | 27 | u.ci.QueuePush(ctx, repo) // dropped: queue capacity is one | ||
| @@ -55,10 +55,10 @@ func TestSettledWaitsForPushDiscoveryAndReleasesOnError(t *testing.T) { | |||||
| 55 | }() | 55 | }() | ||
| 56 | <-entered | 56 | <-entered | ||
| 57 | u.ci.PlanQueuedPush() | 57 | u.ci.PlanQueuedPush() | ||
| 58 | - | assert.False(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | 58 | + | assert.False(t, u.ci.Settled(context.Background(), repo, 1)) |
| 59 | close(release) | 59 | close(release) | ||
| 60 | <-done | 60 | <-done | ||
| 61 | - | assert.True(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | 61 | + | assert.True(t, u.ci.Settled(context.Background(), repo, 1)) |
| 62 | } | 62 | } | ||
| 63 | 63 | ||||
| 64 | func TestSettledReleasesQueuedPushOnShutdown(t *testing.T) { | 64 | func TestSettledReleasesQueuedPushOnShutdown(t *testing.T) { | ||
| @@ -66,7 +66,7 @@ func TestSettledReleasesQueuedPushOnShutdown(t *testing.T) { | |||||
| 66 | repo := name(t, "proj") | 66 | repo := name(t, "proj") | ||
| 67 | u.ci.QueuePush(context.Background(), repo) | 67 | u.ci.QueuePush(context.Background(), repo) | ||
| 68 | u.ci.Close(context.Background()) | 68 | u.ci.Close(context.Background()) | ||
| 69 | - | assert.True(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | 69 | + | assert.True(t, u.ci.Settled(context.Background(), repo, 1)) |
| 70 | u.ci.QueuePush(context.Background(), repo) | 70 | u.ci.QueuePush(context.Background(), repo) | ||
| 71 | - | assert.True(t, u.ci.Settled(context.Background(), repo, 1, "feat", domain.ObjectID{})) | 71 | + | assert.True(t, u.ci.Settled(context.Background(), repo, 1)) |
| 72 | } | 72 | } | ||