Switch Skia Gold tests to use mixin

Switches all tests that use Skia Gold to use a mixin instead of individually specifying arguments in each test definition. As a prerequisite, also ensures that all test types that use Skia Gold use the same naming for Gold-related arguments. This is requisite work for adding a universal Gold kill switch in the unlikely event of an outage that can't be fixed quickly, as the switch can just be added to the mixin. Bug: 1057848 Change-Id: Ib57480f374eab088ac3a389637bfbff1ee41e8b4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2121419 Commit-Queue: Brian Sheedy <bsheedy@chromium.org> Reviewed-by: Sven Zheng <svenzheng@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Kenneth Russell <kbr@chromium.org> Auto-Submit: Brian Sheedy <bsheedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#755521}
Denger-Network · Apr 1, 2020 · 4d335de · 4d335de
1 parent 0f82c52
commit 4d335de
Show file tree

Hide file tree

Showing 16 changed files with 1,442 additions and 1,511 deletions.
diff --git a/chrome/test/pixel/browser_skia_gold_pixel_diff_unittest.cc b/chrome/test/pixel/browser_skia_gold_pixel_diff_unittest.cc
@@ -42,7 +42,7 @@ class BrowserSkiaGoldPixelDiffTest : public views::test::WidgetTest {
  public:
   BrowserSkiaGoldPixelDiffTest() {
     auto* cmd_line = base::CommandLine::ForCurrentProcess();
-    cmd_line->AppendSwitchASCII("build-revision", "test");
+    cmd_line->AppendSwitchASCII("git-revision", "test");
   }
 
  private:

diff --git a/content/test/gpu/gpu_tests/skia_gold_integration_test_base.py b/content/test/gpu/gpu_tests/skia_gold_integration_test_base.py
@@ -77,7 +77,7 @@ class SkiaGoldIntegrationTestBase(gpu_integration_test.GpuIntegrationTest):
   _skia_gold_temp_dir = None
 
   _local_run = None
-  _build_revision = None
+  _git_revision = None
 
   @classmethod
   def SetParsedCommandLineOptions(cls, options):
@@ -123,7 +123,7 @@ def TearDownProcess(cls):
   def AddCommandlineArgs(cls, parser):
     super(SkiaGoldIntegrationTestBase, cls).AddCommandlineArgs(parser)
     parser.add_option(
-      '--build-revision',
+      '--git-revision',
       help='Chrome revision being tested.',
       default=None)
     parser.add_option(
@@ -140,15 +140,15 @@ def AddCommandlineArgs(cls, parser):
       'profile after the test completes; leave the system using the sRGB color '
       'profile. See http://crbug.com/784456.')
     parser.add_option(
-      '--review-patch-issue',
+      '--gerrit-issue',
       help='For Skia Gold integration. Gerrit issue ID.',
       default='')
     parser.add_option(
-      '--review-patch-set',
+      '--gerrit-patchset',
       help='For Skia Gold integration. Gerrit patch set number.',
       default='')
     parser.add_option(
-      '--buildbucket-build-id',
+      '--buildbucket-id',
       help='For Skia Gold integration. Buildbucket build ID.',
       default='')
     parser.add_option(
@@ -335,16 +335,15 @@ def _GetBuildIdArgs(self):
       '--commit',
       self._GetBuildRevision(),
     ]
-    # If --review-patch-issue is passed, then we assume we're running on a
-    # trybot.
-    if parsed_options.review_patch_issue:
+    # If --gerrit-issue is passed, then we assume we're running on a trybot.
+    if parsed_options.gerrit_issue:
       build_id_args += [
         '--issue',
-        parsed_options.review_patch_issue,
+        parsed_options.gerrit_issue,
         '--patchset',
-        parsed_options.review_patch_set,
+        parsed_options.gerrit_patchset,
         '--jobid',
-        parsed_options.buildbucket_build_id,
+        parsed_options.buildbucket_id,
         '--crs',
         'gerrit',
         '--cis',
@@ -439,10 +438,10 @@ def _UploadTestResultToSkiaGold(self, image_name, screenshot,
       # If we're running on a trybot, instead generate a link to all results
       # for the CL so that the user can visit a single page instead of
       # clicking on multiple links on potentially multiple bots.
-      elif parsed_options.review_patch_issue:
+      elif parsed_options.gerrit_issue:
         cl_images = ('https://%s-gold.skia.org/search?'
                      'issue=%s&new_clstore=true' % (
-                       SKIA_GOLD_INSTANCE, parsed_options.review_patch_issue))
+                       SKIA_GOLD_INSTANCE, parsed_options.gerrit_issue))
         self.artifacts.CreateLink('triage_link_for_entire_cl', cl_images)
       else:
         try:
@@ -503,7 +502,7 @@ def _ShouldReportGoldFailure(self, page):
     # grace period. However, fail if we're on a trybot so that as many images
     # can be triaged as possible before a new test is committed.
     if (page.grace_period_end and date.today() <= page.grace_period_end and
-        not parsed_options.review_patch_issue):
+        not parsed_options.gerrit_issue):
       return False
     return True
 
@@ -570,25 +569,25 @@ def _IsLocalRun(cls):
   @classmethod
   def _GetBuildRevision(cls):
     """Returns the current git master revision being tested."""
-    # Do nothing if we've already determined the build revision.
-    if cls._build_revision is not None:
+    # Do nothing if we've already determined the git revision.
+    if cls._git_revision is not None:
       pass
-    # use the --build-revision value if it's been set.
-    elif cls.GetParsedCommandLineOptions().build_revision:
-      cls._build_revision = cls.GetParsedCommandLineOptions().build_revision
+    # use the --git-revision value if it's been set.
+    elif cls.GetParsedCommandLineOptions().git_revision:
+      cls._git_revision = cls.GetParsedCommandLineOptions().git_revision
     # Try to determine what revision we're on using git.
     else:
       try:
-        cls._build_revision = subprocess.check_output(
+        cls._git_revision = subprocess.check_output(
             ['git', 'rev-parse', 'origin/master'],
             shell=IsWin(),
             cwd=path_util.GetChromiumSrcDir()).strip()
-        logging.warning('Automatically determined build revision to be %s',
-            cls._build_revision)
+        logging.warning('Automatically determined git revision to be %s',
+            cls._git_revision)
       except subprocess.CalledProcessError:
-        raise Exception('--build-revision not passed, and unable to '
+        raise Exception('--git-revision not passed, and unable to '
                         'determine revision using git')
-    return cls._build_revision
+    return cls._git_revision
 
   @classmethod
   def GenerateGpuTests(cls, options):

diff --git a/docs/gpu/gpu_testing.md b/docs/gpu/gpu_testing.md
@@ -296,12 +296,12 @@ Example usage:
 --passthrough`
 
 If, for some reason, the local run code is unable to determine what the git
-revision is, simply pass '--build-revision aabbccdd'. Note that `aabbccdd` must
+revision is, simply pass `--git-revision aabbccdd`. Note that `aabbccdd` must
 be replaced with an actual Chromium src revision (typically whatever revision
 origin/master is currently synced to) in order for the tests to work. This can
 be done automatically using:
 ``run_gpu_integration_test.py pixel --no-skia-gold-failure --local-run
---passthrough --build-revision `git rev-parse origin/master` ``
+--passthrough --git-revision `git rev-parse origin/master` ``
 
 ## Running Binaries from the Bots Locally
 

diff --git a/testing/buildbot/chromium.android.json b/testing/buildbot/chromium.android.json
@@ -21229,10 +21229,10 @@
           "--shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json",
           "--replace-system-package=com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk",
           "--additional-apk=//third_party/gvr-android-sdk/test-apks/vr_keyboard/vr_keyboard_current.apk",
-          "--git-revision",
-          "${got_revision}",
           "--gs-results-bucket=chromium-result-details",
-          "--recover-devices"
+          "--recover-devices",
+          "--git-revision",
+          "${got_revision}"
         ],
         "merge": {
           "args": [
@@ -21621,10 +21621,10 @@
           "--shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json",
           "--replace-system-package=com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk",
           "--additional-apk=//third_party/gvr-android-sdk/test-apks/vr_keyboard/vr_keyboard_current.apk",
-          "--git-revision",
-          "${got_revision}",
           "--gs-results-bucket=chromium-result-details",
-          "--recover-devices"
+          "--recover-devices",
+          "--git-revision",
+          "${got_revision}"
         ],
         "merge": {
           "args": [
@@ -37207,10 +37207,10 @@
           "--shared-prefs-file=//chrome/android/shared_preference_files/test/vr_ddview_skipdon_setupcomplete.json",
           "--replace-system-package=com.google.vr.vrcore,//third_party/gvr-android-sdk/test-apks/vr_services/vr_services_current.apk",
           "--additional-apk=//third_party/gvr-android-sdk/test-apks/vr_keyboard/vr_keyboard_current.apk",
-          "--git-revision",
-          "${got_revision}",
           "--gs-results-bucket=chromium-result-details",
-          "--recover-devices"
+          "--recover-devices",
+          "--git-revision",
+          "${got_revision}"
         ],
         "merge": {
           "args": [