zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

PRESUBMIT: Enforce tracker prefix for all BUG entries

Browse Source 
Changed function definition from private to public. This was needed to test the function and to maintain the consistency.

BUG=webrtc:8197
NOTRY=True
R=kjellander@webrtc.org

Review-Url: https://codereview.webrtc.org/3010153002 .
Cr-Commit-Position: refs/heads/master@{#19831}
This commit is contained in:
charujain
2017-09-14 13:33:22 +02:00
parent 66ca7e3b06
commit 9893e253f9
4 changed files with 162 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
DEPS
View File

@ -24,7 +24,7 @@ vars = {
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling catapult
# and whatever else without interference from each other.
'catapult_revision': '49fbcfa16b5d148a595cc50036d5e8354739f13f',
'catapult_revision': '596aa47eec509819730749df6e0a848d374265d6',
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling libFuzzer
# and whatever else without interference from each other.
@ -34,7 +34,7 @@ deps = {
# TODO(kjellander): Move this to be Android-only once the libevent dependency
# in base/third_party/libevent is solved.
'src/base':
Var('chromium_git') + '/chromium/src/base' + '@' + '5d1c21aaf0929fe090100a32087e2b355ef6ef86',
Var('chromium_git') + '/chromium/src/base' + '@' + '058098bf6b3e9181d1d28d356d8d7118302f6d5e',
'src/build':
Var('chromium_git') + '/chromium/src/build' + '@' + '465f7fab2721803cf51852375f0f64a62e99bcf8',
'src/buildtools':
@ -42,7 +42,7 @@ deps = {
'src/testing':
Var('chromium_git') + '/chromium/src/testing' + '@' + '57bea64dc9f9ea43c639dc5ace2a22c6cb3dd2c2',
'src/third_party':
Var('chromium_git') + '/chromium/src/third_party' + '@' + '766258b4a329f04c358b091336591652ebb22849',
Var('chromium_git') + '/chromium/src/third_party' + '@' + 'b1265e559b7f22be328d29d9dc1aa22de37ebd60',
'src/third_party/boringssl/src':
Var('boringssl_git') + '/boringssl.git' + '@' + Var('boringssl_revision'),
'src/third_party/catapult':
@ -77,7 +77,7 @@ deps = {
'src/third_party/yasm/source/patched-yasm':
Var('chromium_git') + '/chromium/deps/yasm/patched-yasm.git' + '@' + 'b98114e18d8b9b84586b10d24353ab8616d4c5fc',
'src/tools':
Var('chromium_git') + '/chromium/src/tools' + '@' + 'e2203ba97907eeb490035c7a271c114f793c3031',
Var('chromium_git') + '/chromium/src/tools' + '@' + '87be0ad4d04f0c57d1266ecdc3658953162cc556',
'src/tools/gyp':
Var('chromium_git') + '/external/gyp.git' + '@' + 'd61a9397e668fa9843c4aa7da9e79460fe590bfb',
'src/tools/swarming_client':
@ -88,7 +88,7 @@ deps = {
'src/third_party/gflags/src':
Var('chromium_git') + '/external/github.com/gflags/gflags' + '@' + '03bebcb065c83beff83d50ae025a55a4bf94dfca',
'src/third_party/gtest-parallel':
Var('chromium_git') + '/external/github.com/google/gtest-parallel' + '@' + '76767784389ed944027630759723b7d142cf2a5e',
Var('chromium_git') + '/external/github.com/google/gtest-parallel' + '@' + 'de7390a463e030bf22747d962ddc3ea0592f2c18',
}
deps_os = {
'android': {
@ -119,7 +119,7 @@ deps_os = {
},
'ios': {
'src/ios':
Var('chromium_git') + '/chromium/src/ios' + '@' + '26552e2cb601390d0b6d25d0081857b288c27723',
Var('chromium_git') + '/chromium/src/ios' + '@' + '50b48ba029cebee340341eda248939136dfa890f',
},
'unix': {
'src/third_party/lss':

107
PRESUBMIT.py
View File

@ -106,7 +106,7 @@ def _RunCommand(command, cwd):
return p.returncode, stdout, stderr
def _VerifyNativeApiHeadersListIsValid(input_api, output_api):
def VerifyNativeApiHeadersListIsValid(input_api, output_api):
"""Ensures the list of native API header directories is up to date."""
non_existing_paths = []
native_api_full_paths = [
@ -142,7 +142,7 @@ You seem to be changing native API header files. Please make sure that you:
Related files:
"""
def _CheckNativeApiHeaderChanges(input_api, output_api):
def CheckNativeApiHeaderChanges(input_api, output_api):
"""Checks to remind proper changing of native APIs."""
files = []
for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
@ -156,7 +156,7 @@ def _CheckNativeApiHeaderChanges(input_api, output_api):
return []
def _CheckNoIOStreamInHeaders(input_api, output_api):
def CheckNoIOStreamInHeaders(input_api, output_api):
"""Checks to make sure no .h files include <iostream>."""
files = []
pattern = input_api.re.compile(r'^#include\s*<iostream>',
@ -177,7 +177,7 @@ def _CheckNoIOStreamInHeaders(input_api, output_api):
return []
def _CheckNoPragmaOnce(input_api, output_api):
def CheckNoPragmaOnce(input_api, output_api):
"""Make sure that banned functions are not used."""
files = []
pattern = input_api.re.compile(r'^#pragma\s+once',
@ -197,7 +197,7 @@ def _CheckNoPragmaOnce(input_api, output_api):
return []
def _CheckNoFRIEND_TEST(input_api, output_api): # pylint: disable=invalid-name
def CheckNoFRIEND_TEST(input_api, output_api): # pylint: disable=invalid-name
"""Make sure that gtest's FRIEND_TEST() macro is not used, the
FRIEND_TEST_ALL_PREFIXES() macro from testsupport/gtest_prod_util.h should be
used instead since that allows for FLAKY_, FAILS_ and DISABLED_ prefixes."""
@ -216,7 +216,7 @@ def _CheckNoFRIEND_TEST(input_api, output_api): # pylint: disable=invalid-name
'use FRIEND_TEST_ALL_PREFIXES() instead.\n' + '\n'.join(problems))]
def _IsLintBlacklisted(blacklist_paths, file_path):
def IsLintBlacklisted(blacklist_paths, file_path):
""" Checks if a file is blacklisted for lint check."""
for path in blacklist_paths:
if file_path == path or os.path.dirname(file_path).startswith(path):
@ -224,10 +224,10 @@ def _IsLintBlacklisted(blacklist_paths, file_path):
return False
def _CheckApprovedFilesLintClean(input_api, output_api,
def CheckApprovedFilesLintClean(input_api, output_api,
source_file_filter=None):
"""Checks that all new or non-blacklisted .cc and .h files pass cpplint.py.
This check is based on _CheckChangeLintsClean in
This check is based on CheckChangeLintsClean in
depot_tools/presubmit_canned_checks.py but has less filters and only checks
added files."""
result = []
@ -254,7 +254,7 @@ def _CheckApprovedFilesLintClean(input_api, output_api,
files = []
for f in input_api.AffectedSourceFiles(source_file_filter):
# Note that moved/renamed files also count as added.
if f.Action() == 'A' or not _IsLintBlacklisted(blacklist_paths,
if f.Action() == 'A' or not IsLintBlacklisted(blacklist_paths,
f.LocalPath()):
files.append(f.AbsoluteLocalPath())
@ -270,7 +270,7 @@ def _CheckApprovedFilesLintClean(input_api, output_api,
return result
def _CheckNoSourcesAbove(input_api, gn_files, output_api):
def CheckNoSourcesAbove(input_api, gn_files, output_api):
# Disallow referencing source files with paths above the GN file location.
source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
re.MULTILINE | re.DOTALL)
@ -298,7 +298,7 @@ def _CheckNoSourcesAbove(input_api, gn_files, output_api):
items=violating_gn_files)]
return []
def _CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
def CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
# Disallow mixing .c and .cc source files in the same target.
source_pattern = input_api.re.compile(r' +sources \+?= \[(.*?)\]',
re.MULTILINE | re.DOTALL)
@ -327,7 +327,7 @@ def _CheckNoMixingCAndCCSources(input_api, gn_files, output_api):
items=violating_gn_files.keys())]
return []
def _CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
def CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
cwd = input_api.PresubmitLocalPath()
script_path = os.path.join('tools_webrtc', 'presubmit_checks_lib',
'check_package_boundaries.py')
@ -341,7 +341,7 @@ def _CheckNoPackageBoundaryViolations(input_api, gn_files, output_api):
'%s' % stderr)]
return []
def _CheckGnChanges(input_api, output_api):
def CheckGnChanges(input_api, output_api):
source_file_filter = lambda x: input_api.FilterSourceFile(
x, white_list=(r'.+\.(gn|gni)$',))
@ -352,13 +352,13 @@ def _CheckGnChanges(input_api, output_api):
result = []
if gn_files:
result.extend(_CheckNoSourcesAbove(input_api, gn_files, output_api))
result.extend(_CheckNoMixingCAndCCSources(input_api, gn_files, output_api))
result.extend(_CheckNoPackageBoundaryViolations(
result.extend(CheckNoSourcesAbove(input_api, gn_files, output_api))
result.extend(CheckNoMixingCAndCCSources(input_api, gn_files, output_api))
result.extend(CheckNoPackageBoundaryViolations(
input_api, gn_files, output_api))
return result
def _CheckUnwantedDependencies(input_api, output_api):
def CheckUnwantedDependencies(input_api, output_api):
"""Runs checkdeps on #include statements added in this
change. Breaking - rules is an error, breaking ! rules is a
warning.
@ -422,7 +422,32 @@ def _CheckUnwantedDependencies(input_api, output_api):
warning_descriptions))
return results
def _CheckChangeHasBugField(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 '
'issue number, separated by a colon, e.g. webrtc:123 or chromium:12345.')
results = []
for bug in (input_api.change.BUG or '').split(','):
bug = bug.strip()
if bug.lower() == 'none':
continue
if ':' not in bug:
try:
if int(bug) > 100000:
# Rough indicator for current chromium bugs.
prefix_guess = 'chromium'
else:
prefix_guess = 'webrtc'
results.append('BUG entry requires issue tracker prefix, e.g. %s:%s' %
(prefix_guess, bug))
except ValueError:
results.append(bogus_bug_msg % bug)
elif not re.match(r'\w+:\d+', bug):
results.append(bogus_bug_msg % bug)
return [output_api.PresubmitError(r) for r in results]
def CheckChangeHasBugField(input_api, output_api):
"""Requires that the changelist have a BUG= field.
This check is stricter than the one in depot_tools/presubmit_canned_checks.py
@ -438,7 +463,7 @@ def _CheckChangeHasBugField(input_api, output_api):
' * 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):
def CheckJSONParseErrors(input_api, output_api):
"""Check that JSON files do not contain syntax errors."""
def FilterFile(affected_file):
@ -463,11 +488,12 @@ def _CheckJSONParseErrors(input_api, output_api):
return results
def _RunPythonTests(input_api, output_api):
def RunPythonTests(input_api, output_api):
def Join(*args):
return input_api.os_path.join(input_api.PresubmitLocalPath(), *args)
test_directories = [
'/',
Join('webrtc', 'rtc_tools', 'py_event_log_analyzer'),
Join('webrtc', 'rtc_tools'),
Join('webrtc', 'audio', 'test', 'unittests'),
@ -487,7 +513,7 @@ def _RunPythonTests(input_api, output_api):
return input_api.RunTests(tests, parallel=True)
def _CheckUsageOfGoogleProtobufNamespace(input_api, output_api):
def CheckUsageOfGoogleProtobufNamespace(input_api, output_api):
"""Checks that the namespace google::protobuf has not been used."""
files = []
pattern = input_api.re.compile(r'google::protobuf')
@ -507,7 +533,7 @@ def _CheckUsageOfGoogleProtobufNamespace(input_api, output_api):
return []
def _CommonChecks(input_api, output_api):
def CommonChecks(input_api, output_api):
"""Checks common to both upload and commit."""
results = []
# Filter out files that are in objc or ios dirs from being cpplint-ed since
@ -518,7 +544,7 @@ def _CommonChecks(input_api, output_api):
r"webrtc\/build\/ios\/SDK\/.*",
)
source_file_filter = lambda x: input_api.FilterSourceFile(x, None, black_list)
results.extend(_CheckApprovedFilesLintClean(
results.extend(CheckApprovedFilesLintClean(
input_api, output_api, source_file_filter))
results.extend(input_api.canned_checks.RunPylint(input_api, output_api,
black_list=(r'^base[\\\/].*\.py$',
@ -564,23 +590,23 @@ def _CommonChecks(input_api, output_api):
input_api, output_api))
results.extend(input_api.canned_checks.CheckChangeTodoHasOwner(
input_api, output_api))
results.extend(_CheckNativeApiHeaderChanges(input_api, output_api))
results.extend(_CheckNoIOStreamInHeaders(input_api, output_api))
results.extend(_CheckNoPragmaOnce(input_api, output_api))
results.extend(_CheckNoFRIEND_TEST(input_api, output_api))
results.extend(_CheckGnChanges(input_api, output_api))
results.extend(_CheckUnwantedDependencies(input_api, output_api))
results.extend(_CheckJSONParseErrors(input_api, output_api))
results.extend(_RunPythonTests(input_api, output_api))
results.extend(_CheckUsageOfGoogleProtobufNamespace(input_api, output_api))
results.extend(_CheckOrphanHeaders(input_api, output_api))
results.extend(_CheckNewLineAtTheEndOfProtoFiles(input_api, output_api))
results.extend(CheckNativeApiHeaderChanges(input_api, output_api))
results.extend(CheckNoIOStreamInHeaders(input_api, output_api))
results.extend(CheckNoPragmaOnce(input_api, output_api))
results.extend(CheckNoFRIEND_TEST(input_api, output_api))
results.extend(CheckGnChanges(input_api, output_api))
results.extend(CheckUnwantedDependencies(input_api, output_api))
results.extend(CheckJSONParseErrors(input_api, output_api))
results.extend(RunPythonTests(input_api, output_api))
results.extend(CheckUsageOfGoogleProtobufNamespace(input_api, output_api))
results.extend(CheckOrphanHeaders(input_api, output_api))
results.extend(CheckNewLineAtTheEndOfProtoFiles(input_api, output_api))
return results
def CheckChangeOnUpload(input_api, output_api):
results = []
results.extend(_CommonChecks(input_api, output_api))
results.extend(CommonChecks(input_api, output_api))
results.extend(
input_api.canned_checks.CheckGNFormatted(input_api, output_api))
return results
@ -588,21 +614,22 @@ def CheckChangeOnUpload(input_api, output_api):
def CheckChangeOnCommit(input_api, output_api):
results = []
results.extend(_CommonChecks(input_api, output_api))
results.extend(_VerifyNativeApiHeadersListIsValid(input_api, output_api))
results.extend(CommonChecks(input_api, output_api))
results.extend(VerifyNativeApiHeadersListIsValid(input_api, output_api))
results.extend(input_api.canned_checks.CheckOwners(input_api, output_api))
results.extend(input_api.canned_checks.CheckChangeWasUploaded(
input_api, output_api))
results.extend(input_api.canned_checks.CheckChangeHasDescription(
input_api, output_api))
results.extend(_CheckChangeHasBugField(input_api, output_api))
results.extend(CheckChangeHasBugField(input_api, output_api))
results.extend(CheckCommitMessageBugEntry(input_api, output_api))
results.extend(input_api.canned_checks.CheckTreeIsOpen(
input_api, output_api,
json_url='http://webrtc-status.appspot.com/current?format=json'))
return results
def _CheckOrphanHeaders(input_api, output_api):
def CheckOrphanHeaders(input_api, output_api):
# We need to wait until we have an input_api object and use this
# roundabout construct to import prebubmit_checks_lib because this file is
# eval-ed and thus doesn't have __file__.
@ -632,7 +659,7 @@ def _CheckOrphanHeaders(input_api, output_api):
return results
def _CheckNewLineAtTheEndOfProtoFiles(input_api, output_api):
def CheckNewLineAtTheEndOfProtoFiles(input_api, output_api):
"""Checks that all .proto files are terminated with a newline."""
error_msg = 'File {} must end with exactly one newline.'
results = []

39
presubmit_test.py Normal file
View File

@ -0,0 +1,39 @@
import unittest
import PRESUBMIT
from presubmit_test_mocks import MockInputApi, MockOutputApi
class CheckBugEntryField(unittest.TestCase):
def testCommitMessageBugEntryWithNoError(self):
mock_input_api = MockInputApi()
mock_output_api = MockOutputApi()
mock_input_api.change.BUG = 'webrtc:1234'
errors = PRESUBMIT.CheckCommitMessageBugEntry(mock_input_api,
mock_output_api)
self.assertEqual(0, len(errors))
def testCommitMessageBugEntryReturnError(self):
mock_input_api = MockInputApi()
mock_output_api = MockOutputApi()
mock_input_api.change.BUG = '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'
' the issue tracker prefix and the issue number,'
' separated by a colon, e.g. webrtc:123 or'
' chromium:12345.'), str(errors[0]))
def testCommitMessageBugEntryIsNone(self):
mock_input_api = MockInputApi()
mock_output_api = MockOutputApi()
mock_input_api.change.BUG = 'None'
errors = PRESUBMIT.CheckCommitMessageBugEntry(mock_input_api,
mock_output_api)
self.assertEqual(0, len(errors))
if __name__ == '__main__':
unittest.main()

50
presubmit_test_mocks.py Normal file
View File

@ -0,0 +1,50 @@
# Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
#
# Use of this source code is governed by a BSD-style license
# that can be found in the LICENSE file in the root of the source
# tree. An additional intellectual property rights grant can be found
# in the file PATENTS. All contributing project authors may
# be found in the AUTHORS file in the root of the source tree.
class MockInputApi(object):
"""Mock class for the InputApi class.
This class can be used for unittests for presubmit by initializing the files
attribute as the list of changed files.
"""
def __init__(self):
self.change = MockChange([])
class MockOutputApi(object):
"""Mock class for the OutputApi class.
An instance of this class can be passed to presubmit unittests for outputing
various types of results.
"""
class PresubmitResult(object):
def __init__(self, message, items=None, long_text=''):
self.message = message
self.items = items
self.long_text = long_text
def __repr__(self):
return self.message
class PresubmitError(PresubmitResult):
def __init__(self, message, items=None, long_text=''):
MockOutputApi.PresubmitResult.__init__(self, message, items, long_text)
self.type = 'error'
class MockChange(object):
"""Mock class for Change class.
This class can be used in presubmit unittests to mock the query of the
current change.
"""
def __init__(self, changed_files):
self._changed_files = changed_files