test(mr): cover stale retry timers, branch disappearance and CAS conflict; drop unused clock
- commit
- fb5ff065561be451273dd15bf31fa732f9377595
- author
- brice · · 2026-10-19 16:11 UTC
- committer
- brice · · 2026-10-19 18:30 UTC
- parent
- c22da12
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +87 -4
M internal/usecase/mergeability_reconcile_test.go +0 -4
| @@ -16,10 +16,6 @@ import ( | |||
| 16 | 16 | "bnema.dev/ermite/internal/ports/mocks" | |
| 17 | 17 | ) | |
| 18 | 18 | ||
| 19 | - | type mergeTestClock struct{} | |
| 20 | - | ||
| 21 | - | func (mergeTestClock) Now() time.Time { return time.Time{} } | |
| 22 | - | ||
| 23 | 19 | func TestMergeabilityBackoffIsBounded(t *testing.T) { | |
| 24 | 20 | assert.Equal(t, time.Second, mergeBackoff(1)) | |
| 25 | 21 | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | |
| @@ -16,10 +16,6 @@ import ( | |||||
| 16 | "bnema.dev/ermite/internal/ports/mocks" | 16 | "bnema.dev/ermite/internal/ports/mocks" | ||
| 17 | ) | 17 | ) | ||
| 18 | 18 | ||||
| 19 | - | type mergeTestClock struct{} | |||
| 20 | - | ||||
| 21 | - | func (mergeTestClock) Now() time.Time { return time.Time{} } | |||
| 22 | - | ||||
| 23 | func TestMergeabilityBackoffIsBounded(t *testing.T) { | 19 | func TestMergeabilityBackoffIsBounded(t *testing.T) { | ||
| 24 | assert.Equal(t, time.Second, mergeBackoff(1)) | 20 | assert.Equal(t, time.Second, mergeBackoff(1)) | ||
| 25 | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | 21 | assert.Equal(t, 2*time.Second, mergeBackoff(2)) | ||
M internal/usecase/mergeability_test.go +87 -0
| @@ -584,3 +584,90 @@ func TestMergeabilitySuccessfulPassForgetsTheRetryBudget(t *testing.T) { | |||
| 584 | 584 | assert.Empty(t, r.afters) | |
| 585 | 585 | assert.Empty(t, r.passes) | |
| 586 | 586 | } | |
| 587 | + | ||
| 588 | + | // stepClock is a clock the test moves. | |
| 589 | + | type stepClock struct{ at atomic.Int64 } | |
| 590 | + | ||
| 591 | + | func (c *stepClock) Now() time.Time { return time.Unix(0, c.at.Load()).UTC() } | |
| 592 | + | func (c *stepClock) advance(d time.Duration) { c.at.Add(int64(d)) } | |
| 593 | + | ||
| 594 | + | // An expired timer must not consume a replacement retry of its repository. | |
| 595 | + | func TestMergeabilityStaleTimerKeepsReplacementRetry(t *testing.T) { | |
| 596 | + | repo := name(t, "proj") | |
| 597 | + | meta := mocks.NewRepoMetaStore(t) | |
| 598 | + | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 599 | + | clock := &stepClock{} | |
| 600 | + | clock.advance(time.Hour) | |
| 601 | + | armed := make(chan time.Duration, 8) | |
| 602 | + | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| 603 | + | Meta: meta, Clock: clock, Workers: 1, | |
| 604 | + | After: func(d time.Duration) <-chan time.Time { armed <- d; return make(chan time.Time) }, | |
| 605 | + | }) | |
| 606 | + | t.Cleanup(func() { svc.Close(context.Background()) }) | |
| 607 | + | svc.Start() | |
| 608 | + | ||
| 609 | + | svc.RetryForTest(repo) | |
| 610 | + | assert.Equal(t, time.Second, <-armed, "the scheduler armed the timer of retry A") | |
| 611 | + | oldDue, ok := svc.RetryDueForTest(repo) | |
| 612 | + | require.True(t, ok) | |
| 613 | + | ||
| 614 | + | svc.RetryDoneForTest(repo) | |
| 615 | + | clock.advance(10 * time.Second) | |
| 616 | + | svc.RetryForTest(repo) | |
| 617 | + | newDue, ok := svc.RetryDueForTest(repo) | |
| 618 | + | require.True(t, ok) | |
| 619 | + | require.True(t, newDue.After(oldDue)) | |
| 620 | + | ||
| 621 | + | // The old timer fires while the replacement is not yet due. | |
| 622 | + | assert.Empty(t, svc.PopDueForTest(repo, oldDue)) | |
| 623 | + | assert.Equal(t, 1, svc.ScheduledRetries()) | |
| 624 | + | // The timer armed for the replacement itself releases it. | |
| 625 | + | assert.Equal(t, []domain.RepoName{repo}, svc.PopDueForTest(repo, newDue)) | |
| 626 | + | assert.Zero(t, svc.ScheduledRetries()) | |
| 627 | + | } | |
| 628 | + | ||
| 629 | + | func reconcileMocks(t *testing.T, repo domain.RepoName) (*mocks.MergeRequestStore, *mocks.RepoMetaStore, *mocks.MergeGit, domain.MergeRequest) { | |
| 630 | + | t.Helper() | |
| 631 | + | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |
| 632 | + | store, meta, git := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t) | |
| 633 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | |
| 634 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) | |
| 635 | + | return store, meta, git, mr | |
| 636 | + | } | |
| 637 | + | ||
| 638 | + | func TestMergeabilityDisappearedBranchDeletesTheRecord(t *testing.T) { | |
| 639 | + | repo := name(t, "proj") | |
| 640 | + | store, meta, git, _ := reconcileMocks(t, repo) | |
| 641 | + | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(domain.ObjectID{}, domain.ErrNotFound) | |
| 642 | + | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(fakeOID(t, "b"), nil) | |
| 643 | + | store.EXPECT().DeleteMergeability(mock.Anything, repo, int64(7)).Return(nil).Once() | |
| 644 | + | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| 645 | + | Store: store, Meta: meta, Git: git, Clock: fixedClock{}, | |
| 646 | + | }) | |
| 647 | + | t.Cleanup(func() { svc.Close(context.Background()) }) | |
| 648 | + | assert.True(t, svc.ReconcileForTest(repo)) | |
| 649 | + | } | |
| 650 | + | ||
| 651 | + | func TestMergeabilityConflictMakesThePassFailAndRetries(t *testing.T) { | |
| 652 | + | repo := name(t, "proj") | |
| 653 | + | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |
| 654 | + | store, meta, git, mr := reconcileMocks(t, repo) | |
| 655 | + | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |
| 656 | + | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |
| 657 | + | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |
| 658 | + | git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | |
| 659 | + | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |
| 660 | + | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrConflict) | |
| 661 | + | ||
| 662 | + | asked := make(chan time.Duration, 4) | |
| 663 | + | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |
| 664 | + | Store: store, Meta: meta, Git: git, Clock: fixedClock{}, Workers: 1, | |
| 665 | + | After: func(d time.Duration) <-chan time.Time { asked <- d; return make(chan time.Time) }, | |
| 666 | + | }) | |
| 667 | + | t.Cleanup(func() { svc.Close(context.Background()) }) | |
| 668 | + | assert.False(t, svc.ReconcileForTest(repo)) | |
| 669 | + | ||
| 670 | + | svc.Start() | |
| 671 | + | svc.OnOpened(context.Background(), mr) | |
| 672 | + | assert.Equal(t, time.Second, <-asked, "the conflict scheduled a retry") | |
| 673 | + | } | |
| @@ -584,3 +584,90 @@ func TestMergeabilitySuccessfulPassForgetsTheRetryBudget(t *testing.T) { | |||||
| 584 | assert.Empty(t, r.afters) | 584 | assert.Empty(t, r.afters) | ||
| 585 | assert.Empty(t, r.passes) | 585 | assert.Empty(t, r.passes) | ||
| 586 | } | 586 | } | ||
| 587 | + | ||||
| 588 | + | // stepClock is a clock the test moves. | |||
| 589 | + | type stepClock struct{ at atomic.Int64 } | |||
| 590 | + | ||||
| 591 | + | func (c *stepClock) Now() time.Time { return time.Unix(0, c.at.Load()).UTC() } | |||
| 592 | + | func (c *stepClock) advance(d time.Duration) { c.at.Add(int64(d)) } | |||
| 593 | + | ||||
| 594 | + | // An expired timer must not consume a replacement retry of its repository. | |||
| 595 | + | func TestMergeabilityStaleTimerKeepsReplacementRetry(t *testing.T) { | |||
| 596 | + | repo := name(t, "proj") | |||
| 597 | + | meta := mocks.NewRepoMetaStore(t) | |||
| 598 | + | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |||
| 599 | + | clock := &stepClock{} | |||
| 600 | + | clock.advance(time.Hour) | |||
| 601 | + | armed := make(chan time.Duration, 8) | |||
| 602 | + | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |||
| 603 | + | Meta: meta, Clock: clock, Workers: 1, | |||
| 604 | + | After: func(d time.Duration) <-chan time.Time { armed <- d; return make(chan time.Time) }, | |||
| 605 | + | }) | |||
| 606 | + | t.Cleanup(func() { svc.Close(context.Background()) }) | |||
| 607 | + | svc.Start() | |||
| 608 | + | ||||
| 609 | + | svc.RetryForTest(repo) | |||
| 610 | + | assert.Equal(t, time.Second, <-armed, "the scheduler armed the timer of retry A") | |||
| 611 | + | oldDue, ok := svc.RetryDueForTest(repo) | |||
| 612 | + | require.True(t, ok) | |||
| 613 | + | ||||
| 614 | + | svc.RetryDoneForTest(repo) | |||
| 615 | + | clock.advance(10 * time.Second) | |||
| 616 | + | svc.RetryForTest(repo) | |||
| 617 | + | newDue, ok := svc.RetryDueForTest(repo) | |||
| 618 | + | require.True(t, ok) | |||
| 619 | + | require.True(t, newDue.After(oldDue)) | |||
| 620 | + | ||||
| 621 | + | // The old timer fires while the replacement is not yet due. | |||
| 622 | + | assert.Empty(t, svc.PopDueForTest(repo, oldDue)) | |||
| 623 | + | assert.Equal(t, 1, svc.ScheduledRetries()) | |||
| 624 | + | // The timer armed for the replacement itself releases it. | |||
| 625 | + | assert.Equal(t, []domain.RepoName{repo}, svc.PopDueForTest(repo, newDue)) | |||
| 626 | + | assert.Zero(t, svc.ScheduledRetries()) | |||
| 627 | + | } | |||
| 628 | + | ||||
| 629 | + | func reconcileMocks(t *testing.T, repo domain.RepoName) (*mocks.MergeRequestStore, *mocks.RepoMetaStore, *mocks.MergeGit, domain.MergeRequest) { | |||
| 630 | + | t.Helper() | |||
| 631 | + | mr := domain.MergeRequest{Repo: repo, Number: 7, Source: "feat", Target: "main", State: domain.MRStateOpen} | |||
| 632 | + | store, meta, git := mocks.NewMergeRequestStore(t), mocks.NewRepoMetaStore(t), mocks.NewMergeGit(t) | |||
| 633 | + | meta.EXPECT().Get(mock.Anything, repo).Return(domain.Repo{Name: repo, CreatedAt: now}, nil) | |||
| 634 | + | store.EXPECT().ListOpen(mock.Anything, repo).Return([]domain.MergeRequest{mr}, nil) | |||
| 635 | + | return store, meta, git, mr | |||
| 636 | + | } | |||
| 637 | + | ||||
| 638 | + | func TestMergeabilityDisappearedBranchDeletesTheRecord(t *testing.T) { | |||
| 639 | + | repo := name(t, "proj") | |||
| 640 | + | store, meta, git, _ := reconcileMocks(t, repo) | |||
| 641 | + | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(domain.ObjectID{}, domain.ErrNotFound) | |||
| 642 | + | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(fakeOID(t, "b"), nil) | |||
| 643 | + | store.EXPECT().DeleteMergeability(mock.Anything, repo, int64(7)).Return(nil).Once() | |||
| 644 | + | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |||
| 645 | + | Store: store, Meta: meta, Git: git, Clock: fixedClock{}, | |||
| 646 | + | }) | |||
| 647 | + | t.Cleanup(func() { svc.Close(context.Background()) }) | |||
| 648 | + | assert.True(t, svc.ReconcileForTest(repo)) | |||
| 649 | + | } | |||
| 650 | + | ||||
| 651 | + | func TestMergeabilityConflictMakesThePassFailAndRetries(t *testing.T) { | |||
| 652 | + | repo := name(t, "proj") | |||
| 653 | + | head, tip := fakeOID(t, "a"), fakeOID(t, "b") | |||
| 654 | + | store, meta, git, mr := reconcileMocks(t, repo) | |||
| 655 | + | meta.EXPECT().List(mock.Anything).Return(nil, nil) | |||
| 656 | + | git.EXPECT().BranchHead(mock.Anything, repo, "feat").Return(head, nil) | |||
| 657 | + | git.EXPECT().BranchHead(mock.Anything, repo, "main").Return(tip, nil) | |||
| 658 | + | git.EXPECT().IsAncestor(mock.Anything, repo, tip, head).Return(true, nil) | |||
| 659 | + | store.EXPECT().GetMergeability(mock.Anything, repo, int64(7)).Return(domain.Mergeability{}, domain.ErrNotFound) | |||
| 660 | + | store.EXPECT().PutMergeability(mock.Anything, mock.Anything).Return(domain.ErrConflict) | |||
| 661 | + | ||||
| 662 | + | asked := make(chan time.Duration, 4) | |||
| 663 | + | svc := usecase.NewMergeability(context.Background(), usecase.MergeabilityConfig{ | |||
| 664 | + | Store: store, Meta: meta, Git: git, Clock: fixedClock{}, Workers: 1, | |||
| 665 | + | After: func(d time.Duration) <-chan time.Time { asked <- d; return make(chan time.Time) }, | |||
| 666 | + | }) | |||
| 667 | + | t.Cleanup(func() { svc.Close(context.Background()) }) | |||
| 668 | + | assert.False(t, svc.ReconcileForTest(repo)) | |||
| 669 | + | ||||
| 670 | + | svc.Start() | |||
| 671 | + | svc.OnOpened(context.Background(), mr) | |||
| 672 | + | assert.Equal(t, time.Second, <-asked, "the conflict scheduled a retry") | |||
| 673 | + | } | |||