From e7c058174054c97f05a1c4b009f5226bee5983e5 Mon Sep 17 00:00:00 2001 From: Dirk Pranke Date: Fri, 19 Mar 2021 02:05:14 +0000 Subject: [PATCH] Revert "presubmit: Don't skip OWNERS check when Bot-Commit+1 is present." This reverts commit 9757ad58837498ee2d8e4a33deb2c91af19d6cf1. Reason for revert: We need to update callers first (e.g., //PRESUBMIT.py in chromium/src). Original change's description: > presubmit: Don't skip OWNERS check when Bot-Commit+1 is present. > > Change-Id: I17b07796a86c5214e13a0058428889c1bb2b850d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2774080 > Auto-Submit: Edward Lesmes > Reviewed-by: Jason Clinton > Commit-Queue: Edward Lesmes Change-Id: I02c3d5ea2e65ef852d34a6816d04fe1cad82823a No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2775101 Auto-Submit: Dirk Pranke Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper --- presubmit_canned_checks.py | 17 +++++++++++------ presubmit_support.py | 3 +++ tests/presubmit_unittest.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index d4714e5d4..16c149765 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1127,12 +1127,17 @@ def CheckOwnersFormat(input_api, output_api): def CheckOwners( input_api, output_api, source_file_filter=None, allow_tbr=True): - # Skip OWNERS check when Owners-Override label is approved. This is intended - # for global owners, trusted bots, and on-call sheriffs. Review is still - # required for these changes. - if (input_api.change.issue - and input_api.gerrit.IsOwnersOverrideApproved(input_api.change.issue)): - return [] + if input_api.change.issue: + # Skip OWNERS check when Bot-Commit label is approved. This label is + # intended for commits made by trusted bots that don't require review nor + # owners approval. + if input_api.gerrit.IsBotCommitApproved(input_api.change.issue): + return [] + # Skip OWNERS check when Owners-Override label is approved. This is intended + # for global owners, trusted bots, and on-call sheriffs. Review is still + # required for these changes. + if input_api.gerrit.IsOwnersOverrideApproved(input_api.change.issue): + return [] # Ignore tbr if not allowed for this repo. tbr = input_api.tbr and allow_tbr diff --git a/presubmit_support.py b/presubmit_support.py index c53ed8fa1..3a05c4b8e 100755 --- a/presubmit_support.py +++ b/presubmit_support.py @@ -446,6 +446,9 @@ class GerritAccessor(object): return [v for v in label_info.get('all', []) if v.get('value', 0) == max_value] + def IsBotCommitApproved(self, issue): + return bool(self._GetApproversForLabel(issue, 'Bot-Commit')) + def IsOwnersOverrideApproved(self, issue): return bool(self._GetApproversForLabel(issue, 'Owners-Override')) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 739a2e339..fc4e1df40 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2795,6 +2795,37 @@ the current line as well! response=response, expected_output='') + def testCannedCheckOwners_BotCommit(self): + response = { + "owner": {"email": "john@example.com"}, + "labels": {"Bot-Commit": { + u'all': [ + { + u'email': u'bot@example.com', + u'value': 1 + }, + ], + u'approved': {u'email': u'bot@example.org'}, + u'default_value': 0, + u'values': {u' 0': u'No score', + u'+1': u'Looks good to me'}, + }}, + "reviewers": {"REVIEWER": [{u'email': u'bot@example.com'}]}, + } + self.AssertOwnersWorks( + approvers=set(), + modified_files={'foo/xyz.cc': ['john@example.com']}, + response=response, + is_committing=True, + expected_output='') + + self.AssertOwnersWorks( + approvers=set(), + modified_files={'foo/xyz.cc': ['john@example.com']}, + is_committing=False, + response=response, + expected_output='') + def testCannedCheckOwners_OwnersOverride(self): response = { "owner": {"email": "john@example.com"},