diff --git a/private/buf/bufsync/backfill_tags_test.go b/private/buf/bufsync/backfill_tags_test.go index 4163861823..161878ba27 100644 --- a/private/buf/bufsync/backfill_tags_test.go +++ b/private/buf/bufsync/backfill_tags_test.go @@ -37,7 +37,7 @@ func TestBackfilltags(t *testing.T) { moduleIdentityInHEAD, err := bufmoduleref.NewModuleIdentity("buf.build", "acme", "foo") require.NoError(t, err) prepareGitRepoBackfillTags(t, repo, moduleIdentityInHEAD) - mockHandler := newMockSyncHandler() + mockHandler := newTestSyncHandler() // prepare the top 5 commits as syncable commits, mark the rest as if they were already synced var ( commitCount int @@ -76,7 +76,8 @@ func TestBackfilltags(t *testing.T) { // in total the repo has at least 20 commits, we expect to backfill 11 of them // and sync the next 4 commits assert.GreaterOrEqual(t, len(allCommitsHashes), 20) - assert.Len(t, mockHandler.tagsByHash, 15) + tagsForHash := mockHandler.getRepo(moduleIdentityInHEAD).tagsForHash + assert.Len(t, tagsForHash, 15) // as follows: for i, commitHash := range allCommitsHashes { if i < 15 { @@ -85,10 +86,10 @@ func TestBackfilltags(t *testing.T) { // // The func it's backfilling more than 5 commits, because it needs to backfill until both // conditions are met, at least 5 commits and at least 24 hours. - assert.Contains(t, mockHandler.tagsByHash, commitHash) + assert.Contains(t, tagsForHash, commitHash) } else { // past the #15 the commits are too old, we don't backfill back there - assert.NotContains(t, mockHandler.tagsByHash, commitHash) + assert.NotContains(t, tagsForHash, commitHash) } } } diff --git a/private/buf/bufsync/bufsync_test.go b/private/buf/bufsync/bufsync_test.go index f5bac82812..af16de7548 100644 --- a/private/buf/bufsync/bufsync_test.go +++ b/private/buf/bufsync/bufsync_test.go @@ -32,35 +32,62 @@ type mockClock struct { func (c *mockClock) Now() time.Time { return c.now } -type mockSyncHandler struct { - syncedCommitsSHAs map[string]struct{} - commitsByBranch map[string][]bufsync.ModuleCommit - hashByTag map[string]git.Hash - tagsByHash map[string][]string - manualSyncPointByModuleByBranch map[string]map[string]git.Hash -} - -func newMockSyncHandler() *mockSyncHandler { - return &mockSyncHandler{ - syncedCommitsSHAs: make(map[string]struct{}), - commitsByBranch: make(map[string][]bufsync.ModuleCommit), - hashByTag: make(map[string]git.Hash), - tagsByHash: make(map[string][]string), - manualSyncPointByModuleByBranch: make(map[string]map[string]git.Hash), +type testRepo struct { + syncedGitHashes map[string]struct{} + branches map[string]*testBranch + tagsByName map[string]git.Hash + tagsForHash map[string][]string +} + +type testBranch struct { + manualSyncPoint git.Hash + commits []*testCommit +} + +type testCommit struct { + hash git.Hash + fromSync bufsync.ModuleCommit +} + +type testSyncHandler struct { + repos map[string]*testRepo +} + +func newTestSyncHandler() *testSyncHandler { + return &testSyncHandler{ + repos: make(map[string]*testRepo), } } -func (c *mockSyncHandler) setSyncPoint(branch string, hash git.Hash, identity bufmoduleref.ModuleIdentity) { - branchSyncpoints, ok := c.manualSyncPointByModuleByBranch[branch] - if !ok { - branchSyncpoints = make(map[string]git.Hash) - c.manualSyncPointByModuleByBranch[branch] = branchSyncpoints +func (c *testSyncHandler) getRepo(identity bufmoduleref.ModuleIdentity) *testRepo { + fullName := identity.IdentityString() + if _, ok := c.repos[fullName]; !ok { + c.repos[fullName] = &testRepo{ + syncedGitHashes: make(map[string]struct{}), + branches: make(map[string]*testBranch), + tagsByName: make(map[string]git.Hash), + tagsForHash: make(map[string][]string), + } } - branchSyncpoints[identity.IdentityString()] = hash - c.syncedCommitsSHAs[hash.Hex()] = struct{}{} + return c.repos[fullName] } -func (c *mockSyncHandler) HandleReadModuleError( +func (c *testSyncHandler) getRepoBranch(identity bufmoduleref.ModuleIdentity, branch string) *testBranch { + repo := c.getRepo(identity) + if _, ok := repo.branches[branch]; !ok { + repo.branches[branch] = &testBranch{} + } + return repo.branches[branch] +} + +func (c *testSyncHandler) setSyncPoint(branchName string, hash git.Hash, identity bufmoduleref.ModuleIdentity) { + repo := c.getRepo(identity) + repo.syncedGitHashes[hash.Hex()] = struct{}{} + branch := c.getRepoBranch(identity, branchName) + branch.manualSyncPoint = hash +} + +func (c *testSyncHandler) HandleReadModuleError( readErr *bufsync.ReadModuleError, ) bufsync.LookbackDecisionCode { if readErr.Code() == bufsync.ReadModuleErrorCodeUnexpectedName { @@ -69,7 +96,7 @@ func (c *mockSyncHandler) HandleReadModuleError( return bufsync.LookbackDecisionCodeSkip } -func (c *mockSyncHandler) InvalidBSRSyncPoint( +func (c *testSyncHandler) InvalidBSRSyncPoint( identity bufmoduleref.ModuleIdentity, branch string, gitHash git.Hash, @@ -79,7 +106,7 @@ func (c *mockSyncHandler) InvalidBSRSyncPoint( return errors.New("unimplemented") } -func (c *mockSyncHandler) BackfillTags( +func (c *testSyncHandler) BackfillTags( ctx context.Context, module bufmoduleref.ModuleIdentity, alreadySyncedHash git.Hash, @@ -87,42 +114,48 @@ func (c *mockSyncHandler) BackfillTags( committer git.Ident, tags []string, ) (string, error) { + repo := c.getRepo(module) for _, tag := range tags { - if previousHash, ok := c.hashByTag[tag]; ok { + if previousHash, ok := repo.tagsByName[tag]; ok { // clear previous tag - c.tagsByHash[previousHash.Hex()] = slices.DeleteFunc( - c.tagsByHash[previousHash.Hex()], + repo.tagsForHash[previousHash.Hex()] = slices.DeleteFunc( + repo.tagsForHash[previousHash.Hex()], func(previousTag string) bool { return previousTag == tag }, ) } - c.hashByTag[tag] = alreadySyncedHash + repo.tagsByName[tag] = alreadySyncedHash } - c.tagsByHash[alreadySyncedHash.Hex()] = tags + repo.tagsForHash[alreadySyncedHash.Hex()] = tags return "some-BSR-commit-name", nil } -func (c *mockSyncHandler) ResolveSyncPoint( +func (c *testSyncHandler) ResolveSyncPoint( ctx context.Context, module bufmoduleref.ModuleIdentity, - branch string, + branchName string, ) (git.Hash, error) { + branch := c.getRepoBranch(module, branchName) // if we have commits from SyncModuleCommit, prefer that over // manually set sync point - if branch, ok := c.commitsByBranch[branch]; ok && len(branch) > 0 { - // everything here is synced; return tip of branch - return branch[len(branch)-1].Commit().Hash(), nil - } - if branch, ok := c.manualSyncPointByModuleByBranch[branch]; ok { - if syncPoint, ok := branch[module.IdentityString()]; ok { - return syncPoint, nil + if len(branch.commits) > 0 { + for i := len(branch.commits) - 1; i >= 0; i-- { + commit := branch.commits[i] + if commit.fromSync != nil { + // the latest synced commit + return commit.hash, nil + } } + return nil, nil + } + if branch.manualSyncPoint != nil { + return branch.manualSyncPoint, nil } return nil, nil } -func (c *mockSyncHandler) SyncModuleCommit( +func (c *testSyncHandler) SyncModuleCommit( ctx context.Context, commit bufsync.ModuleCommit, ) error { @@ -131,8 +164,12 @@ func (c *mockSyncHandler) SyncModuleCommit( commit.Commit().Hash(), commit.Identity(), ) + branch := c.getRepoBranch(commit.Identity(), commit.Branch()) // append-only, no backfill; good enough for now! - c.commitsByBranch[commit.Branch()] = append(c.commitsByBranch[commit.Branch()], commit) + branch.commits = append(branch.commits, &testCommit{ + hash: commit.Commit().Hash(), + fromSync: commit, + }) _, err := c.BackfillTags( ctx, commit.Identity(), @@ -144,16 +181,17 @@ func (c *mockSyncHandler) SyncModuleCommit( return err } -func (c *mockSyncHandler) IsGitCommitSynced( +func (c *testSyncHandler) IsGitCommitSynced( ctx context.Context, module bufmoduleref.ModuleIdentity, hash git.Hash, ) (bool, error) { - _, isSynced := c.syncedCommitsSHAs[hash.Hex()] + repo := c.getRepo(module) + _, isSynced := repo.syncedGitHashes[hash.Hex()] return isSynced, nil } -func (c *mockSyncHandler) IsProtectedBranch( +func (c *testSyncHandler) IsProtectedBranch( ctx context.Context, moduleIdentity bufmoduleref.ModuleIdentity, branch string, @@ -161,4 +199,4 @@ func (c *mockSyncHandler) IsProtectedBranch( return branch == gittest.DefaultBranch || branch == bufmoduleref.Main, nil } -var _ bufsync.Handler = (*mockSyncHandler)(nil) +var _ bufsync.Handler = (*testSyncHandler)(nil) diff --git a/private/buf/bufsync/commits_to_sync_test.go b/private/buf/bufsync/commits_to_sync_test.go index af212df051..68064ccc1b 100644 --- a/private/buf/bufsync/commits_to_sync_test.go +++ b/private/buf/bufsync/commits_to_sync_test.go @@ -62,7 +62,7 @@ func TestCommitsToSyncWithNoPreviousSyncPoints(t *testing.T) { expectedCommits: 1, }, } - handler := newMockSyncHandler() // use same handler for all test cases + handler := newTestSyncHandler() // use same handler for all test cases for _, withOverride := range []bool{false, true} { for _, tc := range testCases { func(tc testCase) { @@ -86,7 +86,11 @@ func TestCommitsToSyncWithNoPreviousSyncPoints(t *testing.T) { ) require.NoError(t, err) require.NoError(t, syncer.Sync(context.Background())) - syncedCommits := handler.commitsByBranch[tc.branch] + identity := moduleIdentityInHEAD + if withOverride { + identity = moduleIdentityOverride + } + syncedCommits := handler.getRepoBranch(identity, tc.branch).commits require.Len(t, syncedCommits, tc.expectedCommits) }) }(tc) diff --git a/private/buf/bufsync/prepare_sync_test.go b/private/buf/bufsync/prepare_sync_test.go index 48cd809922..d30ed5835c 100644 --- a/private/buf/bufsync/prepare_sync_test.go +++ b/private/buf/bufsync/prepare_sync_test.go @@ -85,7 +85,7 @@ func TestPrepareSyncDuplicateIdentities(t *testing.T) { bufsync.NewRealClock(), repo, storagegit.NewProvider(repo.Objects()), - newMockSyncHandler(), + newTestSyncHandler(), opts..., ) require.NoError(t, err)