js_checker.py: Replace custom GetElementByIdCheck with ESLint no-restricted-properties.

tools/web_dev_style/js_checker_test.py

 #!/usr/bin/env python
 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import js_checker
 from os import path as os_path
 import re
 from sys import path as sys_path
 import test_util
@@ skipping 185 matching lines @@
 
   def testExtraDotInGenericFails(self):
     lines = [
         "/** @private {!Array.<!Frobber>} */",
         "var a = /** @type {Object.<number>} */({});",
         "* @return {!Promise.<Change>}"
     ]
     for line in lines:
       self.ShouldFailExtraDotInGenericCheck(line)
 
-  def ShouldFailGetElementByIdCheck(self, line):

[Dan Beam, 2017/05/30 16:17:26]

can we leave some of these?

[dpapad, 2017/05/30 17:08:24]

We would need to modify those tests to run self.checker.RunEsLintChecks(), which
accepts files, not lines of text. Which means we would need to keep some test
files around to pass them to the test.

Do you think is valuable to maintain our own tests to cover ESLint's potential
regressions? It seems unnecessary to me, given that we are (and will be) on
stable version's of ESLint.

[Dan Beam, 2017/05/30 17:11:41]

yes

[dpapad, 2017/05/30 18:30:15]

Trying to address this comment, but I keep hitting obstacles. Specifically, our
python tests (specifically SuperMoxTestBase) seem to run in a very restricted
environment where most things have been replaced by mock objects. For example in
order to run ESLint we need to use subprocess.Popen at [1], but this is
restricted at [2]. It seems that SuperMoxTestBase tests are not meant to spawn
other processes.

[1]
https://cs.chromium.org/chromium/src/third_party/node/node.py?q=third_party/n...
[2]
https://cs.chromium.org/chromium/tools/depot_tools/testing_support/super_mox....

[Dan Beam, 2017/05/30 20:33:30]

i think we should only use SuperMoxTestBase if it's helping us.

[dpapad, 2017/05/30 22:11:21]

I am restoring similar tests at https://codereview.chromium.org/2917473002.

-    """Checks that the 'getElementById' checker flags |line| as a style
-       error.
-    """
-    error = self.checker.GetElementByIdCheck(1, line)
-    self.assertNotEqual('', error,
-        'Should be flagged as style error: ' + line)
-    self.assertEqual(test_util.GetHighlight(line, error),
-                     'document.getElementById')
-
-  def ShouldPassGetElementByIdCheck(self, line):
-    """Checks that the 'getElementById' checker doesn't flag |line| as a style
-       error.
-    """
-    self.assertEqual('', self.checker.GetElementByIdCheck(1, line),
-        'Should not be flagged as style error: ' + line)
-
-  def testGetElementByIdFails(self):
-    lines = [
-        "document.getElementById('foo');",
-        "  document.getElementById('foo');",
-        "var x = document.getElementById('foo');",
-        "if (document.getElementById('foo').hidden) {",
-    ]
-    for line in lines:
-      self.ShouldFailGetElementByIdCheck(line)
-
-  def testGetElementByIdPasses(self):
-    lines = [
-        "elem.ownerDocument.getElementById('foo');",
-        "  elem.ownerDocument.getElementById('foo');",
-        "var x = elem.ownerDocument.getElementById('foo');",
-        "if (elem.ownerDocument.getElementById('foo').hidden) {",
-        "doc.getElementById('foo');",
-        "  doc.getElementById('foo');",
-        "cr.doc.getElementById('foo');",
-        "  cr.doc.getElementById('foo');",
-        "var x = doc.getElementById('foo');",
-        "if (doc.getElementById('foo').hidden) {",
-    ]
-    for line in lines:
-      self.ShouldPassGetElementByIdCheck(line)
-
   def ShouldFailInheritDocCheck(self, line):
     """Checks that the '@inheritDoc' checker flags |line| as a style error."""
     error = self.checker.InheritDocCheck(1, line)
     self.assertNotEqual('', error,
         msg='Should be flagged as style error: ' + line)
     self.assertEqual(test_util.GetHighlight(line, error), '@inheritDoc')
 
   def ShouldPassInheritDocCheck(self, line):
     """Checks that the '@inheritDoc' checker doesn't flag |line| as a style
        error.
@@ skipping 132 matching lines @@
         " var SCARE_SMALL_CHILDREN = [",         # TODO(dbeam): add @const in
                                                  # front of all these vars like
         "/** @const */ CONST_VAR = 1;",          # this line has (<--).
     ]
     for line in lines:
       self.ShouldPassVarNameCheck(line)
 
 
 if __name__ == '__main__':
   unittest.main()