diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index e4d5e63cb..0ab1c51e5 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -1121,7 +1121,8 @@ def CheckOwnersFormat(input_api, output_api): 'Error parsing OWNERS files:\n%s' % e)] -def CheckOwners(input_api, output_api, source_file_filter=None): +def CheckOwners( + input_api, output_api, source_file_filter=None, allow_tbr=True): 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 @@ -1134,6 +1135,9 @@ def CheckOwners(input_api, output_api, source_file_filter=None): 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 + affected_files = set([f.LocalPath() for f in input_api.change.AffectedFiles(file_filter=source_file_filter)]) owners_db = input_api.owners_db @@ -1159,7 +1163,7 @@ def CheckOwners(input_api, output_api, source_file_filter=None): affects_owners = any('OWNERS' in name for name in missing_files) if input_api.is_committing: - if input_api.tbr and not affects_owners: + if tbr and not affects_owners: return [output_api.PresubmitNotifyResult( '--tbr was specified, skipping OWNERS check')] needed = 'LGTM from an OWNER' @@ -1181,7 +1185,7 @@ def CheckOwners(input_api, output_api, source_file_filter=None): output_list = [ output_fn('Missing %s for these files:\n %s' % (needed, '\n '.join(sorted(missing_files))))] - if input_api.tbr and affects_owners: + if tbr and affects_owners: output_list.append(output_fn('TBR for OWNERS files are ignored.')) if not input_api.is_committing: suggested_owners = owners_db.reviewers_for(missing_files, owner_email) diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 7e9659084..2bfc80991 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2672,7 +2672,7 @@ the current line as well! reviewers=None, is_committing=True, response=None, uncovered_files=None, expected_output='', manually_specified_reviewers=None, dry_run=None, - modified_file='foo/xyz.cc'): + modified_file='foo/xyz.cc', allow_tbr=True): if approvers is None: # The set of people who lgtm'ed a change. approvers = set() @@ -2745,7 +2745,7 @@ the current line as well! fake_db.reviewers_for.return_value = change.author_email results = presubmit_canned_checks.CheckOwners( - input_api, presubmit.OutputApi) + input_api, presubmit.OutputApi, allow_tbr=allow_tbr) for result in results: result.handle() if expected_output: @@ -3049,6 +3049,18 @@ the current line as well! expected_output='--tbr was specified, skipping OWNERS check\n') self.AssertOwnersWorks(tbr=True, is_committing=False, expected_output='') + def testCannedCheckOwners_TBRIgnored(self): + self.AssertOwnersWorks( + tbr=True, + allow_tbr=False, + expected_output='Missing LGTM from someone other than ' + 'john@example.com\n') + self.AssertOwnersWorks( + tbr=True, + allow_tbr=False, + is_committing=False, + expected_output='') + def testCannedCheckOwners_TBROWNERSFile(self): self.AssertOwnersWorks( tbr=True,