Refactor GPU integration test classes

Refactors most of the code from "PixelIntegrationTest" and
"CloudStorageIntegrationTestBase" into "SkiaGoldIntegrationTestBase",
deleting some old unused code in the process. "PixelIntegrationTest" now
only contains code specific to the tests invoked via the "pixel" test
type.

Bug: 1008524
Change-Id: Ibe8e80ea7a4e16becd889d1484738e1abbcf32bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1845951
Auto-Submit: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703531}

content/test/gpu/gpu_tests/cloud_storage_integration_test_base.py

-# Copyright 2016 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.
-
-"""Base classes for a test which uploads results (reference images,
-error images) to cloud storage."""
-
-import logging
-import os
-import re
-
-import tempfile
-
-from py_utils import cloud_storage
-from telemetry.util import image_util
-from telemetry.util import rgba_color
-
-from gpu_tests import gpu_integration_test
-
-test_data_dir = os.path.abspath(os.path.join(
-    os.path.dirname(__file__), '..', '..', 'data', 'gpu'))
-
-default_generated_data_dir = os.path.join(test_data_dir, 'generated')
-
-
-class CloudStorageIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
-  # This class is abstract; don't warn about the superclass's abstract
-  # methods that aren't overridden.
-  # pylint: disable=abstract-method
-
-  # The command line options (which are passed to subclasses'
-  # GenerateGpuTests) *must* be configured here, via a call to
-  # SetParsedCommandLineOptions. If they are not, an error will be
-  # raised when running the tests.
-  _parsed_command_line_options = None
-
-  _error_image_cloud_storage_bucket = 'chromium-browser-gpu-tests'
-
-  @classmethod
-  def SetParsedCommandLineOptions(cls, options):
-    cls._parsed_command_line_options = options
-
-  @classmethod
-  def GetParsedCommandLineOptions(cls):
-    return cls._parsed_command_line_options
-
-  @classmethod
-  def AddCommandlineArgs(cls, parser):
-    super(CloudStorageIntegrationTestBase, cls).AddCommandlineArgs(parser)
-    parser.add_option(
-      '--build-revision',
-      help='Chrome revision being tested.',
-      default="unknownrev")
-    parser.add_option(
-      '--os-type',
-      help='Type of operating system on which the pixel test is being run, '
-      'used only to distinguish different operating systems with the same '
-      'graphics card. Any value is acceptable, but canonical values are '
-      '"win", "mac", and "linux", and probably, eventually, "chromeos" '
-      'and "android").',
-      default='')
-    parser.add_option(
-      '--test-machine-name',
-      help='Name of the test machine. Specifying this argument causes this '
-      'script to upload failure images and diffs to cloud storage directly, '
-      'instead of relying on the archive_gpu_pixel_test_results.py script.',
-      default='')
-    parser.add_option(
-      '--generated-dir',
-      help='Overrides the default on-disk location for generated test images '
-      '(only used for local testing without a cloud storage account)',
-      default=default_generated_data_dir)
-    parser.add_option(
-      '--dont-restore-color-profile-after-test',
-      dest='dont_restore_color_profile_after_test',
-      action='store_true', default=False,
-      help='(Mainly on Mac) don\'t restore the system\'s original color '
-      'profile after the test completes; leave the system using the sRGB color '
-      'profile. See http://crbug.com/784456.')
-
-  def _CompareScreenshotSamples(self, tab, screenshot, expected_colors,
-                                tolerance, device_pixel_ratio,
-                                test_machine_name):
-    # First scan through the expected_colors and see if there are any scale
-    # factor overrides that would preempt the device pixel ratio. This
-    # is mainly a workaround for complex tests like the Maps test.
-    for expectation in expected_colors:
-      if 'scale_factor_overrides' in expectation:
-        for override in expectation['scale_factor_overrides']:
-          # Require exact matches to avoid confusion, because some
-          # machine models and names might be subsets of others
-          # (e.g. Nexus 5 vs Nexus 5X).
-          if ('device_type' in override and
-              (tab.browser.platform.GetDeviceTypeName() ==
-               override['device_type'])):
-            logging.warning(
-              'Overriding device_pixel_ratio ' + str(device_pixel_ratio) +
-              ' with scale factor ' + str(override['scale_factor']) +
-              ' for device type ' + override['device_type'])
-            device_pixel_ratio = override['scale_factor']
-            break
-          if (test_machine_name and 'machine_name' in override and
-              override["machine_name"] == test_machine_name):
-            logging.warning(
-              'Overriding device_pixel_ratio ' + str(device_pixel_ratio) +
-              ' with scale factor ' + str(override['scale_factor']) +
-              ' for machine name ' + test_machine_name)
-            device_pixel_ratio = override['scale_factor']
-            break
-        # Only support one "scale_factor_overrides" in the expectation format.
-        break
-    for expectation in expected_colors:
-      if "scale_factor_overrides" in expectation:
-        continue
-      location = expectation["location"]
-      size = expectation["size"]
-      x0 = int(location[0] * device_pixel_ratio)
-      x1 = int((location[0] + size[0]) * device_pixel_ratio)
-      y0 = int(location[1] * device_pixel_ratio)
-      y1 = int((location[1] + size[1]) * device_pixel_ratio)
-      for x in range(x0, x1):
-        for y in range(y0, y1):
-          if (x < 0 or y < 0 or x >= image_util.Width(screenshot) or
-              y >= image_util.Height(screenshot)):
-            self.fail(
-                ('Expected pixel location [%d, %d] is out of range on ' +
-                 '[%d, %d] image') %
-                (x, y, image_util.Width(screenshot),
-                 image_util.Height(screenshot)))
-
-          actual_color = image_util.GetPixelColor(screenshot, x, y)
-          expected_color = rgba_color.RgbaColor(
-              expectation["color"][0],
-              expectation["color"][1],
-              expectation["color"][2],
-              expectation["color"][3] if len(expectation["color"]) > 3 else 255)
-          if not actual_color.IsEqual(expected_color, tolerance):
-            self.fail('Expected pixel at ' + str(location) +
-                ' (actual pixel (' + str(x) + ', ' + str(y) + ')) ' +
-                ' to be ' +
-                str(expectation["color"]) + " but got [" +
-                str(actual_color.r) + ", " +
-                str(actual_color.g) + ", " +
-                str(actual_color.b) + ", " +
-                str(actual_color.a) + "]")
-
-  ###
-  ### Routines working with the local disk (only used for local
-  ### testing without a cloud storage account -- the bots do not use
-  ### this code path).
-  ###
-
-  def _UrlToImageName(self, url):
-    image_name = re.sub(r'^(http|https|file)://(/*)', '', url)
-    image_name = re.sub(r'\.\./', '', image_name)
-    image_name = re.sub(r'(\.|/|-)', '_', image_name)
-    return image_name
-
-  def _WriteImage(self, image_path, png_image):
-    output_dir = os.path.dirname(image_path)
-    if not os.path.exists(output_dir):
-      os.makedirs(output_dir)
-    image_util.WritePngFile(png_image, image_path)
-
-  def _WriteErrorImages(self, img_dir, img_name, screenshot, ref_png):
-    full_image_name = img_name + '_' + str(
-      self.GetParsedCommandLineOptions().build_revision)
-    full_image_name = full_image_name + '.png'
-
-    # Always write the failing image.
-    self._WriteImage(
-        os.path.join(img_dir, 'FAIL_' + full_image_name), screenshot)
-
-    if ref_png is not None:
-      # Save the reference image.
-      # This ensures that we get the right revision number.
-      self._WriteImage(
-          os.path.join(img_dir, full_image_name), ref_png)
-
-      # Save the difference image.
-      diff_png = image_util.Diff(screenshot, ref_png)
-      self._WriteImage(
-          os.path.join(img_dir, 'DIFF_' + full_image_name), diff_png)
-
-  @classmethod
-  def _UploadBitmapToCloudStorage(cls, bucket, name, bitmap, public=False):
-    # This sequence of steps works on all platforms to write a temporary
-    # PNG to disk, following the pattern in bitmap_unittest.py. The key to
-    # avoiding PermissionErrors seems to be to not actually try to write to
-    # the temporary file object, but to re-open its name for all operations.
-    temp_file = tempfile.NamedTemporaryFile(suffix='.png').name
-    image_util.WritePngFile(bitmap, temp_file)
-    cloud_storage.Insert(bucket, name, temp_file, publicly_readable=public)
-
-  @classmethod
-  def GenerateGpuTests(cls, options):
-    del options
-    return []

content/test/gpu/gpu_tests/maps_integration_test.py

@@ -6,11 +6,11 @@ import json
 import os
 import sys
 
+from gpu_tests import color_profile_manager
 from gpu_tests import gpu_integration_test
-from gpu_tests import pixel_integration_test
 from gpu_tests import path_util
-from gpu_tests import color_profile_manager
 from gpu_tests import pixel_test_pages
+from gpu_tests import skia_gold_integration_test_base
 
 from py_utils import cloud_storage
 
@@ -22,7 +22,8 @@ _DATA_PATH = os.path.join(path_util.GetChromiumSrcDir(),
 
 _TOLERANCE = 3
 
-class MapsIntegrationTest(pixel_integration_test.PixelIntegrationTest):
+class MapsIntegrationTest(
+    skia_gold_integration_test_base.SkiaGoldIntegrationTestBase):
   """Google Maps pixel tests.
 
   Note: this test uses the same WPR as the smoothness.maps benchmark
@@ -110,7 +111,7 @@ class MapsIntegrationTest(pixel_integration_test.PixelIntegrationTest):
   def _MapsExpectationToPixelExpectation(self, url, expected_colors, tolerance):
     page = pixel_test_pages.PixelTestPage(
         url=url,
-        name=('Maps_' + url),
+        name=('Maps_maps'),
         # Exact test_rect is arbitrary, just needs to encapsulate all pixels
         # that are tested.
         test_rect=[0, 0, 600, 400],

testing/buildbot/chromium.gpu.json

@@ -166,8 +166,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "android",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -212,8 +210,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "android",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -578,8 +574,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -623,8 +617,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -1010,8 +1002,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -1055,8 +1045,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -1432,8 +1420,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -1476,8 +1462,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -1843,8 +1827,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -1887,8 +1869,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -2246,8 +2226,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -2292,8 +2270,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -2685,8 +2661,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -2731,8 +2705,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -3184,8 +3156,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "win",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -3229,8 +3199,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "win",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -3816,8 +3784,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "win",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",
@@ -3870,8 +3836,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "win",
           "--build-revision",
           "${got_revision}",
           "--test-machine-name",

testing/buildbot/client.v8.fyi.json

@@ -128,8 +128,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "android",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -173,8 +171,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "android",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -455,8 +451,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -499,8 +493,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -776,8 +768,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -820,8 +810,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "linux",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -1093,8 +1081,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -1136,8 +1122,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "mac",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -1734,8 +1718,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "win",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",
@@ -1778,8 +1760,6 @@
           "-v",
           "--extra-browser-args=--enable-logging=stderr --js-flags=--expose-gc",
           "--dont-restore-color-profile-after-test",
-          "--os-type",
-          "win",
           "--build-revision",
           "${got_cr_revision}",
           "--test-machine-name",

testing/buildbot/test_suites.pyl

@@ -175,8 +175,6 @@
         'name': 'android_webview_pixel_skia_gold_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',
@@ -3714,8 +3712,6 @@
         'name': 'egl_pixel_skia_gold_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',
@@ -3745,8 +3741,6 @@
         'name': 'skia_renderer_pixel_skia_gold_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',
@@ -3794,8 +3788,6 @@
         'name': 'maps_pixel_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',
@@ -3853,8 +3845,6 @@
         'name': 'vulkan_pixel_skia_gold_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',
@@ -3895,8 +3885,6 @@
         'name': 'maps_pixel_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',
@@ -3921,8 +3909,6 @@
         'name': 'pixel_skia_gold_test',
         'args': [
           '--dont-restore-color-profile-after-test',
-          '--os-type',
-          '${os_type}',
           '--build-revision',
           '${got_revision}',
           '--test-machine-name',