From aba677688945aa43c0db99d9f1e866cde609b5c3 Mon Sep 17 00:00:00 2001 From: "pam@chromium.org" Date: Thu, 15 Mar 2012 12:57:44 +0000 Subject: [PATCH] Add a message that the list of directories needing owners may not be accurate on first upload, because we have no way to know whether the user is an OWNER for any of them. BUG=117974 TEST=covered by presubmit unit tests Review URL: http://codereview.chromium.org/9692039 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@126894 0039d316-1c4b-4281-b951-d872f2087c98 --- presubmit_canned_checks.py | 10 +++++----- tests/presubmit_unittest.py | 18 ++++++++++++++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 271753611e..b033f6257b 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -752,19 +752,19 @@ def CheckOwners(input_api, output_api, source_file_filter=None): approval_needed=input_api.is_committing) if owner_email: + message = '' reviewers_plus_owner = reviewers.union(set([owner_email])) - elif input_api.is_committing: - return [output_api.PresubmitWarning( - 'The issue was not uploaded so you have no OWNER approval.')] else: + message = ('\nUntil the issue is uploaded, this list will include ' + 'directories for which you \nare an OWNER.') owner_email = '' reviewers_plus_owner = set() missing_directories = owners_db.directories_not_covered_by(affected_files, reviewers_plus_owner) if missing_directories: - return [output('Missing %s for files in these directories:\n %s' % - (needed, '\n '.join(missing_directories)))] + return [output('Missing %s for files in these directories:\n %s%s' % + (needed, '\n '.join(missing_directories), message))] if input_api.is_committing and not reviewers: return [output('Missing LGTM from someone other than %s' % owner_email)] diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 6c85ac95f4..6559d572d5 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -2250,14 +2250,28 @@ class CannedChecksUnittest(PresubmitTestsBase): rietveld_response=response, expected_output='') - def testCannedCheckOwners_NoIssue(self): + def testCannedCheckOwners_NoIssueNoFiles(self): self.AssertOwnersWorks(issue=None, expected_output="OWNERS check failed: this change has no Rietveld " "issue number, so we can't check it for approvals.\n") - self.AssertOwnersWorks(issue=None, is_committing=False, expected_output="") + def testCannedCheckOwners_NoIssue(self): + self.AssertOwnersWorks(issue=None, + uncovered_dirs=set(['foo']), + expected_output="OWNERS check failed: this change has no Rietveld " + "issue number, so we can't check it for approvals.\n") + self.AssertOwnersWorks(issue=None, + is_committing=False, + uncovered_dirs=set(['foo']), + expected_output='Missing OWNER reviewers for files in these ' + 'directories:\n' + ' foo\n' + 'Until the issue is uploaded, this list will include ' + 'directories for which you \n' + 'are an OWNER.\n') + def testCannedCheckOwners_NoLGTM(self): self.AssertOwnersWorks(expected_output='Missing LGTM from someone ' 'other than john@example.com\n')