Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(267)

Unified Diff: PRESUBMIT.py

Issue 7582002: Add check to help prevent test-only function usage in production code. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: PRESUBMIT.py
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index b11ed8d40d31f70cfa535ce9e0e80d3ea7c3599d..8dc4cc876362cc2d4536f6ff6e4d19ff08f566c1 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -38,6 +38,51 @@ def _CheckNoInterfacesInBase(input_api, output_api):
return []
+def _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api):
+ """Attempts to prevent use of functions intended only for testing in
+ non-testing code. For now this is just a best-effort implementation
+ that ignores header files and may have some false positives. A
+ better implementation would probably need a proper C++ parser.
+ """
+ # We only scan .cc files and the like, as the declaration of
+ # for-testing functions in header files are hard to distinguish from
+ # calls to such functions without a proper C++ parser.
+ source_extensions = r'\.(cc|cpp|cxx|mm)$'
+ file_inclusion_pattern = input_api.re.compile(source_extensions)
+ file_exclusion_pattern = input_api.re.compile(
+ r'(_test_support|_(unit|browser|ui|perf)test)%s' % source_extensions)
+
+ base_function_pattern = r'ForTest(ing)?|for_test(ing)?'
+ inclusion_pattern = input_api.re.compile(r'(%s)\s*\(' % base_function_pattern)
+ exclusion_pattern = input_api.re.compile(
+ r'::[A-Za-z0-9_]+(%s)|(%s)[^;]+\{' % (
+ base_function_pattern, base_function_pattern))
+
+ problems = []
+ for f in input_api.AffectedSourceFiles(input_api.FilterSourceFile):
+ local_path = f.LocalPath()
+ if (file_inclusion_pattern.search(local_path) and
+ not file_exclusion_pattern.search(local_path)):
M-A Ruel 2011/08/05 18:27:41 It should use a source filter instead and you need
Jói 2011/08/08 23:11:00 Done.
+ lines = input_api.ReadFile(f).split('\n')
M-A Ruel 2011/08/05 18:27:41 splitlines()
Jói 2011/08/08 23:11:00 Done.
+ line_number = 0
+ for line in lines:
+ if (inclusion_pattern.search(line) and
+ not exclusion_pattern.search(line)):
+ problems.append(
+ '%s:%d\n %s' % (local_path, line_number, line.strip()))
+ line_number += 1
+
+ if problems:
+ return [output_api.PresubmitPromptWarning(
+ 'You might be calling functions intended only for testing from\n' +
M-A Ruel 2011/08/05 18:27:41 no need for +
Jói 2011/08/08 23:11:00 Done.
+ 'production code. Please verify that the following usages are OK,\n' +
+ 'and email joi@chromium.org if this presubmit check needs to be\n' +
+ 'improved because of frequent false positives:',
+ problems)]
+ else:
+ return []
+
+
def _CommonChecks(input_api, output_api):
"""Checks common to both upload and commit."""
results = []
@@ -45,6 +90,8 @@ def _CommonChecks(input_api, output_api):
input_api, output_api, excluded_paths=_EXCLUDED_PATHS))
results.extend(_CheckNoInterfacesInBase(input_api, output_api))
results.extend(_CheckAuthorizedAuthor(input_api, output_api))
+ results.extend(
+ _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
return results
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698