diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 4232a5915..f1f61834f 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -827,8 +827,11 @@ def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings, def CheckOwners(input_api, output_api, source_file_filter=None): + affected_files = set([f.LocalPath() for f in + input_api.change.AffectedFiles(file_filter=source_file_filter)]) + if input_api.is_committing: - if input_api.tbr: + if input_api.tbr and not any(['OWNERS' in name for name in affected_files]): return [output_api.PresubmitNotifyResult( '--tbr was specified, skipping OWNERS check')] needed = 'LGTM from an OWNER' @@ -846,9 +849,6 @@ def CheckOwners(input_api, output_api, source_file_filter=None): needed = 'OWNER reviewers' output_fn = output_api.PresubmitNotifyResult - affected_files = set([f.LocalPath() for f in - input_api.change.AffectedFiles(file_filter=source_file_filter)]) - owners_db = input_api.owners_db owners_db.override_files = input_api.change.OriginalOwnersFiles() owner_email, reviewers = GetCodereviewOwnerAndReviewers( diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 9c10e94a6..e3894db7b 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2398,9 +2398,9 @@ class CannedChecksUnittest(PresubmitTestsBase): presubmit.OutputApi.PresubmitNotifyResult) def AssertOwnersWorks(self, tbr=False, issue='1', approvers=None, - reviewers=None, is_committing=True, - response=None, uncovered_files=None, expected_output='', - manually_specified_reviewers=None, dry_run=None): + reviewers=None, is_committing=True, response=None, uncovered_files=None, + expected_output='', manually_specified_reviewers=None, dry_run=None, + modified_file='foo/xyz.cc'): if approvers is None: # The set of people who lgtm'ed a change. approvers = set() @@ -2438,9 +2438,9 @@ class CannedChecksUnittest(PresubmitTestsBase): input_api.tbr = tbr input_api.dry_run = dry_run - if not is_committing or (not tbr and issue): - affected_file.LocalPath().AndReturn('foo/xyz.cc') - change.AffectedFiles(file_filter=None).AndReturn([affected_file]) + affected_file.LocalPath().AndReturn(modified_file) + change.AffectedFiles(file_filter=None).AndReturn([affected_file]) + if not is_committing or (not tbr and issue) or ('OWNERS' in modified_file): change.OriginalOwnersFiles().AndReturn({}) if issue and not response: response = { @@ -2721,6 +2721,13 @@ class CannedChecksUnittest(PresubmitTestsBase): expected_output='--tbr was specified, skipping OWNERS check\n') self.AssertOwnersWorks(tbr=True, is_committing=False, expected_output='') + def testCannedCheckOwners_TBROWNERSFile(self): + self.AssertOwnersWorks( + tbr=True, uncovered_files=set(['foo']), + modified_file='foo/OWNERS', + expected_output='Missing LGTM from an OWNER for these files:\n' + ' foo\n') + def testCannedCheckOwners_WithoutOwnerLGTM(self): self.AssertOwnersWorks(uncovered_files=set(['foo']), expected_output='Missing LGTM from an OWNER for these files:\n'