Clean up remaining GPU PyLint disables

Re-enables or provides disable reasons for all remaining PyLint
disables in the //content/test/gpu pylintrc files and fixes any
resulting warnings.

Bug: 1076144
Change-Id: Ia2732ad9063812e89f00beb6f1d36f989958b624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2327180Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792961}

content/test/gpu/gpu_tests/gpu_integration_test_unittest.py

@@ -30,6 +30,10 @@ from telemetry.testing import run_browser_tests
 path_util.AddDirToPathIfNeeded(path_util.GetChromiumSrcDir(), 'tools', 'perf')
 from chrome_telemetry_build import chromium_config
 
+# Unittest test cases are defined as public methods, so ignore complaints about
+# having too many.
+# pylint: disable=too-many-public-methods
+
 VENDOR_NVIDIA = 0x10DE
 VENDOR_AMD = 0x1002
 VENDOR_INTEL = 0x8086

content/test/gpu/gpu_tests/info_collection_test.py

@@ -8,6 +8,15 @@ import os
 import sys
 
 
+class InfoCollectionTestArgs(object):
+  """Struct-like class for passing args to an InfoCollection test."""
+
+  def __init__(self, expected_vendor_id_str=None, expected_device_id_strs=None):
+    self.gpu = None
+    self.expected_vendor_id_str = expected_vendor_id_str
+    self.expected_device_id_strs = expected_device_id_strs
+
+
 class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
   @classmethod
   def Name(cls):
@@ -26,15 +35,15 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
 
   @classmethod
   def GenerateGpuTests(cls, options):
-    yield ('InfoCollection_basic', '_', ('_RunBasicTest', {
-        'expected_vendor_id_str':
-        options.expected_vendor_id,
-        'expected_device_id_strs':
-        options.expected_device_ids,
-    }))
+    yield ('InfoCollection_basic', '_',
+           ('_RunBasicTest',
+            InfoCollectionTestArgs(
+                expected_vendor_id_str=options.expected_vendor_id,
+                expected_device_id_strs=options.expected_device_ids)))
     yield ('InfoCollection_direct_composition', '_',
-           ('_RunDirectCompositionTest', {}))
-    yield ('InfoCollection_dx12_vulkan', '_', ('_RunDX12VulkanTest', {}))
+           ('_RunDirectCompositionTest', InfoCollectionTestArgs()))
+    yield ('InfoCollection_dx12_vulkan', '_', ('_RunDX12VulkanTest',
+                                               InfoCollectionTestArgs()))
 
   @classmethod
   def SetUpProcess(cls):
@@ -54,18 +63,16 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
 
     assert len(args) == 2
     test_func = args[0]
-    kwargs = args[1]
-    assert 'gpu' not in kwargs
-    kwargs['gpu'] = system_info.gpu
-    getattr(self, test_func)(**kwargs)
+    test_args = args[1]
+    assert test_args.gpu is None
+    test_args.gpu = system_info.gpu
+    getattr(self, test_func)(test_args)
 
   ######################################
   # Helper functions for the tests below
 
-  def _RunBasicTest(self, gpu, expected_vendor_id_str, expected_device_id_strs,
-                    **kwargs):
-    del kwargs  # Any unused extra arguments that got passed in.
-    device = gpu.devices[0]
+  def _RunBasicTest(self, test_args):
+    device = test_args.gpu.devices[0]
     if not device:
       self.fail("System Info doesn't have a gpu")
 
@@ -73,12 +80,13 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
     detected_device_id = device.device_id
 
     # Gather the expected IDs passed on the command line
-    if expected_vendor_id_str is None or expected_device_id_strs is []:
+    if (not test_args.expected_vendor_id_str
+        or not test_args.expected_device_id_strs):
       self.fail("Missing --expected-[vendor|device]-id command line args")
 
-    expected_vendor_id = int(expected_vendor_id_str, 16)
+    expected_vendor_id = int(test_args.expected_vendor_id_str, 16)
     expected_device_ids = [
-        int(id_str, 16) for id_str in expected_device_id_strs
+        int(id_str, 16) for id_str in test_args.expected_device_id_strs
     ]
 
     # Check expected and detected GPUs match
@@ -90,12 +98,11 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
       self.fail('Device ID mismatch, expected %s but got %s.' %
                 (expected_device_ids, detected_device_id))
 
-  def _RunDirectCompositionTest(self, gpu, **kwargs):
-    del kwargs  # Any unused extra arguments that got passed in.
+  def _RunDirectCompositionTest(self, test_args):
     os_name = self.browser.platform.GetOSName()
     if os_name and os_name.lower() == 'win':
       overlay_bot_config = self.GetOverlayBotConfig()
-      aux_attributes = gpu.aux_attributes
+      aux_attributes = test_args.gpu.aux_attributes
       if not aux_attributes:
         self.fail('GPU info does not have aux_attributes.')
       for field, expected in overlay_bot_config.iteritems():
@@ -105,8 +112,7 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
               '%s mismatch, expected %s but got %s.' %
               (field, self._ValueToStr(expected), self._ValueToStr(detected)))
 
-  def _RunDX12VulkanTest(self, **kwargs):
-    del kwargs  # Any unused extra arguments that got passed in.
+  def _RunDX12VulkanTest(self, _):
     os_name = self.browser.platform.GetOSName()
     if os_name and os_name.lower() == 'win':
       self.RestartBrowserIfNecessaryWithArgs(

content/test/gpu/gpu_tests/trace_integration_test.py

@@ -229,17 +229,6 @@ class TraceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
     ])
     return default_args
 
-  def _GetOverlayBotConfigHelper(self):
-    system_info = self.browser.GetSystemInfo()
-    if not system_info:
-      raise Exception("Browser doesn't support GetSystemInfo")
-    gpu = system_info.gpu.devices[0]
-    if not gpu:
-      raise Exception("System Info doesn't have a gpu")
-    os_version_name = self.browser.platform.GetOSVersionName()
-    return self.GetOverlayBotConfig(os_version_name, gpu.vendor_id,
-                                    gpu.device_id)
-
   def _GetAndAssertOverlayBotConfig(self):
     overlay_bot_config = self.GetOverlayBotConfig()
     if overlay_bot_config is None:

content/test/gpu/gpu_tests/webgl_conformance_integration_test.py

@@ -65,6 +65,15 @@ def _CompareVersion(version1, version2):
   return cmp(ver_num1[0:size], ver_num2[0:size])
 
 
+class WebGLTestArgs(object):
+  """Struct-like class for passing args to a WebGLConformance test."""
+
+  def __init__(self, webgl_version=None, extension=None, extension_list=None):
+    self.webgl_version = webgl_version
+    self.extension = extension
+    self.extension_list = extension_list
+
+
 class WebGLConformanceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
 
   _webgl_version = None
@@ -114,7 +123,8 @@ class WebGLConformanceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
         test_path_with_args += '?webglVersion=' + str(cls._webgl_version)
       yield (test_path.replace(os.path.sep, '/'),
              os.path.join(webgl_test_util.conformance_relpath,
-                          test_path_with_args), ('_RunConformanceTest'))
+                          test_path_with_args), ('_RunConformanceTest',
+                                                 WebGLTestArgs()))
 
     #
     # Extension tests
@@ -124,13 +134,17 @@ class WebGLConformanceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
     yield ('WebglExtension_TestCoverage',
            os.path.join(webgl_test_util.extensions_relpath,
                         'webgl_extension_test.html'),
-           ('_RunExtensionCoverageTest', extension_tests, cls._webgl_version))
+           ('_RunExtensionCoverageTest',
+            WebGLTestArgs(webgl_version=cls._webgl_version,
+                          extension_list=extension_tests)))
     # Individual extension tests.
     for extension in extension_tests:
       yield ('WebglExtension_%s' % extension,
              os.path.join(webgl_test_util.extensions_relpath,
                           'webgl_extension_test.html'),
-             ('_RunExtensionTest', extension, cls._webgl_version))
+             ('_RunExtensionTest',
+              WebGLTestArgs(webgl_version=cls._webgl_version,
+                            extension=extension)))
 
   @classmethod
   def _GetExtensionList(cls):
@@ -202,8 +216,10 @@ class WebGLConformanceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
   def RunActualGpuTest(self, test_path, *args):
     # This indirection allows these tests to trampoline through
     # _RunGpuTest.
+    assert len(args) == 2
     test_name = args[0]
-    getattr(self, test_name)(test_path, *args[1:])
+    test_args = args[1]
+    getattr(self, test_name)(test_path, test_args)
 
   def _VerifyGLBackend(self, gpu_info):
     # Verify that Chrome's GL backend matches if a specific one was requested
@@ -280,20 +296,17 @@ class WebGLConformanceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
     elif not self._DidWebGLTestSucceed(self.tab):
       self.fail(self._WebGLTestMessages(self.tab))
 
-  def _RunConformanceTest(self, test_path, *args):
-    del args  # Unused in conformance tests.
+  def _RunConformanceTest(self, test_path, _):
     self._NavigateTo(test_path, conformance_harness_script)
     self._CheckTestCompletion()
 
-  def _RunExtensionCoverageTest(self, test_path, *args):
+  def _RunExtensionCoverageTest(self, test_path, test_args):
     self._NavigateTo(test_path, _GetExtensionHarnessScript())
     self.tab.action_runner.WaitForJavaScriptCondition(
         'window._loaded', timeout=self._GetTestTimeout())
-    extension_list = args[0]
-    webgl_version = args[1]
-    context_type = "webgl2" if webgl_version == 2 else "webgl"
+    context_type = "webgl2" if test_args.webgl_version == 2 else "webgl"
     extension_list_string = "["
-    for extension in extension_list:
+    for extension in test_args.extension_list:
       extension_list_string = extension_list_string + extension + ", "
     extension_list_string = extension_list_string + "]"
     self.tab.action_runner.EvaluateJavaScript(
@@ -302,16 +315,14 @@ class WebGLConformanceIntegrationTest(gpu_integration_test.GpuIntegrationTest):
         context_type=context_type)
     self._CheckTestCompletion()
 
-  def _RunExtensionTest(self, test_path, *args):
+  def _RunExtensionTest(self, test_path, test_args):
     self._NavigateTo(test_path, _GetExtensionHarnessScript())
     self.tab.action_runner.WaitForJavaScriptCondition(
         'window._loaded', timeout=self._GetTestTimeout())
-    extension = args[0]
-    webgl_version = args[1]
-    context_type = "webgl2" if webgl_version == 2 else "webgl"
+    context_type = "webgl2" if test_args.webgl_version == 2 else "webgl"
     self.tab.action_runner.EvaluateJavaScript(
         'checkExtension({{ extension }}, {{ context_type }})',
-        extension=extension,
+        extension=test_args.extension,
         context_type=context_type)
     self._CheckTestCompletion()
 

content/test/gpu/gpu_tests/webgl_test_util.py

@@ -10,14 +10,17 @@ conformance_relcomps = ('third_party', 'webgl', 'src', 'sdk', 'tests')
 
 extensions_relcomps = ('content', 'test', 'data', 'gpu')
 
+# pylint: disable=star-args
 conformance_relpath = os.path.join(*conformance_relcomps)
+extensions_relpath = os.path.join(*extensions_relcomps)
+# pylint: enable=star-args
 conformance_path = os.path.join(path_util.GetChromiumSrcDir(),
                                 conformance_relpath)
-extensions_relpath = os.path.join(*extensions_relcomps)
 
 # These URL prefixes are needed because having more than one static
 # server dir is causing the base server directory to be moved up the
 # directory hierarchy.
 url_prefixes_to_trim = [
-    '/'.join(conformance_relcomps) + '/', '/'.join(extensions_relcomps) + '/'
+    '/'.join(conformance_relcomps) + '/',
+    '/'.join(extensions_relcomps) + '/',
 ]

content/test/gpu/pylintrc

 [MESSAGES CONTROL]
 
 # Disable the message, report, category or checker with the given id(s).
-# TODO(crbug.com/1076144): Shrink this list to as small as possible.
-disable=fixme,
+disable=duplicate-code,
+        fixme,
         invalid-name,
         locally-disabled,
         locally-enabled,
         missing-docstring,
-        star-args,
         too-few-public-methods,
-        too-many-function-args,
-        too-many-instance-attributes,
-        too-many-public-methods,
-        too-many-statements,
-        duplicate-code
+        too-many-instance-attributes
 
 # Pylint, or at least v1.5.6, does not properly handle adding comments between
 # lines of disable= entries, nor does it allow multiple disable= lines, so
@@ -41,11 +36,21 @@ disable=fixme,
 # There are valid cases where we want to selectively enable/disable lint errors
 # in a particular file/scope, e.g. allowing protected access in unittests.
 
+# missing-docstring
+# Docstrings are important, and proper use of them should be enforced. However,
+# requiring a docstring on *every* method and class is overkill, as many are
+# self-documenting due to being short and well-named.
+
 # too-few-public-methods
 # This is supposedly to catch "functions disguised as classes", but we have
 # never had issues with that. We do, however, use a handful of struct-like
 # classes, which this complains about.
 
+# too-many-instance-attributes
+# Only ends up complaining about struct-like classes. So, like
+# too-few-public-methods, leave disabled instead of disabling on a per-class
+# basis.
+
 # Other notes
 
 # too-many-arguments