Workaround Gold diff issues

Works around two separate issues in the Gold Python code caused by
`goldctl diff` misbehaving:

1. Creates a copy of the working directory to pass to `goldctl diff`, as
   it currently clobbers the auth files in the given working directory.
2. Uses the public version of the Gold instance for `goldctl diff`, as
   authentication does not currently work for the non-public instance.

Bug: skia:10610, skia:10611, skia:10587
Change-Id: Idff3202c048f4e7baa986cfc8ad21fb918382603
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416861
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Auto-Submit: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808197}

build/android/pylib/utils/gold_utils_test.py

@@ -47,10 +47,18 @@ class AndroidSkiaGoldSessionDiffTest(fake_filesystem_unittest.TestCase):
     call_args = cmd_mock.call_args[0][0]
     self.assertIn('diff', call_args)
     assertArgWith(self, call_args, '--corpus', 'corpus')
-    assertArgWith(self, call_args, '--instance', 'instance')
+    # TODO(skbug.com/10610): Remove the -public once we go back to using the
+    # non-public instance, or add a second test for testing that the correct
+    # instance is chosen if we decide to support both depending on what the
+    # user is authenticated for.
+    assertArgWith(self, call_args, '--instance', 'instance-public')
     assertArgWith(self, call_args, '--input', 'png_file')
     assertArgWith(self, call_args, '--test', 'name')
-    assertArgWith(self, call_args, '--work-dir', self._working_dir)
+    # TODO(skbug.com/10611): Re-add this assert and remove the check for the
+    # absence of the directory once we switch back to using the proper working
+    # directory.
+    # assertArgWith(self, call_args, '--work-dir', self._working_dir)
+    self.assertNotIn(self._working_dir, call_args)
     i = call_args.index('--out-dir')
     # The output directory should be a subdirectory of the working directory.
     self.assertIn(self._working_dir, call_args[i + 1])

build/skia_gold_common/skia_gold_session.py

@@ -5,6 +5,7 @@
 
 import logging
 import os
+import shutil
 import subprocess
 import sys
 import tempfile
@@ -325,25 +326,39 @@ class SkiaGoldSession(object):
           'tests locally.')
 
     output_dir = self._CreateDiffOutputDir()
-    diff_cmd = [
-        GOLDCTL_BINARY,
-        'diff',
-        '--corpus',
-        self._corpus,
-        '--instance',
-        self._instance,
-        '--input',
-        png_file,
-        '--test',
-        name,
-        '--work-dir',
-        self._working_dir,
-        '--out-dir',
-        output_dir,
-    ]
-    rc, stdout = self._RunCmdForRcAndOutput(diff_cmd)
-    self._StoreDiffLinks(name, output_manager, output_dir)
-    return rc, stdout
+    # TODO(skbug.com/10611): Remove this temporary work dir and instead just use
+    # self._working_dir once `goldctl diff` stops clobbering the auth files in
+    # the provided work directory.
+    temp_work_dir = tempfile.mkdtemp()
+    # shutil.copytree() fails if the destination already exists, so use a
+    # subdirectory of the temporary directory.
+    temp_work_dir = os.path.join(temp_work_dir, 'diff_work_dir')
+    try:
+      shutil.copytree(self._working_dir, temp_work_dir)
+      diff_cmd = [
+          GOLDCTL_BINARY,
+          'diff',
+          '--corpus',
+          self._corpus,
+          '--instance',
+          # TODO(skbug.com/10610): Decide whether to use the public or
+          # non-public instance once authentication is fixed for the non-public
+          # instance.
+          str(self._instance) + '-public',
+          '--input',
+          png_file,
+          '--test',
+          name,
+          '--work-dir',
+          temp_work_dir,
+          '--out-dir',
+          output_dir,
+      ]
+      rc, stdout = self._RunCmdForRcAndOutput(diff_cmd)
+      self._StoreDiffLinks(name, output_manager, output_dir)
+      return rc, stdout
+    finally:
+      shutil.rmtree(os.path.realpath(os.path.join(temp_work_dir, '..')))
 
   def GetTriageLinks(self, name):
     """Gets the triage links for the given image.

content/test/gpu/gpu_tests/skia_gold/gpu_skia_gold_session_unittest.py

@@ -48,10 +48,18 @@ class GpuSkiaGoldSessionDiffTest(fake_filesystem_unittest.TestCase):
     call_args = cmd_mock.call_args[0][0]
     self.assertIn('diff', call_args)
     assertArgWith(self, call_args, '--corpus', 'corpus')
-    assertArgWith(self, call_args, '--instance', 'instance')
+    # TODO(skbug.com/10610): Remove the -public once we go back to using the
+    # non-public instance, or add a second test for testing that the correct
+    # instance is chosen if we decide to support both depending on what the
+    # user is authenticated for.
+    assertArgWith(self, call_args, '--instance', 'instance-public')
     assertArgWith(self, call_args, '--input', 'png_file')
     assertArgWith(self, call_args, '--test', 'name')
-    assertArgWith(self, call_args, '--work-dir', self._working_dir)
+    # TODO(skbug.com/10611): Re-add this assert and remove the check for the
+    # absence of the directory once we switch back to using the proper working
+    # directory.
+    # assertArgWith(self, call_args, '--work-dir', self._working_dir)
+    self.assertNotIn(self._working_dir, call_args)
     i = call_args.index('--out-dir')
     # The output directory should not be a subdirectory of the working
     # directory.