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

tools/web_dev_style/js_checker_test.py

Index: tools/web_dev_style/js_checker_test.py
diff --git a/tools/web_dev_style/js_checker_test.py b/tools/web_dev_style/js_checker_test.py
index 72f99545b69028d16aa81b4ef78c2501635e71c1..428c78248865c7f0fe37de8913721115fb3b28ce 100755
--- a/tools/web_dev_style/js_checker_test.py
+++ b/tools/web_dev_style/js_checker_test.py
@@ -203,49 +203,6 @@ class JsCheckerTest(SuperMoxTestBase):
     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)