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

Unified Diff: PRESUBMIT.py

Issue 589123002: Add a PRESUBMIT check that production code does not call test code (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Created 6 years, 3 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 43d6b5b8eec3c4e8585f21a7243487cfcfc164d2..3a9895db8df02806d2af560970041d9b2c60d323 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -34,6 +34,32 @@ for more details about the presubmit API built into gcl.
import sys
+_EXCLUDED_PATHS = (
+ r"^test[\\\/].*",
+ r"^testing[\\\/].*",
+ r"^third_party[\\\/].*",
+ r"^tools[\\\/].*",
+)
+
+
+# Regular expression that matches code only used for test binaries
+# (best effort).
+_TEST_CODE_EXCLUDED_PATHS = (
+ r'.+-unittest\.cc',
+ # Has a method VisitForTest().
Michael Starzinger 2014/09/22 22:25:51 Please turn this into a TODO(mstarzinger), I'll pr
+ r'src[\\\/]compiler[\\\/]ast-graph-builder\.cc',
+ # Test extension.
+ r'src[\\\/]extensions[\\\/]gc-extension\.cc',
+)
+
+
+_TEST_ONLY_WARNING = (
+ 'You might be calling functions intended only for testing from\n'
+ 'production code. It is OK to ignore this warning if you know what\n'
+ 'you are doing, as the heuristics used to detect the situation are\n'
+ 'not perfect. The commit queue will not block on this warning.')
+
+
def _V8PresubmitChecks(input_api, output_api):
"""Runs the V8 presubmit checks."""
import sys
@@ -113,6 +139,49 @@ def _CheckUnwantedDependencies(input_api, output_api):
return results
+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, as the declaration of for-testing functions in
+ # header files are hard to distinguish from calls to such functions without a
+ # proper C++ parser.
+ file_inclusion_pattern = r'.+\.cc'
+
+ base_function_pattern = r'[ :]test::[^\s]+|ForTest(ing)?|for_test(ing)?'
+ inclusion_pattern = input_api.re.compile(r'(%s)\s*\(' % base_function_pattern)
+ comment_pattern = input_api.re.compile(r'//.*(%s)' % base_function_pattern)
+ exclusion_pattern = input_api.re.compile(
+ r'::[A-Za-z0-9_]+(%s)|(%s)[^;]+\{' % (
+ base_function_pattern, base_function_pattern))
+
+ def FilterFile(affected_file):
+ black_list = (_EXCLUDED_PATHS +
+ _TEST_CODE_EXCLUDED_PATHS +
+ input_api.DEFAULT_BLACK_LIST)
+ return input_api.FilterSourceFile(
+ affected_file,
+ white_list=(file_inclusion_pattern, ),
+ black_list=black_list)
+
+ problems = []
+ for f in input_api.AffectedSourceFiles(FilterFile):
+ local_path = f.LocalPath()
+ for line_number, line in f.ChangedContents():
+ if (inclusion_pattern.search(line) and
+ not comment_pattern.search(line) and
+ not exclusion_pattern.search(line)):
+ problems.append(
+ '%s:%d\n %s' % (local_path, line_number, line.strip()))
+
+ if problems:
+ return [output_api.PresubmitPromptOrNotify(_TEST_ONLY_WARNING, problems)]
+ else:
+ return []
+
+
def _CommonChecks(input_api, output_api):
"""Checks common to both upload and commit."""
results = []
@@ -122,6 +191,8 @@ def _CommonChecks(input_api, output_api):
input_api, output_api))
results.extend(_V8PresubmitChecks(input_api, output_api))
results.extend(_CheckUnwantedDependencies(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