From bb1fe50c6f32042625cc602dd5b55f76c9d1d384 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Tue, 4 Mar 2014 10:15:06 -0800 Subject: [PATCH] Fix ZUUL_COMMIT in certain conditions In a dependent pipeline with changes to multiple branches of a single project, ZUUL_COMMIT would sometimes point to a commit further back in the series rather than the commit that should be tested (ie, not the commit pointed to by ZUUL_REF). It's possible this situation could appear in other circumstances as well. The error was the re-use of a variable when creating Zuul refs for the entire series. The variable should have been reserved to indicate the final commit but instead could end up containing the most recent commit of another branch or even project. A test case is updated to detect both the condition where the ZUUL_COMMIT for two different changes are the same, as well as when ZUUL_REF does not point to ZUUL_COMMIT. Change-Id: I9074746070edeebdff9f2ef3c1bb00526c21cfee --- tests/test_scheduler.py | 31 +++++++++++++++++++++++++++++++ zuul/merger/merger.py | 4 ++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 95764404e6..56b996cbd6 100755 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -1945,6 +1945,15 @@ class TestScheduler(testtools.TestCase): self.fake_gerrit.addEvent(B.addApproval('APRV', 1)) self.fake_gerrit.addEvent(C.addApproval('APRV', 1)) self.waitUntilSettled() + queue = self.gearman_server.getQueue() + job_A = None + for job in queue: + if 'project-merge' in job.name: + job_A = job + ref_A = self.getParameter(job_A, 'ZUUL_REF') + commit_A = self.getParameter(job_A, 'ZUUL_COMMIT') + self.log.debug("Got Zuul ref for change A: %s" % ref_A) + self.log.debug("Got Zuul commit for change A: %s" % commit_A) self.gearman_server.release('.*-merge') self.waitUntilSettled() @@ -1954,7 +1963,10 @@ class TestScheduler(testtools.TestCase): if 'project-merge' in job.name: job_B = job ref_B = self.getParameter(job_B, 'ZUUL_REF') + commit_B = self.getParameter(job_B, 'ZUUL_COMMIT') self.log.debug("Got Zuul ref for change B: %s" % ref_B) + self.log.debug("Got Zuul commit for change B: %s" % commit_B) + self.gearman_server.release('.*-merge') self.waitUntilSettled() queue = self.gearman_server.getQueue() @@ -1962,7 +1974,9 @@ class TestScheduler(testtools.TestCase): if 'project-merge' in job.name: job_C = job ref_C = self.getParameter(job_C, 'ZUUL_REF') + commit_C = self.getParameter(job_C, 'ZUUL_COMMIT') self.log.debug("Got Zuul ref for change C: %s" % ref_C) + self.log.debug("Got Zuul commit for change C: %s" % commit_C) self.gearman_server.hold_jobs_in_queue = False self.gearman_server.release() self.waitUntilSettled() @@ -1972,15 +1986,32 @@ class TestScheduler(testtools.TestCase): repo_messages = [c.message.strip() for c in repo.iter_commits(ref_C)] + repo_shas = [c.hexsha for c in repo.iter_commits(ref_C)] repo_messages.reverse() correct_messages = ['initial commit', 'A-1', 'C-1'] + # Ensure the right commits are in the history for this ref self.assertEqual(repo_messages, correct_messages) + # Ensure ZUUL_REF -> ZUUL_COMMIT + self.assertEqual(repo_shas[0], commit_C) repo_messages = [c.message.strip() for c in repo.iter_commits(ref_B)] + repo_shas = [c.hexsha for c in repo.iter_commits(ref_B)] repo_messages.reverse() correct_messages = ['initial commit', 'mp commit', 'B-1'] self.assertEqual(repo_messages, correct_messages) + self.assertEqual(repo_shas[0], commit_B) + + repo_messages = [c.message.strip() + for c in repo.iter_commits(ref_A)] + repo_shas = [c.hexsha for c in repo.iter_commits(ref_A)] + repo_messages.reverse() + correct_messages = ['initial commit', 'A-1'] + self.assertEqual(repo_messages, correct_messages) + self.assertEqual(repo_shas[0], commit_A) + + self.assertNotEqual(ref_A, ref_B, ref_C) + self.assertNotEqual(commit_A, commit_B, commit_C) def test_one_job_project(self): "Test that queueing works with one job" diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index 10ce82cc60..ff6b682339 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -266,12 +266,12 @@ class Merger(object): recent[key] = commit # Set the Zuul ref for this item to point to the most recent # commits of each project-branch - for key, commit in recent.items(): + for key, mrc in recent.items(): project, branch = key try: repo = self.getRepo(project, None) zuul_ref = branch + '/' + item['ref'] - repo.createZuulRef(zuul_ref, commit) + repo.createZuulRef(zuul_ref, mrc) except Exception: self.log.exception("Unable to set zuul ref %s for " "item %s" % (zuul_ref, item))