DevTools: Make eslint mandatory and check node.js/npm modules in presubmit

third_party/WebKit/Source/devtools/PRESUBMIT.py

 # Copyright (C) 2014 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 19 matching lines @@
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into gcl.
 """
 
 import sys
 
 compile_note = "Be sure to run your patch by the compile_frontend.py script prior to committing!"
 
 
+def _CheckNodeAndNPMModules(input_api, output_api):

[lushnikov, 2016/10/05 16:22:49]

do we really need this presubmit check?

looks like all the necessary job of installing nodes/npm will be done by the
lint_javascript.py - if needed

[chenwilliam, 2016/10/05 17:59:19]

This is necessary to run "npm install", which is how eslint gets installed. We
could do the npm install step in the lint script, but as we add more build steps
using npm modules, it's better to do this as an explicit first step in the
presubmit script.

[lushnikov, 2016/10/07 17:05:43]

Shell we pursue the hermetic build for the github workflow?

[chenwilliam, 2016/10/10 19:36:17]

I'm currently going through the compliance review for the npm modules repo
(which would be used for buildbots) so for now we can't do a hermetic build
using a git checkout of the npm modules. If we're concerned about versioning, we
can shrinkwrap our package.json which locks down the versions of the entire
dependency tree.  For now, I think because it's just eslint we should be OK.

+    node_script_path = input_api.os_path.join(input_api.PresubmitLocalPath(), "scripts", "install_node_deps.py")
+    process = input_api.subprocess.Popen(
+        [input_api.python_executable, node_script_path],
+        stdout=input_api.subprocess.PIPE,
+        stderr=input_api.subprocess.STDOUT)
+    out, _ = process.communicate()
+    if process.returncode != 0:
+        return [output_api.PresubmitError(out)]
+    return [output_api.PresubmitNotifyResult(out)]
+
+
 def _CheckDevtoolsStyle(input_api, output_api):
     local_paths = [f.AbsoluteLocalPath() for f in input_api.AffectedFiles() if f.Action() != "D"]
 
     devtools_root = input_api.PresubmitLocalPath()
     devtools_front_end = input_api.os_path.join(devtools_root, "front_end")
     affected_front_end_files = [file_name for file_name in local_paths if devtools_front_end in file_name and file_name.endswith(".js")]
     affected_front_end_files = [input_api.os_path.relpath(file_name, devtools_root) for file_name in affected_front_end_files]
     if len(affected_front_end_files) > 0:
         lint_path = input_api.os_path.join(input_api.PresubmitLocalPath(),
                                            "scripts", "lint_javascript.py")
         process = input_api.subprocess.Popen(
             [input_api.python_executable, lint_path] + affected_front_end_files,
             stdout=input_api.subprocess.PIPE,
             stderr=input_api.subprocess.STDOUT)
         out, _ = process.communicate()
         if process.returncode != 0:
             return [output_api.PresubmitError(out)]
-        if "NOTE" in out:
-            return [output_api.PresubmitNotifyResult(out)]
+        return [output_api.PresubmitNotifyResult(out)]
     return []
 
 
 def _CompileDevtoolsFrontend(input_api, output_api):
     local_paths = [f.LocalPath() for f in input_api.AffectedFiles()]
 
     # FIXME: The compilation does not actually run if injected script-related files
     # have changed, as they reside in core/inspector, which is not affected
     # by this presubmit.
     # Once this is fixed, injected_script_externs.js
@@ skipping 77 matching lines @@
         for line_number, line in f.ChangedContents():
             if "/deep/" in line:
                 results.append(output_api.PresubmitError(("%s:%d uses /deep/ selector") % (f.LocalPath(), line_number)))
             if "::shadow" in line:
                 results.append(output_api.PresubmitError(("%s:%d uses ::shadow selector") % (f.LocalPath(), line_number)))
     return results
 
 
 def CheckChangeOnUpload(input_api, output_api):
     results = []
+    results.extend(_CheckNodeAndNPMModules(input_api, output_api))
     results.extend(_CheckDevtoolsStyle(input_api, output_api))
     results.extend(_CompileDevtoolsFrontend(input_api, output_api))
     results.extend(_CheckConvertSVGToPNGHashes(input_api, output_api))
     results.extend(_CheckOptimizePNGHashes(input_api, output_api))
     results.extend(_CheckCSSViolations(input_api, output_api))
     return results
 
 
 def CheckChangeOnCommit(input_api, output_api):
     return []