From 7cbd88cde4d7ff0fbf2d49f1fe2be64441330c92 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Fri, 12 Feb 2021 03:52:25 -0500 Subject: cbuildbot: drop CQ-DEPEND support We haven't used CQ-DEPEND tags for a while since the new CQ doesn't respect them, so drop the logic from chromite. BUG=chromium:1025955 TEST=CQ passes Change-Id: Ie353a4d781204e6fc61c10d07b466da0bd585849 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2690934 Reviewed-by: George Engelbrecht Commit-Queue: Mike Frysinger Tested-by: Mike Frysinger --- cbuildbot/patch_series.py | 35 +-- cbuildbot/patch_series_unittest.py | 361 +------------------------------ cbuildbot/stages/sync_stages_unittest.py | 4 - lib/patch.py | 88 -------- lib/patch_unittest.py | 93 -------- scripts/gerrit.py | 5 +- 6 files changed, 11 insertions(+), 575 deletions(-) diff --git a/cbuildbot/patch_series.py b/cbuildbot/patch_series.py index 2c46b8317..3cdab8178 100644 --- a/cbuildbot/patch_series.py +++ b/cbuildbot/patch_series.py @@ -433,9 +433,8 @@ class PatchSeries(object): """ plan = [] gerrit_deps_seen = cros_patch.PatchCache() - cq_deps_seen = cros_patch.PatchCache() self._AddChangeToPlanWithDeps(change, plan, gerrit_deps_seen, - cq_deps_seen, limit_to=limit_to) + limit_to=limit_to) return plan def CreateTransactions(self, changes, limit_to=None): @@ -468,8 +467,7 @@ class PatchSeries(object): @_PatchWrapException def _AddChangeToPlanWithDeps(self, change, plan, gerrit_deps_seen, - cq_deps_seen, limit_to=None, - include_cq_deps=True, + limit_to=None, remaining_depth=MAX_PLAN_RECURSION): """Add a change and its dependencies into a |plan|. @@ -479,12 +477,8 @@ class PatchSeries(object): |change| and any necessary dependencies to |plan|. gerrit_deps_seen: The changes whose Gerrit dependencies have already been processed. - cq_deps_seen: The changes whose CQ-DEPEND and Gerrit dependencies have - already been processed. limit_to: If non-None, limit the allowed uncommitted patches to what's in that container/mapping. - include_cq_deps: If True, include CQ dependencies in the list - of dependencies. Defaults to True. remaining_depth: Amount of permissible recursion depth from this call. Raises: @@ -499,33 +493,20 @@ class PatchSeries(object): # Get a list of the changes that haven't been committed. # These are returned as cros_patch.PatchQuery objects. - gerrit_deps, cq_deps = self.GetDepsForChange(change) + gerrit_deps = self.GetDepsForChange(change) # Only process the Gerrit dependencies for each change once. We prioritize # Gerrit dependencies over CQ dependencies, since Gerrit dependencies might # be required in order for the change to apply. - old_plan_len = len(plan) if change not in gerrit_deps_seen: gerrit_deps = self._LookupUncommittedChanges( gerrit_deps, limit_to=limit_to) gerrit_deps_seen.Inject(change) for dep in gerrit_deps: - self._AddChangeToPlanWithDeps(dep, plan, gerrit_deps_seen, cq_deps_seen, - limit_to=limit_to, include_cq_deps=False, + self._AddChangeToPlanWithDeps(dep, plan, gerrit_deps_seen, + limit_to=limit_to, remaining_depth=remaining_depth - 1) - if include_cq_deps and change not in cq_deps_seen: - cq_deps = self._LookupUncommittedChanges( - cq_deps, limit_to=limit_to) - cq_deps_seen.Inject(change) - for dep in plan[old_plan_len:] + cq_deps: - # Add the requested change (plus deps) to our plan, if it we aren't - # already in the process of doing that. - if dep not in cq_deps_seen: - self._AddChangeToPlanWithDeps(dep, plan, gerrit_deps_seen, - cq_deps_seen, limit_to=limit_to, - remaining_depth=remaining_depth - 1) - # If there are cyclic dependencies, we might have already applied this # patch as part of dependency resolution. If not, apply this patch. if change not in plan: @@ -545,11 +526,7 @@ class PatchSeries(object): """ val = self._change_deps_cache.get(change) if val is None: - git_repo = self.GetGitRepoForChange(change) - val = self._change_deps_cache[change] = ( - change.GerritDependencies(), - change.PaladinDependencies(git_repo)) - + val = self._change_deps_cache[change] = change.GerritDependencies() return val def InjectCommittedPatches(self, changes): diff --git a/cbuildbot/patch_series_unittest.py b/cbuildbot/patch_series_unittest.py index 8d92ce32f..f2ff3f093 100644 --- a/cbuildbot/patch_series_unittest.py +++ b/cbuildbot/patch_series_unittest.py @@ -49,23 +49,20 @@ class FakePatch(partial_mock.PartialMock): """Mocks out dependency and fetch methods of GitRepoPatch. Examples: - set FakePatch.parents, .cq and .build_roots per patch, and set + set FakePatch.parents and .build_roots per patch, and set FakePatch.assertEqual to your TestCase's assertEqual method. The behavior - of GerritDependencies, PaladinDependencies and Fetch` depends on the patch - id. + of GerritDependencies, and Fetch` depends on the patch id. """ TARGET = 'chromite.lib.patch.GitRepoPatch' - ATTRS = ('GerritDependencies', 'PaladinDependencies', 'Fetch') + ATTRS = ('GerritDependencies', 'Fetch') parents = {} - cq = {} build_root = None assertEqual = None def PreStart(self): FakePatch.parents = {} - FakePatch.cq = {} def PreStop(self): FakePatch.build_root = None @@ -74,15 +71,12 @@ class FakePatch(partial_mock.PartialMock): def GerritDependencies(self, patch): return [cros_patch.ParsePatchDep(x) for x in self.parents[patch.id]] - def PaladinDependencies(self, patch, path): - self._assertPath(patch, path) - return [cros_patch.ParsePatchDep(x) for x in self.cq[patch.id]] - def Fetch(self, patch, path): self._assertPath(patch, path) return patch.sha1 def _assertPath(self, patch, path): + # pylint: disable=not-callable self.assertEqual(path, os.path.join(self.build_root, patch.project)) @@ -207,350 +201,3 @@ class TestUploadedLocalPatch(PatchSeriesTestCase): self.assertEqual(len(not_in_manifest), 2) self.assertEqual(not_in_manifest[0].patch, patch_1) self.assertEqual(not_in_manifest[1].patch, patch_2) - - -class TestPatchSeries(PatchSeriesTestCase): - """Tests resolution and applying logic of validation_pool.ValidationPool.""" - - def setUp(self): - self.StartPatcher(FakePatch()) - self.PatchObject(FakePatch, 'assertEqual', new=self.assertEqual) - self.PatchObject(FakePatch, 'build_root', new=self.build_root) - self.PatchObject(patch_series, '_FetchChangesForRepo', - new=FakeFetchChangesForRepo) - self.StartPatcher(FakeGerritPatch()) - - def SetPatchDeps(self, patch, parents=(), cq=()): - """Set the dependencies of |patch|. - - Args: - patch: The patch to process. - parents: A set of strings to set as parents of |patch|. - cq: A set of strings to set as paladin dependencies of |patch|. - """ - FakePatch.parents[patch.id] = parents - FakePatch.cq[patch.id] = cq - - def testApplyWithDeps(self): - """Test that we can apply changes correctly and respect deps. - - This tests a simple out-of-order change where change1 depends on change2 - but tries to get applied before change2. What should happen is that - we should notice change2 is a dep of change1 and apply it first. - """ - series = self.GetPatchSeries() - - patch1, patch2 = patches = self.GetPatches(2) - - self.SetPatchDeps(patch2) - self.SetPatchDeps(patch1, [patch2.id]) - - apply_mocks = [self.SetPatchApply(x) for x in patches] - self.assertResults(series, patches, [patch2, patch1]) - self.CheckPatchApply(apply_mocks) - - def testSha1Deps(self): - """Test that we can apply changes correctly and respect sha1 deps. - - This tests a simple out-of-order change where change1 depends on change2 - but tries to get applied before change2. What should happen is that - we should notice change2 is a dep of change1 and apply it first. - """ - site_params = config_lib.GetSiteParams() - series = self.GetPatchSeries() - - patch1, patch2, patch3 = patches = self.GetPatches(3) - patch3.remote = site_params.INTERNAL_REMOTE - - self.SetPatchDeps(patch1, ['chromium:%s' % patch2.sha1]) - self.SetPatchDeps(patch2, ['chrome-internal:%s' % patch3.sha1]) - self.SetPatchDeps(patch3) - - apply_mocks = [self.SetPatchApply(x) for x in patches] - self.assertResults(series, patches, [patch3, patch2, patch1]) - self.CheckPatchApply(apply_mocks) - - def testGerritNumberDeps(self): - """Test that we can apply CQ-DEPEND changes in the right order.""" - series = self.GetPatchSeries() - - patch1, patch2, patch3 = patches = self.GetPatches(3) - - self.SetPatchDeps(patch1, cq=[patch2.id]) - self.SetPatchDeps(patch2, cq=[patch3.gerrit_number]) - self.SetPatchDeps(patch3, cq=[patch1.gerrit_number]) - - apply_mocks = [self.SetPatchApply(x) for x in patches] - self.assertResults(series, patches, patches[::-1]) - self.CheckPatchApply(apply_mocks) - - def testGerritLazyMapping(self): - """Given a patch lacking a gerrit number, via gerrit, map it to that change. - - Literally, this ensures that local patches pushed up- lacking a gerrit - number- are mapped back to a changeid via asking gerrit for that number, - then the local matching patch is used if available. - """ - series = self.GetPatchSeries() - - patch1 = self.MockPatch() - self.PatchObject(patch1, 'LookupAliases', return_value=[patch1.id]) - patch2 = self.MockPatch(change_id=int(patch1.change_id[1:])) - patch3 = self.MockPatch() - - self.SetPatchDeps(patch3, cq=[patch2.gerrit_number]) - self.SetPatchDeps(patch2) - self.SetPatchDeps(patch1) - - apply_mocks = [self.SetPatchApply(x) for x in (patch1, patch3)] - - helper = series._helper_pool.GetHelper(patch2.remote) - def QueryChecker(query, **kwargs): - self.assertEqual(query, patch2.gerrit_number) - self.assertTrue(kwargs['must_match']) - return patch2 - helper.QuerySingleRecord.side_effect = QueryChecker - - applied = self.assertResults(series, [patch1, patch3], [patch1, patch3])[0] - self.assertIs(applied[0], patch1) - self.assertIs(applied[1], patch3) - self.CheckPatchApply(apply_mocks) - - def testCrosGerritDeps(self, cros_internal=True): - """Test that we can apply changes correctly and respect deps. - - This tests a simple out-of-order change where change1 depends on change3 - but tries to get applied before it. What should happen is that - we should notice the dependency and apply change3 first. - """ - site_params = config_lib.GetSiteParams() - helper_pool = self.MakeHelper(cros_internal=cros_internal, cros=True) - series = self.GetPatchSeries(helper_pool=helper_pool) - - patch1 = self.MockPatch(remote=site_params.EXTERNAL_REMOTE) - patch2 = self.MockPatch(remote=site_params.INTERNAL_REMOTE) - patch3 = self.MockPatch(remote=site_params.EXTERNAL_REMOTE) - patches = [patch1, patch2, patch3] - if cros_internal: - applied_patches = [patch3, patch2, patch1] - else: - applied_patches = [patch3, patch1] - - self.SetPatchDeps(patch1, [patch3.id]) - self.SetPatchDeps(patch2) - self.SetPatchDeps(patch3, cq=[patch2.id]) - - apply_mocks = [ - self.SetPatchApply(patch1), - self.SetPatchApply(patch3), - ] - if cros_internal: - apply_mocks.append(self.SetPatchApply(patch2)) - - self.assertResults(series, patches, applied_patches) - self.CheckPatchApply(apply_mocks) - - def testExternalCrosGerritDeps(self): - """Test that we exclude internal deps on external trybot.""" - self.testCrosGerritDeps(cros_internal=False) - - def testApplyMissingDep(self): - """Test that we don't try to apply a change without met dependencies. - - Patch2 is in the validation pool that depends on Patch1 (which is not) - Nothing should get applied. - """ - series = self.GetPatchSeries() - - patch1, patch2 = self.GetPatches(2) - - self.SetPatchDeps(patch2, [patch1.id]) - - helper = series._helper_pool.GetHelper(patch1.remote) - def QueryChecker(query, **kwargs): - self.assertEqual(query, patch1.full_change_id) - self.assertTrue(kwargs['must_match']) - return patch1 - helper.QuerySingleRecord.side_effect = QueryChecker - - self.assertResults(series, [patch2], - [], [patch2]) - - def testApplyWithCommittedDeps(self): - """Test that we apply a change with dependency already committed.""" - series = self.GetPatchSeries() - - # Use for basic commit check. - patch1 = self.GetPatches(1, is_merged=True) - patch2 = self.GetPatches(1) - - self.SetPatchDeps(patch2, [patch1.id]) - apply_mocks = [self.SetPatchApply(patch2)] - - # Used to ensure that an uncommitted change put in the lookup cache - # isn't invalidly pulled into the graph... - patch3, patch4, patch5 = self.GetPatches(3) - - self.SetPatchDeps(patch4, [patch3.id]) - self.SetPatchDeps(patch5, [patch3.id]) - - # Sanity check so we only set up one mock query. - self.assertEqual(patch1.remote, patch3.remote) - - helper = series._helper_pool.GetHelper(patch1.remote) - def QueryChecker(query, **kwargs): - self.assertTrue(kwargs['must_match']) - if query == patch1.full_change_id: - return patch1 - elif query == patch3.full_change_id: - return patch3 - else: - self.fail() - helper.QuerySingleRecord.side_effect = QueryChecker - - self.assertResults(series, [patch2, patch4, patch5], [patch2], - [patch4, patch5]) - self.CheckPatchApply(apply_mocks) - - def testCyclicalDeps(self): - """Verify that the machinery handles cycles correctly.""" - series = self.GetPatchSeries() - - patch1, patch2, patch3 = patches = self.GetPatches(3) - - self.SetPatchDeps(patch1, [patch2.id]) - self.SetPatchDeps(patch2, cq=[patch3.id]) - self.SetPatchDeps(patch3, [patch1.id]) - - apply_mocks = [self.SetPatchApply(x) for x in patches] - self.assertResults(series, patches, [patch2, patch1, patch3]) - self.CheckPatchApply(apply_mocks) - - def testApplyWithNotInManifestException(self): - """Test Apply with NotInManifest Exception.""" - series = self.GetPatchSeries() - - patch1, patch2, patch3 = patches = self.GetPatches(3) - self.SetPatchDeps(patch1, []) - self.SetPatchDeps(patch2, []) - self.SetPatchDeps(patch3, []) - - apply_mocks = [self.SetPatchApply(x) for x in (patch1, patch2)] - - not_in_manifest = [cros_patch.ChangeNotInManifest(patch3)] - series.FetchChanges = lambda changes: ([patch1, patch2], not_in_manifest) - - self.assertResults(series, patches, applied=[patch1, patch2], - failed_tot=[patch3]) - self.CheckPatchApply(apply_mocks) - - def testComplexCyclicalDeps(self, fail=False): - """Verify handling of two interdependent cycles.""" - series = self.GetPatchSeries() - - # Create two cyclically interdependent patch chains. - # Example: Two patch series A1<-A2<-A3<-A4 and B1<-B2<-B3<-B4. A1 has a - # CQ-DEPEND on B4 and B1 has a CQ-DEPEND on A4, so all of the patches must - # be committed together. - chain1, chain2 = chains = self.GetPatches(4), self.GetPatches(4) - for chain in chains: - (other_chain,) = [x for x in chains if x != chain] - self.SetPatchDeps(chain[0], [], cq=[other_chain[-1].id]) - for i in range(1, len(chain)): - self.SetPatchDeps(chain[i], [chain[i-1].id]) - - # Apply the second-last patch first, so that the last patch in the series - # will be pulled in via the CQ-DEPEND on the other patch chain. - to_apply = [chain1[-2]] + [x for x in (chain1 + chain2) if x != chain1[-2]] - - # Mark all the patches but the last ones as applied successfully. - apply_mocks = [self.SetPatchApply(x) for x in chain1 + chain2[:-1]] - - if fail: - # Pretend that chain2[-1] failed to apply. - self.SetPatchApply(chain2[-1]).side_effect = ( - cros_patch.ApplyPatchException(chain1[-1])) - applied = [] - failed_tot = to_apply - else: - # We apply the patches in this order since the last patch in chain1 - # is pulled in via CQ-DEPEND. - apply_mocks.append(self.SetPatchApply(chain2[-1])) - applied = chain1[:2] + chain2[:-1] + chain1[2:] + chain2[-1:] - failed_tot = [] - - self.assertResults(series, to_apply, applied=applied, failed_tot=failed_tot) - self.CheckPatchApply(apply_mocks) - - def testFailingComplexCyclicalDeps(self): - """Verify handling of failing interlocked cycles.""" - self.testComplexCyclicalDeps(fail=True) - - def testApplyPartialFailures(self): - """Test that can apply changes correctly when one change fails to apply. - - This tests a simple change order where 1 depends on 2 and 1 fails to apply. - Only 1 should get tried as 2 will abort once it sees that 1 can't be - applied. 3 with no dependencies should go through fine. - - Since patch1 fails to apply, we should also get a call to handle the - failure. - """ - series = self.GetPatchSeries() - - patch1, patch2, patch3, patch4 = patches = self.GetPatches(4) - - self.SetPatchDeps(patch1) - self.SetPatchDeps(patch2, [patch1.id]) - self.SetPatchDeps(patch3) - self.SetPatchDeps(patch4) - - self.SetPatchApply(patch1).side_effect = ( - cros_patch.ApplyPatchException(patch1)) - apply_mock = self.SetPatchApply(patch3) - self.SetPatchApply(patch4).side_effect = ( - cros_patch.ApplyPatchException(patch1, inflight=True)) - - self.assertResults(series, patches, - [patch3], [patch2, patch1], [patch4]) - self.CheckPatchApply([apply_mock]) - - def testComplexApply(self): - """More complex deps test. - - This tests a total of 2 change chains where the first change we see - only has a partial chain with the 3rd change having the whole chain i.e. - 1->2, 3->1->2. Since we get these in the order 1,2,3,4,5 the order we - should apply is 2,1,3,4,5. - - This test also checks the patch order to verify that Apply re-orders - correctly based on the chain. - """ - series = self.GetPatchSeries() - - patch1, patch2, patch3, patch4, patch5 = patches = self.GetPatches(5) - - self.SetPatchDeps(patch1, [patch2.id]) - self.SetPatchDeps(patch2) - self.SetPatchDeps(patch3, [patch1.id, patch2.id]) - self.SetPatchDeps(patch4, cq=[patch5.id]) - self.SetPatchDeps(patch5) - - apply_mocks = [self.SetPatchApply(x) - for x in (patch2, patch1, patch3, patch4, patch5)] - self.assertResults( - series, patches, [patch2, patch1, patch3, patch5, patch4]) - self.CheckPatchApply(apply_mocks) - - def testApplyStandalonePatches(self): - """Simple apply of two changes with no dependent CL's.""" - series = self.GetPatchSeries() - - patches = self.GetPatches(3) - - for patch in patches: - self.SetPatchDeps(patch) - - apply_mocks = [self.SetPatchApply(x) for x in patches] - self.assertResults(series, patches, patches) - self.CheckPatchApply(apply_mocks) diff --git a/cbuildbot/stages/sync_stages_unittest.py b/cbuildbot/stages/sync_stages_unittest.py index 3c078501c..37bc49bfb 100644 --- a/cbuildbot/stages/sync_stages_unittest.py +++ b/cbuildbot/stages/sync_stages_unittest.py @@ -222,10 +222,6 @@ class MockPatch(mock.MagicMock): """Return whether this patch is merged or in the middle of being merged.""" return self.status in ('SUBMITTED', 'MERGED') - def IsMergeable(self): - """Default implementation of IsMergeable, stubbed out by some tests.""" - return True - def GetDiffStatus(self, _): return self.mock_diff_status diff --git a/lib/patch.py b/lib/patch.py index 14d56c7a4..77455caa1 100644 --- a/lib/patch.py +++ b/lib/patch.py @@ -63,23 +63,6 @@ ATTR_PASS_COUNT = 'pass_count' ATTR_TOTAL_FAIL_COUNT = 'total_fail_count' ATTR_COMMIT_MESSAGE = 'commit_message' -ALL_ATTRS = ( - ATTR_REMOTE, - ATTR_GERRIT_NUMBER, - ATTR_PROJECT, - ATTR_BRANCH, - ATTR_PROJECT_URL, - ATTR_REF, - ATTR_CHANGE_ID, - ATTR_COMMIT, - ATTR_PATCH_NUMBER, - ATTR_OWNER_EMAIL, - ATTR_FAIL_COUNT, - ATTR_PASS_COUNT, - ATTR_TOTAL_FAIL_COUNT, - ATTR_COMMIT_MESSAGE, -) - def ParseSHA1(text, error_ok=True): """Checks if |text| conforms to the SHA1 format and parses it. @@ -347,21 +330,6 @@ class DependencyError(PatchException): return key_error -class BrokenCQDepends(PatchException): - """Raised if a patch has a CQ-DEPEND line that is ill formated.""" - - def __init__(self, patch, text, msg=None): - PatchException.__init__(self, patch) - self.text = text - self.msg = msg - self.args = (patch, text, msg) - - def ShortExplanation(self): - s = 'has a malformed CQ-DEPEND target: %s' % (self.text,) - if self.msg is not None: - s += '; %s' % (self.msg,) - return s - class BrokenChangeID(PatchException): """Raised if a patch has an invalid or missing Change-ID.""" @@ -1367,37 +1335,6 @@ class GitRepoPatch(PatchQuery): return ParseChangeID(change_id_match) - def PaladinDependencies(self, git_repo): - """Returns an ordered list of dependencies based on the Commit Message. - - Parses the Commit message for this change looking for lines that follow - the format: - - CQ-DEPEND=change_num+ e.g. - - A commit which depends on a couple others. - - BUG=blah - TEST=blah - CQ-DEPEND=10001,10002 - """ - dependencies = [] - logging.debug('Checking for CQ-DEPEND dependencies for change %s', self) - - # Only fetch the commit message if needed. - if self.commit_message is None: - self.Fetch(git_repo) - - try: - dependencies = GetPaladinDeps(self.commit_message) - except ValueError as e: - raise BrokenCQDepends(self, str(e)) - - if dependencies: - logging.debug('Found %s Paladin dependencies for change %s', - dependencies, self) - return dependencies - def _FindEbuildConflicts(self, git_repo, upstream, inflight=False): """Verify that there are no ebuild conflicts in the given |git_repo|. @@ -2049,35 +1986,10 @@ class GerritPatch(GerritFetchOnlyPatch): else: return value in type_approvals - def HasApprovals(self, flags): - """Return whether the current patchset has the specified approval. - - Args: - flags: A dictionary of flag -> value mappings in - GerritPatch.HasApproval format. - ex: { 'CRVW': '2', 'VRIF': '1', 'COMR': ('1', '2') } - - returns boolean telling if all flag requirements are met. - """ - return all(self.HasApproval(field, value) - for field, value in flags.items()) - def IsPrivate(self): """Return whether this CL is currently marked Private.""" return self.private - def WasVetoed(self): - """Return whether this CL was vetoed with VRIF=-1 or CRVW=-2.""" - return self.HasApproval('VRIF', '-1') or self.HasApproval('CRVW', '-2') - - def IsMergeable(self): - """Return true if all Gerrit approvals required for submission are set.""" - return not self.GetMergeException() - - def HasReadyFlag(self): - """Return true if the commit-ready flag is set.""" - return self.HasApproval('COMR', ('1', '2')) - def GetMergeException(self): """Return the reason why this change is not mergeable. diff --git a/lib/patch_unittest.py b/lib/patch_unittest.py index ca02a7f5b..76ca236dd 100644 --- a/lib/patch_unittest.py +++ b/lib/patch_unittest.py @@ -626,99 +626,6 @@ class TestGitRepoPatch(GitRepoPatchTestCase): def testInternalLookupAliases(self): self._assertLookupAliases(config_lib.GetSiteParams().INTERNAL_REMOTE) - def _CheckPaladin(self, repo, master_id, ids, extra): - patch = self.CommitChangeIdFile( - repo, master_id, extra=extra, - filename='paladincheck', content=str(_GetNumber())) - deps = patch.PaladinDependencies(repo) - # Assert that our parsing unique'ifies the results. - self.assertEqual(len(deps), len(set(deps))) - # Verify that we have the correct dependencies. - dep_ids = [] - dep_ids += [(dep.remote, dep.change_id) for dep in deps - if dep.change_id is not None] - dep_ids += [(dep.remote, dep.gerrit_number) for dep in deps - if dep.gerrit_number is not None] - dep_ids += [(dep.remote, dep.sha1) for dep in deps - if dep.sha1 is not None] - for input_id in ids: - change_tuple = cros_patch.StripPrefix(input_id) - self.assertIn(change_tuple, dep_ids) - - return patch - - def testPaladinDependencies(self): - git1 = self._MakeRepo('git1', self.source) - cid1, cid2, cid3, cid4 = self.MakeChangeId(4) - # Verify it handles nonexistant CQ-DEPEND. - self._CheckPaladin(git1, cid1, [], '') - # Single key, single value. - self._CheckPaladin(git1, cid1, [cid2], - 'CQ-DEPEND=%s' % cid2) - self._CheckPaladin(git1, cid1, [cid2], - 'Cq-Depend: %s' % cid2) - # Single key, gerrit number. - self._CheckPaladin(git1, cid1, ['123'], - 'CQ-DEPEND=%s' % 123) - self._CheckPaladin(git1, cid1, ['123'], - 'Cq-Depend: %s' % 123) - # Single key, gerrit number. - self._CheckPaladin(git1, cid1, ['123456'], - 'CQ-DEPEND=%s' % 123456) - self._CheckPaladin(git1, cid1, ['123456'], - 'Cq-Depend: %s' % 123456) - # Single key, gerrit number; ensure it - # cuts off before a million changes (this - # is done to avoid collisions w/ sha1 when - # we're using shortened versions). - self.assertRaises(cros_patch.BrokenCQDepends, - self._CheckPaladin, git1, cid1, - ['123456789'], 'CQ-DEPEND=%s' % '123456789') - # Single key, gerrit number, internal. - self._CheckPaladin(git1, cid1, ['*123'], - 'CQ-DEPEND=%s' % '*123') - # Ensure SHA1's aren't allowed. - sha1 = '0' * 40 - self.assertRaises(cros_patch.BrokenCQDepends, - self._CheckPaladin, git1, cid1, - [sha1], 'CQ-DEPEND=%s' % sha1) - - # Single key, multiple values - self._CheckPaladin(git1, cid1, [cid2, '1223'], - 'CQ-DEPEND=%s %s' % (cid2, '1223')) - self._CheckPaladin(git1, cid1, [cid2, '1223'], - 'Cq-Depend:%s %s' % (cid2, '1223')) - # Dumb comma behaviour - self._CheckPaladin(git1, cid1, [cid2, cid3], - 'CQ-DEPEND=%s, %s,' % (cid2, cid3)) - self._CheckPaladin(git1, cid1, [cid2, cid3], - 'Cq-Depend: %s, %s,' % (cid2, cid3)) - # Multiple keys. - self._CheckPaladin(git1, cid1, [cid2, '*245', cid4], - 'CQ-DEPEND=%s, %s\nCQ-DEPEND=%s' % (cid2, '*245', cid4)) - - # Ensure it goes boom on invalid data. - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], 'CQ-DEPEND=monkeys') - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], 'Cq-Depend:monkeys') - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], 'CQ-DEPEND=%s monkeys' % (cid2,)) - # Validate numeric is allowed. - self._CheckPaladin(git1, cid1, [cid2, '1'], 'CQ-DEPEND=1 %s' % cid2) - # Validate that it unique'ifies the results. - self._CheckPaladin(git1, cid1, ['1'], 'CQ-DEPEND=1 1') - - # Invalid syntax - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], 'CQ-DEPENDS=1') - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], 'CQ_DEPEND=1') - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], ' CQ-DEPEND=1') - self.assertRaises(cros_patch.BrokenCQDepends, self._CheckPaladin, - git1, cid1, [], ' Cq-Depend:1') - def testChangeIdMetadata(self): """Verify Change-Id is set in git metadata.""" git1, git2, _ = self._CommonGitSetup() diff --git a/scripts/gerrit.py b/scripts/gerrit.py index d3790add8..7cf8928fc 100644 --- a/scripts/gerrit.py +++ b/scripts/gerrit.py @@ -432,10 +432,7 @@ class ActionDeps(_ActionSearchQuery): @classmethod def _Children(cls, opts, querier, cl): - """Yields the Gerrit and CQ-Depends dependencies of a patch""" - for change in cls._ProcessDeps( - opts, querier, cl, cl.PaladinDependencies(None), True): - yield change + """Yields the Gerrit dependencies of a patch""" for change in cls._ProcessDeps( opts, querier, cl, cl.GerritDependencies(), False): yield change -- cgit v1.2.3