Make maps_pixel_test upload to Gold

Makes maps_pixel_test upload to Skia Gold instead of to a cloud storage
bucket. This will make it easier to debug failures, as the images get
uploaded to the same place as the regular GPU pixel tests.

Bug: 1009137
Change-Id: Ia2f19f43efa216f551c0ea47c2018e9c67981b47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834825Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702547}

content/test/gpu/gpu_tests/maps_integration_test.py

@@ -4,18 +4,16 @@
 
 import json
 import os
-import re
 import sys
 
 from gpu_tests import gpu_integration_test
-from gpu_tests import cloud_storage_integration_test_base
+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 py_utils import cloud_storage
 
-from telemetry.util import image_util
-
 _MAPS_PERF_TEST_PATH = os.path.join(
   path_util.GetChromiumSrcDir(), 'tools', 'perf', 'page_sets', 'maps_perf_test')
 
@@ -24,8 +22,7 @@ _DATA_PATH = os.path.join(path_util.GetChromiumSrcDir(),
 
 _TOLERANCE = 3
 
-class MapsIntegrationTest(
-    cloud_storage_integration_test_base.CloudStorageIntegrationTestBase):
+class MapsIntegrationTest(pixel_integration_test.PixelIntegrationTest):
   """Google Maps pixel tests.
 
   Note: this test uses the same WPR as the smoothness.maps benchmark
@@ -71,56 +68,6 @@ class MapsIntegrationTest(
       json_contents = json.load(f)
     return json_contents
 
-  @classmethod
-  def _UploadErrorImagesToCloudStorage(cls, image_name, screenshot, ref_img):
-    """For a failing run, uploads the failing image, reference image (if
-    supplied), and diff image (if reference image was supplied) to cloud
-    storage. This subsumes the functionality of the
-    archive_gpu_pixel_test_results.py script."""
-    machine_name = re.sub(r'\W+', '_',
-                          cls.GetParsedCommandLineOptions().test_machine_name)
-    upload_dir = '%s_%s_telemetry' % (
-      cls.GetParsedCommandLineOptions().build_revision, machine_name)
-    base_bucket = '%s/runs/%s' % (
-        cls._error_image_cloud_storage_bucket, upload_dir)
-    image_name_with_revision = '%s_%s.png' % (
-      image_name, cls.GetParsedCommandLineOptions().build_revision)
-    cls._UploadBitmapToCloudStorage(
-      base_bucket + '/gen', image_name_with_revision, screenshot,
-      public=True)
-    if ref_img is not None:
-      cls._UploadBitmapToCloudStorage(
-        base_bucket + '/ref', image_name_with_revision, ref_img, public=True)
-      diff_img = image_util.Diff(screenshot, ref_img)
-      cls._UploadBitmapToCloudStorage(
-        base_bucket + '/diff', image_name_with_revision, diff_img,
-        public=True)
-    print ('See http://%s.commondatastorage.googleapis.com/'
-           'view_test_results.html?%s for this run\'s test results') % (
-      cls._error_image_cloud_storage_bucket, upload_dir)
-
-  def _ValidateScreenshotSamples(self, tab, url, screenshot, expectations,
-                                 tolerance, device_pixel_ratio):
-    """Samples the given screenshot and verifies pixel color values.
-       The sample locations and expected color values are given in expectations.
-       In case any of the samples do not match the expected color, it raises
-       a Failure and dumps the screenshot locally or cloud storage depending on
-       what machine the test is being run."""
-    try:
-      self._CompareScreenshotSamples(
-        tab, screenshot, expectations, tolerance, device_pixel_ratio,
-        self.GetParsedCommandLineOptions().test_machine_name)
-    except Exception:
-      # An exception raised from self.fail() indicates a failure.
-      image_name = self._UrlToImageName(url)
-      if self.GetParsedCommandLineOptions().test_machine_name:
-        self._UploadErrorImagesToCloudStorage(image_name, screenshot, None)
-      else:
-        self._WriteErrorImages(
-          self.GetParsedCommandLineOptions().generated_dir, image_name,
-          screenshot, None)
-      raise
-
   def RunActualGpuTest(self, url, *args):
     tab = self.tab
     pixel_expectations_file = args[0]
@@ -155,8 +102,21 @@ class MapsIntegrationTest(
     # the test-machine-name argument being specified on the command
     # line.
     expected = self._ReadPixelExpectations(pixel_expectations_file)
-    self._ValidateScreenshotSamples(
-      tab, url, screenshot, expected, _TOLERANCE, dpr)
+    page = self._MapsExpectationToPixelExpectation(url, expected, _TOLERANCE)
+    self._ValidateScreenshotSamplesWithSkiaGold(
+        tab, page, screenshot, dpr, self._GetBuildIdArgs())
+
+
+  def _MapsExpectationToPixelExpectation(self, url, expected_colors, tolerance):
+    page = pixel_test_pages.PixelTestPage(
+        url=url,
+        name=('Maps_' + url),
+        # Exact test_rect is arbitrary, just needs to encapsulate all pixels
+        # that are tested.
+        test_rect=[0, 0, 600, 400],
+        tolerance=tolerance,
+        expected_colors=expected_colors)
+    return page
 
   @classmethod
   def ExpectationsFiles(cls):

content/test/gpu/gpu_tests/pixel_integration_test.py

@@ -302,23 +302,7 @@ class PixelIntegrationTest(
             int(page.test_rect[1] * dpr), int(page.test_rect[2] * dpr),
             int(page.test_rect[3] * dpr))
 
-      # Get all the information that goldctl requires.
-      parsed_options = self.GetParsedCommandLineOptions()
-      build_id_args = [
-        '--commit',
-        parsed_options.build_revision,
-      ]
-      # If --review-patch-issue is passed, then we assume we're running on a
-      # trybot.
-      if parsed_options.review_patch_issue:
-        build_id_args += [
-          '--issue',
-          parsed_options.review_patch_issue,
-          '--patchset',
-          parsed_options.review_patch_set,
-          '--jobid',
-          parsed_options.buildbucket_build_id
-        ]
+      build_id_args = self._GetBuildIdArgs()
 
       # Compare images against approved images/colors.
       if page.expected_colors:
@@ -336,6 +320,26 @@ class PixelIntegrationTest(
         # Assume that page actions might have killed the GPU process.
         self._RestartBrowser('Must restart after page actions')
 
+  def _GetBuildIdArgs(self):
+    # Get all the information that goldctl requires.
+    parsed_options = self.GetParsedCommandLineOptions()
+    build_id_args = [
+      '--commit',
+      parsed_options.build_revision,
+    ]
+    # If --review-patch-issue is passed, then we assume we're running on a
+    # trybot.
+    if parsed_options.review_patch_issue:
+      build_id_args += [
+        '--issue',
+        parsed_options.review_patch_issue,
+        '--patchset',
+        parsed_options.review_patch_set,
+        '--jobid',
+        parsed_options.buildbucket_build_id
+      ]
+    return build_id_args
+
   def _UploadTestResultToSkiaGold(self, image_name, screenshot,
                                   tab, page, build_id_args=None):
     """Compares the given image using Skia Gold and uploads the result.

testing/buildbot/chromium.gpu.json

@@ -179,6 +179,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -191,7 +199,8 @@
               "os": "Android"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -582,6 +591,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -593,7 +610,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -1005,6 +1023,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -1016,7 +1042,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -1418,6 +1445,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -1428,7 +1463,8 @@
               "os": "mac-intel-stable"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -1820,6 +1856,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -1830,7 +1874,8 @@
               "os": "mac-intel-stable"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -2214,6 +2259,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -2226,7 +2279,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -2644,6 +2698,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -2656,7 +2718,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -3134,6 +3197,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -3145,7 +3216,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -3757,6 +3829,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -3768,7 +3848,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         },
         "trigger_script": {
           "args": [

testing/buildbot/client.v8.fyi.json

@@ -131,7 +131,7 @@
           "--os-type",
           "android",
           "--build-revision",
-          "${got_revision}",
+          "${got_cr_revision}",
           "--test-machine-name",
           "${buildername}"
         ],
@@ -141,6 +141,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -152,7 +160,8 @@
               "os": "Android"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -449,7 +458,7 @@
           "--os-type",
           "linux",
           "--build-revision",
-          "${got_revision}",
+          "${got_cr_revision}",
           "--test-machine-name",
           "${buildername}"
         ],
@@ -459,6 +468,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -469,7 +486,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -761,7 +779,7 @@
           "--os-type",
           "linux",
           "--build-revision",
-          "${got_revision}",
+          "${got_cr_revision}",
           "--test-machine-name",
           "${buildername}"
         ],
@@ -771,6 +789,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -781,7 +807,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -1069,7 +1096,7 @@
           "--os-type",
           "mac",
           "--build-revision",
-          "${got_revision}",
+          "${got_cr_revision}",
           "--test-machine-name",
           "${buildername}"
         ],
@@ -1079,6 +1106,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -1088,7 +1123,8 @@
               "os": "mac-intel-stable"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {
@@ -1701,7 +1737,7 @@
           "--os-type",
           "win",
           "--build-revision",
-          "${got_revision}",
+          "${got_cr_revision}",
           "--test-machine-name",
           "${buildername}"
         ],
@@ -1711,6 +1747,14 @@
           "script": "//testing/merge_scripts/standard_isolated_script_merge.py"
         },
         "name": "maps_pixel_test",
+        "precommit_args": [
+          "--review-patch-issue",
+          "${patch_issue}",
+          "--review-patch-set",
+          "${patch_set}",
+          "--buildbucket-build-id",
+          "${buildbucket_build_id}"
+        ],
         "should_retry_with_patch": false,
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -1721,7 +1765,8 @@
               "pool": "Chrome-GPU"
             }
           ],
-          "idempotent": false
+          "idempotent": false,
+          "service_account": "chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com"
         }
       },
       {

testing/buildbot/test_suite_exceptions.pyl

@@ -778,6 +778,36 @@
       # chromium.gpu.fyi
       'Android FYI Release (Pixel 2)',
     ],
+    'replacements': {
+      # client.v8.fyi
+      # The V8 builders pass the V8 revision for ${got_revision}, so instead
+      # use ${got_cr_revision}, which is only set on the V8 bots.
+      'Android V8 FYI Release (Nexus 5X)': {
+        'args': {
+          '--build-revision': '${got_cr_revision}',
+        },
+      },
+      'Linux V8 FYI Release (NVIDIA)': {
+        'args': {
+          '--build-revision': '${got_cr_revision}',
+        },
+      },
+      'Linux V8 FYI Release - pointer compression (NVIDIA)': {
+        'args': {
+          '--build-revision': '${got_cr_revision}',
+        },
+      },
+      'Mac V8 FYI Release (Intel)': {
+        'args': {
+          '--build-revision': '${got_cr_revision}',
+        },
+      },
+      'Win V8 FYI Release (NVIDIA)': {
+        'args': {
+          '--build-revision': '${got_cr_revision}',
+        },
+      },
+    },
   },
   'mojo_unittests': {
     'modifications': {

testing/buildbot/test_suites.pyl

@@ -3783,6 +3783,20 @@
           '${buildername}',
           '--extra-browser-args=--enable-features=UseSkiaRenderer',
         ],
+        'precommit_args': [
+          # Gerrit issue ID
+          '--review-patch-issue',
+          '${patch_issue}',
+          # Patch set number
+          '--review-patch-set',
+          '${patch_set}',
+          # Buildbucket ID
+          '--buildbucket-build-id',
+          '${buildbucket_build_id}',
+        ],
+        'swarming': {
+          'service_account': 'chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com'
+        },
       },
       'screenshot_sync': {
         'args': [
@@ -3869,6 +3883,20 @@
           '--test-machine-name',
           '${buildername}',
         ],
+        'precommit_args': [
+          # Gerrit issue ID
+          '--review-patch-issue',
+          '${patch_issue}',
+          # Patch set number
+          '--review-patch-set',
+          '${patch_set}',
+          # Buildbucket ID
+          '--buildbucket-build-id',
+          '${buildbucket_build_id}',
+        ],
+        'swarming': {
+          'service_account': 'chrome-gpu-gold@chops-service-accounts.iam.gserviceaccount.com'
+        },
       },
       'pixel_skia': {
         'name': 'pixel_skia_gold_test',