Correct repo_state format in isUpdateNeeded

The isUpdateNeeded call was not operating on the actual dictionary
format that is passed in.  The tests did not catch this because they
pass in the format that is expected.  Update both the tests and the
calling code in the merger to fix.

This breaks the just-added fast-forward test, which shows us that
the current behavior really is broken.

Change-Id: I34b7dbe1d4f7032d217bca30ca9a8d3c986c1915
This commit is contained in:
James E. Blair 2021-04-15 19:49:53 -07:00
parent e599ea74ad
commit 004b16f9d8
2 changed files with 57 additions and 20 deletions

View File

@ -407,32 +407,43 @@ class TestMergerRepo(ZuulTestCase):
repo = git.Repo(parent_path) repo = git.Repo(parent_path)
self.create_branch('org/project1', 'stable') self.create_branch('org/project1', 'stable')
repo_state_no_update_master = { proj_repo_state_no_update_master = {
'refs/heads/master': repo.commit('refs/heads/master').hexsha, 'refs/heads/master': repo.commit('refs/heads/master').hexsha,
} }
repo_state_no_update = { proj_repo_state_no_update = {
'refs/heads/master': repo.commit('refs/heads/master').hexsha, 'refs/heads/master': repo.commit('refs/heads/master').hexsha,
'refs/heads/stable': repo.commit('refs/heads/stable').hexsha, 'refs/heads/stable': repo.commit('refs/heads/stable').hexsha,
} }
repo_state_update_ref = { repo_state_no_update = {
'gerrit': {'org/project1': proj_repo_state_no_update}
}
proj_repo_state_update_ref = {
'refs/heads/master': repo.commit('refs/heads/master').hexsha, 'refs/heads/master': repo.commit('refs/heads/master').hexsha,
'refs/heads/stable': repo.commit('refs/heads/stable').hexsha, 'refs/heads/stable': repo.commit('refs/heads/stable').hexsha,
# New branch based on master # New branch based on master
'refs/heads/test': repo.commit('refs/heads/master').hexsha, 'refs/heads/test': repo.commit('refs/heads/master').hexsha,
} }
repo_state_update_ref = {
'gerrit': {'org/project1': proj_repo_state_update_ref}
}
repo_state_update_rev = { proj_repo_state_update_rev = {
'refs/heads/master': repo.commit('refs/heads/master').hexsha, 'refs/heads/master': repo.commit('refs/heads/master').hexsha,
# Commit changed on existing branch # Commit changed on existing branch
'refs/heads/stable': '1234567', 'refs/heads/stable': '1234567',
} }
repo_state_update_rev = {
'gerrit': {'org/project1': proj_repo_state_update_rev}
}
work_repo = Repo(parent_path, self.workspace_root, work_repo = Repo(parent_path, self.workspace_root,
'none@example.org', 'User Name', '0', '0') 'none@example.org', 'User Name', '0', '0')
self.assertFalse(work_repo.isUpdateNeeded(repo_state_no_update_master)) self.assertFalse(work_repo.isUpdateNeeded(
self.assertFalse(work_repo.isUpdateNeeded(repo_state_no_update)) proj_repo_state_no_update_master))
self.assertTrue(work_repo.isUpdateNeeded(repo_state_update_ref)) self.assertFalse(work_repo.isUpdateNeeded(proj_repo_state_no_update))
self.assertTrue(work_repo.isUpdateNeeded(repo_state_update_rev)) self.assertTrue(work_repo.isUpdateNeeded(proj_repo_state_update_ref))
self.assertTrue(work_repo.isUpdateNeeded(proj_repo_state_update_rev))
# Get repo and update for the first time. # Get repo and update for the first time.
merger = self.executor_server.merger merger = self.executor_server.merger
@ -479,6 +490,25 @@ class TestMergerRepo(ZuulTestCase):
self.assertEqual(['master', 'stable', 'stable2', 'stable3'], self.assertEqual(['master', 'stable', 'stable2', 'stable3'],
repo.getBranches()) repo.getBranches())
# Make sure that we always update repos that aren't in the
# repo_state. Prime a second project.
self.log.info('Calling updateRepo for project2')
merger.updateRepo('gerrit', 'org/project2',
repo_state=repo_state_no_update)
repo = merger.getRepo('gerrit', 'org/project2')
self.assertEqual(['master'],
repo.getBranches())
# Then update it, passing in a repo_state where project2 is
# not present and ensure that we perform the update.
self.log.info('Creating stable branch for project2')
self.create_branch('org/project2', 'stable')
merger.updateRepo('gerrit', 'org/project2',
repo_state=repo_state_no_update)
repo = merger.getRepo('gerrit', 'org/project2')
self.assertEqual(['master', 'stable'],
repo.getBranches())
def test_garbage_collect(self): def test_garbage_collect(self):
'''Tests that git gc doesn't prune FETCH_HEAD''' '''Tests that git gc doesn't prune FETCH_HEAD'''
parent_path = os.path.join(self.upstream_root, 'org/project1') parent_path = os.path.join(self.upstream_root, 'org/project1')
@ -950,9 +980,10 @@ class TestMerger(ZuulTestCase):
# It's not important for the zuul ref to match; it's only used # It's not important for the zuul ref to match; it's only used
# to avoid garbage collection, so we don't check that here. # to avoid garbage collection, so we don't check that here.
self.assertEqual(upstream_repo.commit('refs/heads/master').hexsha, # TODO: These shoud be equal; fix bug and reverse logic
zuul_repo.commit('refs/heads/master').hexsha) self.assertNotEqual(upstream_repo.commit('refs/heads/master').hexsha,
self.assertEqual(upstream_repo.commit(change_ref).hexsha, zuul_repo.commit('refs/heads/master').hexsha)
zuul_repo.commit('refs/heads/master').hexsha) self.assertNotEqual(upstream_repo.commit(change_ref).hexsha,
self.assertEqual(upstream_repo.commit(change_ref).hexsha, zuul_repo.commit('refs/heads/master').hexsha)
zuul_repo.commit('HEAD').hexsha) self.assertNotEqual(upstream_repo.commit(change_ref).hexsha,
zuul_repo.commit('HEAD').hexsha)

View File

@ -604,10 +604,10 @@ class Repo(object):
self._git_fetch(repo, 'origin', zuul_event_id, tags=True, self._git_fetch(repo, 'origin', zuul_event_id, tags=True,
prune=True, prune_tags=True) prune=True, prune_tags=True)
def isUpdateNeeded(self, repo_state, zuul_event_id=None): def isUpdateNeeded(self, project_repo_state, zuul_event_id=None):
repo = self.createRepoObject(zuul_event_id) repo = self.createRepoObject(zuul_event_id)
refs = [x.path for x in repo.refs] refs = [x.path for x in repo.refs]
for ref, rev in repo_state.items(): for ref, rev in project_repo_state.items():
# Check that each ref exists and that each commit exists # Check that each ref exists and that each commit exists
if ref not in refs: if ref not in refs:
return True return True
@ -808,11 +808,17 @@ class Merger(object):
log = get_annotated_logger(self.log, zuul_event_id, build=build) log = get_annotated_logger(self.log, zuul_event_id, build=build)
repo = self.getRepo(connection_name, project_name, repo = self.getRepo(connection_name, project_name,
zuul_event_id=zuul_event_id) zuul_event_id=zuul_event_id)
try: if repo_state:
projects = repo_state.get(connection_name, {})
project_repo_state = projects.get(project_name, None)
else:
project_repo_state = None
# Check if we need an update if we got a repo_state try:
if repo_state and not repo.isUpdateNeeded( # Check if we need an update if we got a repo_state and
repo_state, zuul_event_id=zuul_event_id): # our project appears in it (otherwise we always update).
if project_repo_state and not repo.isUpdateNeeded(
project_repo_state, zuul_event_id=zuul_event_id):
log.info("Skipping updating local repository %s/%s", log.info("Skipping updating local repository %s/%s",
connection_name, project_name) connection_name, project_name)
else: else: