Using Change.BugsFromDescription to read CL bugs in PRESUBMIT checks.
Since we migrated to Gerrit recently we cannot rely on the BUG= format for this check because: * it is deprecated: https://cs.chromium.org/chromium/tools/depot_tools/presubmit_support.py?l=908&rcl=94652a37677488738626b96ff504fc07afbbaa87 * it causes confusion in our users because Gerrit uses Bug: and all the error messages were requiring BUG= This CL uses a more general API to get the list of bugs from in a CL and renames BUG= to Bug:. Bug: None Change-Id: I7e86fe6d8ca426d9e4bf3bd39021d2a510ec196f No-Treechecks: True No-Try: True Reviewed-on: https://webrtc-review.googlesource.com/8881 Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Reviewed-by: Henrik Kjellander <kjellander@webrtc.org> Cr-Commit-Position: refs/heads/master@{#20260}
This commit is contained in:

committed by
Commit Bot

parent
69cc48bb8b
commit
6188018e1d
21
PRESUBMIT.py
21
PRESUBMIT.py
@ -484,10 +484,10 @@ def CheckUnwantedDependencies(input_api, output_api):
|
||||
def CheckCommitMessageBugEntry(input_api, output_api):
|
||||
"""Check that bug entries are well-formed in commit message."""
|
||||
bogus_bug_msg = (
|
||||
'Bogus BUG entry: %s. Please specify the issue tracker prefix and the '
|
||||
'Bogus Bug entry: %s. Please specify the issue tracker prefix and the '
|
||||
'issue number, separated by a colon, e.g. webrtc:123 or chromium:12345.')
|
||||
results = []
|
||||
for bug in (input_api.change.BUG or '').split(','):
|
||||
for bug in input_api.change.BugsFromDescription():
|
||||
bug = bug.strip()
|
||||
if bug.lower() == 'none':
|
||||
continue
|
||||
@ -498,7 +498,7 @@ def CheckCommitMessageBugEntry(input_api, output_api):
|
||||
prefix_guess = 'chromium'
|
||||
else:
|
||||
prefix_guess = 'webrtc'
|
||||
results.append('BUG entry requires issue tracker prefix, e.g. %s:%s' %
|
||||
results.append('Bug entry requires issue tracker prefix, e.g. %s:%s' %
|
||||
(prefix_guess, bug))
|
||||
except ValueError:
|
||||
results.append(bogus_bug_msg % bug)
|
||||
@ -507,20 +507,23 @@ def CheckCommitMessageBugEntry(input_api, output_api):
|
||||
return [output_api.PresubmitError(r) for r in results]
|
||||
|
||||
def CheckChangeHasBugField(input_api, output_api):
|
||||
"""Requires that the changelist have a BUG= field.
|
||||
"""Requires that the changelist is associated with a bug.
|
||||
|
||||
This check is stricter than the one in depot_tools/presubmit_canned_checks.py
|
||||
since it fails the presubmit if the BUG= field is missing or doesn't contain
|
||||
since it fails the presubmit if the bug field is missing or doesn't contain
|
||||
a bug reference.
|
||||
|
||||
This supports both 'BUG=' and 'Bug:' since we are in the process of migrating
|
||||
to Gerrit and it encourages the usage of 'Bug:'.
|
||||
"""
|
||||
if input_api.change.BUG:
|
||||
if input_api.change.BugsFromDescription():
|
||||
return []
|
||||
else:
|
||||
return [output_api.PresubmitError(
|
||||
'The BUG=[bug number] field is mandatory. Please create a bug and '
|
||||
'The "Bug: [bug number]" footer is mandatory. Please create a bug and '
|
||||
'reference it using either of:\n'
|
||||
' * https://bugs.webrtc.org - reference it using BUG=webrtc:XXXX\n'
|
||||
' * https://crbug.com - reference it using BUG=chromium:XXXXXX')]
|
||||
' * https://bugs.webrtc.org - reference it using Bug: webrtc:XXXX\n'
|
||||
' * https://crbug.com - reference it using Bug: chromium:XXXXXX')]
|
||||
|
||||
def CheckJSONParseErrors(input_api, output_api):
|
||||
"""Check that JSON files do not contain syntax errors."""
|
||||
|
@ -15,14 +15,15 @@ import textwrap
|
||||
import unittest
|
||||
|
||||
import PRESUBMIT
|
||||
from presubmit_test_mocks import MockInputApi, MockOutputApi, MockFile
|
||||
# pylint: disable=line-too-long
|
||||
from presubmit_test_mocks import MockInputApi, MockOutputApi, MockFile, MockChange
|
||||
|
||||
|
||||
class CheckBugEntryFieldTest(unittest.TestCase):
|
||||
def testCommitMessageBugEntryWithNoError(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.change.BUG = 'webrtc:1234'
|
||||
mock_input_api.change = MockChange([], ['webrtc:1234'])
|
||||
errors = PRESUBMIT.CheckCommitMessageBugEntry(mock_input_api,
|
||||
mock_output_api)
|
||||
self.assertEqual(0, len(errors))
|
||||
@ -30,19 +31,29 @@ class CheckBugEntryFieldTest(unittest.TestCase):
|
||||
def testCommitMessageBugEntryReturnError(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.change.BUG = 'webrtc:1234,webrtc=4321'
|
||||
mock_input_api.change = MockChange([], ['webrtc:1234', 'webrtc=4321'])
|
||||
errors = PRESUBMIT.CheckCommitMessageBugEntry(mock_input_api,
|
||||
mock_output_api)
|
||||
self.assertEqual(1, len(errors))
|
||||
self.assertEqual(('Bogus BUG entry: webrtc=4321. Please specify'
|
||||
self.assertEqual(('Bogus Bug entry: webrtc=4321. Please specify'
|
||||
' the issue tracker prefix and the issue number,'
|
||||
' separated by a colon, e.g. webrtc:123 or'
|
||||
' chromium:12345.'), str(errors[0]))
|
||||
|
||||
def testCommitMessageBugEntryWithoutPrefix(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.change = MockChange([], ['1234'])
|
||||
errors = PRESUBMIT.CheckCommitMessageBugEntry(mock_input_api,
|
||||
mock_output_api)
|
||||
self.assertEqual(1, len(errors))
|
||||
self.assertEqual(('Bug entry requires issue tracker prefix, '
|
||||
'e.g. webrtc:1234'), str(errors[0]))
|
||||
|
||||
def testCommitMessageBugEntryIsNone(self):
|
||||
mock_input_api = MockInputApi()
|
||||
mock_output_api = MockOutputApi()
|
||||
mock_input_api.change.BUG = 'None'
|
||||
mock_input_api.change = MockChange([], ['None'])
|
||||
errors = PRESUBMIT.CheckCommitMessageBugEntry(mock_input_api,
|
||||
mock_output_api)
|
||||
self.assertEqual(0, len(errors))
|
||||
|
@ -18,7 +18,7 @@ class MockInputApi(object):
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
self.change = MockChange([])
|
||||
self.change = MockChange([], [])
|
||||
self.files = []
|
||||
|
||||
def AffectedSourceFiles(self, file_filter=None):
|
||||
@ -64,8 +64,12 @@ class MockChange(object):
|
||||
current change.
|
||||
"""
|
||||
|
||||
def __init__(self, changed_files):
|
||||
def __init__(self, changed_files, bugs_from_description):
|
||||
self._changed_files = changed_files
|
||||
self._bugs_from_description = bugs_from_description
|
||||
|
||||
def BugsFromDescription(self):
|
||||
return self._bugs_from_description
|
||||
|
||||
|
||||
class MockFile(object):
|
||||
|
Reference in New Issue
Block a user