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

Unified Diff: third_party/WebKit/Source/devtools/PRESUBMIT.py

Issue 2538003002: DevTools: bail out presubmit early if error after node.js or formatting checks (Closed)
Patch Set: address CL feedback Created 4 years 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 | « third_party/WebKit/Source/devtools/.gitignore ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/devtools/PRESUBMIT.py
diff --git a/third_party/WebKit/Source/devtools/PRESUBMIT.py b/third_party/WebKit/Source/devtools/PRESUBMIT.py
index c7ebd1a7a94f641115dc8be574b8f9da3427376a..bd9c1116b742389c28fff6da005bf3a35339ae44 100644
--- a/third_party/WebKit/Source/devtools/PRESUBMIT.py
+++ b/third_party/WebKit/Source/devtools/PRESUBMIT.py
@@ -32,10 +32,13 @@ See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
for more details about the presubmit API built into gcl.
"""
+from collections import namedtuple
import sys
compile_note = "Be sure to run your patch by the compile_frontend.py script prior to committing!"
+CheckOutput = namedtuple('CheckOutput', ['results', 'has_errors'])
+
def _CheckNodeAndNPMModules(input_api, output_api):
node_script_path = input_api.os_path.join(input_api.PresubmitLocalPath(), "scripts", "install_node_deps.py")
@@ -45,13 +48,14 @@ def _CheckNodeAndNPMModules(input_api, output_api):
stderr=input_api.subprocess.STDOUT)
out, _ = process.communicate()
if process.returncode != 0:
- return [output_api.PresubmitError(out)]
- return [output_api.PresubmitNotifyResult(out)]
+ return CheckOutput([output_api.PresubmitError(out)], has_errors=True)
+ return CheckOutput([output_api.PresubmitNotifyResult(out)], has_errors=False)
+
def _FormatDevtools(input_api, output_api):
affected_front_end_files = _getAffectedFrontEndFiles(input_api)
if len(affected_front_end_files) == 0:
- return []
+ return CheckOutput([], has_errors=False)
original_sys_path = sys.path
try:
sys.path = sys.path + [input_api.os_path.join(input_api.PresubmitLocalPath(), "scripts")]
@@ -66,9 +70,9 @@ def _FormatDevtools(input_api, output_api):
args=[node_path, format_path] + [glob_arg, "--output-replacements-xml"])
check_formatting_out, _ = check_formatting_process.communicate()
if check_formatting_process.returncode != 0:
- return [output_api.PresubmitError(check_formatting_out)]
+ return CheckOutput([output_api.PresubmitError(check_formatting_out)], has_errors=True)
if "</replacement>" not in check_formatting_out:
- return [output_api.PresubmitNotifyResult("CL is properly formatted")]
+ return CheckOutput([output_api.PresubmitNotifyResult("CL is properly formatted")], has_errors=False)
format_args = [node_path, format_path] + [glob_arg]
format_process = _inputPopen(input_api, format_args)
@@ -86,10 +90,10 @@ def _FormatDevtools(input_api, output_api):
reformat_process = _inputPopen(input_api, format_args)
reformat_process.communicate()
- return [output_api.PresubmitError("ERROR: Found formatting violations.\n"
+ return CheckOutput([output_api.PresubmitError("ERROR: Found formatting violations.\n"
"Ran clang-format on files changed in CL\n"
"Use git status to check the formatting changes"),
- output_api.PresubmitError(format_process_out)]
+ output_api.PresubmitError(format_process_out)], has_errors=True)
def _CheckDevtoolsStyle(input_api, output_api):
@@ -202,8 +206,19 @@ def _CheckCSSViolations(input_api, output_api):
def CheckChangeOnUpload(input_api, output_api):
results = []
- results.extend(_CheckNodeAndNPMModules(input_api, output_api))
- results.extend(_FormatDevtools(input_api, output_api))
+
+ (node_results, has_errors) = _CheckNodeAndNPMModules(input_api, output_api)
+ results.extend(node_results)
+ if has_errors:
+ results.append(output_api.PresubmitError("ERROR: Bailed out early because error using node.js/npm"))
+ return results
+
+ (format_results, has_errors) = _FormatDevtools(input_api, output_api)
+ results.extend(format_results)
+ if has_errors:
+ results.append(output_api.PresubmitError("ERROR: Bailed out early because formatting errors were found"))
+ return results
+
results.extend(_CheckDevtoolsStyle(input_api, output_api))
results.extend(_CompileDevtoolsFrontend(input_api, output_api))
results.extend(_CheckConvertSVGToPNGHashes(input_api, output_api))
@@ -215,6 +230,7 @@ def CheckChangeOnUpload(input_api, output_api):
def CheckChangeOnCommit(input_api, output_api):
return []
+
def _getAffectedFrontEndFiles(input_api):
local_paths = [f.AbsoluteLocalPath() for f in input_api.AffectedFiles() if f.Action() != "D"]
devtools_root = input_api.PresubmitLocalPath()
@@ -222,6 +238,7 @@ def _getAffectedFrontEndFiles(input_api):
affected_front_end_files = [file_name for file_name in local_paths if devtools_front_end in file_name and file_name.endswith(".js")]
return [input_api.os_path.relpath(file_name, devtools_root) for file_name in affected_front_end_files]
+
def _inputPopen(input_api, args):
return input_api.subprocess.Popen(
args,
« no previous file with comments | « third_party/WebKit/Source/devtools/.gitignore ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698