Integrate Skia Gold with restricted trace tests.

src/tests/perf_tests/TracePerfTest.cpp

@@ -385,8 +385,15 @@ void TracePerfTest::saveScreenshot(const std::string &screenshotName)
         }
     }
 
-    angle::SavePNGRGB(screenshotName.c_str(), "ANGLE Screenshot", mTestParams.windowWidth,
-                      mTestParams.windowHeight, rgbData);
+    if (!angle::SavePNGRGB(screenshotName.c_str(), "ANGLE Screenshot", mTestParams.windowWidth,
+                           mTestParams.windowHeight, rgbData))
+    {
+        FAIL() << "Error saving screenshot: " << screenshotName;
+    }
+    else
+    {
+        printf("Saved screenshot: '%s'\n", screenshotName.c_str());
+    }
 
     // Finish the frame loop.
     for (uint32_t nextFrame = traceInfo.startFrame + 1; nextFrame <= traceInfo.endFrame;

src/tests/restricted_traces/BUILD.gn

@@ -64,12 +64,33 @@ angle_shared_library("angle_restricted_traces") {
   }
 }
 
+group("goldctl") {
+  data = []
+
+  # Because this links to a CIPD dependency, which is a symlink on Unix
+  # platforms, refer to the actual executable rather than the whole directory;
+  # copying the whole directory doesn't work, at least with "mb.py zip".
+  if (is_win) {
+    data += [ "//tools/skia_goldctl/win/goldctl.exe" ]
+  } else if (is_mac) {
+    data += [ "//tools/skia_goldctl/mac/goldctl" ]
+  } else {
+    data += [ "//tools/skia_goldctl/linux/goldctl" ]
+  }
+}
+
 group("angle_restricted_trace_gold_tests") {
   testonly = true
-  data_deps = [ "$angle_root/src/tests:angle_perftests" ]
+  data_deps = [
+    ":goldctl",
+    "$angle_root/src/tests:angle_perftests",
+    "$angle_root/src/tests:angle_system_info_test",
+  ]
   data = [
     "restricted_trace_gold_tests.py",
     "restricted_traces.json",
+    "skia_gold/",
+    "//build/skia_gold_common/",
     "//testing/scripts/common.py",
     "//testing/xvfb.py",
   ]

src/tests/restricted_traces/restricted_trace_gold_tests.py

@@ -23,13 +23,19 @@
 import argparse
 import contextlib
 import json
+import logging
 import os
+import platform
+import re
 import shutil
 import sys
 import tempfile
 import time
 import traceback
 
+from skia_gold import angle_skia_gold_properties
+from skia_gold import angle_skia_gold_session_manager
+
 # Add //src/testing into sys.path for importing xvfb and test_env, and
 # //src/testing/scripts for importing common.
 d = os.path.dirname
@@ -54,6 +60,16 @@ def IsWindows():
 
 DEFAULT_TEST_SUITE = 'angle_perftests'
 DEFAULT_TEST_PREFIX = '--gtest_filter=TracePerfTest.Run/vulkan_'
+DEFAULT_SCREENSHOT_PREFIX = 'angle_vulkan_'
+
+# Filters out stuff like: " I   72.572s run_tests_on_device(96071FFAZ00096) "
+ANDROID_LOGGING_PREFIX = r'I +\d+.\d+s \w+\(\w+\)  '
+ANDROID_BEGIN_SYSTEM_INFO = '>>ScopedMainEntryLogger'
+
+# Test expectations
+FAIL = 'FAIL'
+PASS = 'PASS'
+SKIP = 'SKIP'
 
 
 @contextlib.contextmanager
@@ -62,9 +78,249 @@ def temporary_dir(prefix=''):
     try:
         yield path
     finally:
+        logging.info("Removing temporary directory: %s" + path)
         shutil.rmtree(path)
 
 
+def add_skia_gold_args(parser):
+    group = parser.add_argument_group('Skia Gold Arguments')
+    group.add_argument('--git-revision', help='Revision being tested.', default=None)
+    group.add_argument(
+        '--gerrit-issue', help='For Skia Gold integration. Gerrit issue ID.', default='')
+    group.add_argument(
+        '--gerrit-patchset',
+        help='For Skia Gold integration. Gerrit patch set number.',
+        default='')
+    group.add_argument(
+        '--buildbucket-id', help='For Skia Gold integration. Buildbucket build ID.', default='')
+    group.add_argument(
+        '--bypass-skia-gold-functionality',
+        action='store_true',
+        default=False,
+        help='Bypass all interaction with Skia Gold, effectively disabling the '
+        'image comparison portion of any tests that use Gold. Only meant to '
+        'be used in case a Gold outage occurs and cannot be fixed quickly.')
+    local_group = group.add_mutually_exclusive_group()
+    local_group.add_argument(
+        '--local-pixel-tests',
+        action='store_true',
+        default=None,
+        help='Specifies to run the test harness in local run mode or not. When '
+        'run in local mode, uploading to Gold is disabled and links to '
+        'help with local debugging are output. Running in local mode also '
+        'implies --no-luci-auth. If both this and --no-local-pixel-tests are '
+        'left unset, the test harness will attempt to detect whether it is '
+        'running on a workstation or not and set this option accordingly.')
+    local_group.add_argument(
+        '--no-local-pixel-tests',
+        action='store_false',
+        dest='local_pixel_tests',
+        help='Specifies to run the test harness in non-local (bot) mode. When '
+        'run in this mode, data is actually uploaded to Gold and triage links '
+        'arge generated. If both this and --local-pixel-tests are left unset, '
+        'the test harness will attempt to detect whether it is running on a '
+        'workstation or not and set this option accordingly.')
+    group.add_argument(
+        '--no-luci-auth',
+        action='store_true',
+        default=False,
+        help='Don\'t use the service account provided by LUCI for '
+        'authentication for Skia Gold, instead relying on gsutil to be '
+        'pre-authenticated. Meant for testing locally instead of on the bots.')
+
+
+def run_wrapper(args, cmd, env, stdoutfile=None):
+    if args.xvfb:
+        return xvfb.run_executable(cmd, env, stdoutfile=stdoutfile)
+    else:
+        return test_env.run_command_with_output(cmd, env=env, stdoutfile=stdoutfile)
+
+
+def to_hex(num):
+    return hex(int(num))
+
+
+def to_hex_or_none(num):
+    return 'None' if num == None else to_hex(num)
+
+
+def to_non_empty_string_or_none(val):
+    return 'None' if val == '' else str(val)
+
+
+def to_non_empty_string_or_none_dict(d, key):
+    return 'None' if not key in d else to_non_empty_string_or_none(d[key])
+
+
+def get_binary_name(binary):
+    if IsWindows():
+        return '.\\%s.exe' % binary
+    else:
+        return './%s' % binary
+
+
+def get_skia_gold_keys(args):
+    """Get all the JSON metadata that will be passed to golctl."""
+    # All values need to be strings, otherwise goldctl fails.
+
+    # Only call this method one time
+    if hasattr(get_skia_gold_keys, 'called') and get_skia_gold_keys.called:
+        logging.exception('get_skia_gold_keys may only be called once')
+    get_skia_gold_keys.called = True
+
+    env = os.environ.copy()
+
+    class Filter:
+
+        def __init__(self):
+            self.accepting_lines = True
+            self.done_accepting_lines = False
+            self.android_prefix = re.compile(ANDROID_LOGGING_PREFIX)
+            self.lines = []
+            self.is_android = False
+
+        def append(self, line):
+            if self.done_accepting_lines:
+                return
+            if 'android/test_runner.py' in line:
+                self.accepting_lines = False
+                self.is_android = True
+            if ANDROID_BEGIN_SYSTEM_INFO in line:
+                self.accepting_lines = True
+                return
+            if not self.accepting_lines:
+                return
+
+            if self.is_android:
+                line = self.android_prefix.sub('', line)
+
+            if line[0] == '}':
+                self.done_accepting_lines = True
+
+            self.lines.append(line)
+
+        def get(self):
+            return self.lines
+
+    with common.temporary_file() as tempfile_path:
+        binary = get_binary_name('angle_system_info_test')
+        if run_wrapper(args, [binary, '--vulkan', '-v'], env, tempfile_path):
+            raise Exception('Error getting system info.')
+
+        filter = Filter()
+
+        with open(tempfile_path) as f:
+            for line in f:
+                filter.append(line)
+
+        str = ''.join(filter.get())
+        logging.info(str)
+        json_data = json.loads(str)
+
+    if len(json_data.get('gpus', [])) == 0 or not 'activeGPUIndex' in json_data:
+        raise Exception('Error getting system info.')
+
+    active_gpu = json_data['gpus'][json_data['activeGPUIndex']]
+
+    angle_keys = {
+        'vendor_id': to_hex_or_none(active_gpu['vendorId']),
+        'device_id': to_hex_or_none(active_gpu['deviceId']),
+        'model_name': to_non_empty_string_or_none_dict(active_gpu, 'machineModelVersion'),
+        'manufacturer_name': to_non_empty_string_or_none_dict(active_gpu, 'machineManufacturer'),
+        'os': to_non_empty_string_or_none(platform.system()),
+        'os_version': to_non_empty_string_or_none(platform.version()),
+        'driver_version': to_non_empty_string_or_none_dict(active_gpu, 'driverVersion'),
+        'driver_vendor': to_non_empty_string_or_none_dict(active_gpu, 'driverVendor'),
+    }
+
+    return angle_keys
+
+
+def output_diff_local_files(gold_session, image_name):
+    """Logs the local diff image files from the given SkiaGoldSession.
+
+  Args:
+    gold_session: A skia_gold_session.SkiaGoldSession instance to pull files
+        from.
+    image_name: A string containing the name of the image/test that was
+        compared.
+  """
+    given_file = gold_session.GetGivenImageLink(image_name)
+    closest_file = gold_session.GetClosestImageLink(image_name)
+    diff_file = gold_session.GetDiffImageLink(image_name)
+    failure_message = 'Unable to retrieve link'
+    logging.error('Generated image: %s', given_file or failure_message)
+    logging.error('Closest image: %s', closest_file or failure_message)
+    logging.error('Diff image: %s', diff_file or failure_message)
+
+
+def upload_test_result_to_skia_gold(args, gold_session_manager, gold_session, gold_properties,
+                                    screenshot_dir, image_name, artifacts):
+    """Compares the given image using Skia Gold and uploads the result.
+
+    No uploading is done if the test is being run in local run mode. Compares
+    the given screenshot to baselines provided by Gold, raising an Exception if
+    a match is not found.
+
+    Args:
+      args: Command line options.
+      gold_session_manager: Skia Gold session manager.
+      gold_session: Skia Gold session.
+      gold_properties: Skia Gold properties.
+      screenshot_dir: directory where the test stores screenshots.
+      image_name: the name of the image being checked.
+      artifacts: dictionary of JSON artifacts to pass to the result merger.
+    """
+
+    use_luci = not (gold_properties.local_pixel_tests or gold_properties.no_luci_auth)
+
+    # Note: this would be better done by iterating the screenshot directory.
+    png_file_name = os.path.join(screenshot_dir, DEFAULT_SCREENSHOT_PREFIX + image_name + '.png')
+
+    if not os.path.isfile(png_file_name):
+        logging.info('Screenshot not found, test skipped.')
+        return SKIP
+
+    status, error = gold_session.RunComparison(
+        name=image_name, png_file=png_file_name, use_luci=use_luci)
+
+    artifact_name = os.path.basename(png_file_name)
+    artifacts[artifact_name] = [artifact_name]
+
+    if not status:
+        return PASS
+
+    status_codes = gold_session_manager.GetSessionClass().StatusCodes
+    if status == status_codes.AUTH_FAILURE:
+        logging.error('Gold authentication failed with output %s', error)
+    elif status == status_codes.INIT_FAILURE:
+        logging.error('Gold initialization failed with output %s', error)
+    elif status == status_codes.COMPARISON_FAILURE_REMOTE:
+        _, triage_link = gold_session.GetTriageLinks(image_name)
+        if not triage_link:
+            logging.error('Failed to get triage link for %s, raw output: %s', image_name, error)
+            logging.error('Reason for no triage link: %s',
+                          gold_session.GetTriageLinkOmissionReason(image_name))
+        elif gold_properties.IsTryjobRun():
+            artifacts['triage_link_for_entire_cl'] = [triage_link]
+        else:
+            artifacts['gold_triage_link'] = [triage_link]
+    elif status == status_codes.COMPARISON_FAILURE_LOCAL:
+        logging.error('Local comparison failed. Local diff files:')
+        output_diff_local_files(gold_session, image_name)
+    elif status == status_codes.LOCAL_DIFF_FAILURE:
+        logging.error(
+            'Local comparison failed and an error occurred during diff '
+            'generation: %s', error)
+        # There might be some files, so try outputting them.
+        logging.error('Local diff files:')
+        output_diff_local_files(gold_session, image_name)
+    else:
+        logging.error('Given unhandled SkiaGoldSession StatusCode %s with error %s', status, error)
+
+    return FAIL
+
+
 def main():
     parser = argparse.ArgumentParser()
     parser.add_argument('--isolated-script-test-output', type=str, required=True)
@@ -73,11 +329,13 @@ def main():
     parser.add_argument('--render-test-output-dir', help='Directory to store screenshots')
     parser.add_argument('--xvfb', help='Start xvfb.', action='store_true')
 
+    add_skia_gold_args(parser)
+
     args, extra_flags = parser.parse_known_args()
     env = os.environ.copy()
 
     if 'GTEST_TOTAL_SHARDS' in env and int(env['GTEST_TOTAL_SHARDS']) != 1:
-        print('Sharding not yet implemented.')
+        logging.error('Sharding not yet implemented.')
         sys.exit(1)
 
     results = {
@@ -89,14 +347,23 @@ def main():
         'path_delimiter': '.',
         'version': 3,
         'num_failures_by_type': {
-            'PASS': 0,
-            'FAIL': 0,
-        }
+            FAIL: 0,
+            PASS: 0,
+            SKIP: 0,
+        },
     }
 
     result_tests = results['tests']['angle_restricted_trace_gold_tests']
 
     def run_tests(args, tests, extra_flags, env, screenshot_dir):
+        keys = get_skia_gold_keys(args)
+
+        with temporary_dir('angle_skia_gold_') as skia_gold_temp_dir:
+            gold_properties = angle_skia_gold_properties.ANGLESkiaGoldProperties(args)
+            gold_session_manager = angle_skia_gold_session_manager.ANGLESkiaGoldSessionManager(
+                skia_gold_temp_dir, gold_properties)
+            gold_session = gold_session_manager.GetSkiaGoldSession(keys)
+
             for test in tests['traces']:
                 with common.temporary_file() as tempfile_path:
                     cmd = [
@@ -104,20 +371,28 @@ def main():
                         DEFAULT_TEST_PREFIX + test,
                         '--render-test-output-dir=%s' % screenshot_dir,
                         '--one-frame-only',
+                        '--verbose-logging',
                     ] + extra_flags
 
-                if args.xvfb:
-                    rc = xvfb.run_executable(cmd, env, stdoutfile=tempfile_path)
-                else:
-                    rc = test_env.run_command_with_output(cmd, env=env, stdoutfile=tempfile_path)
+                    result = PASS if run_wrapper(args, cmd, env, tempfile_path) == 0 else FAIL
 
-                pass_fail = 'PASS' if rc == 0 else 'FAIL'
-                result_tests[test] = {'expected': 'PASS', 'actual': pass_fail}
-                results['num_failures_by_type'][pass_fail] += 1
+                    artifacts = {}
 
-        return results['num_failures_by_type']['FAIL'] == 0
+                    if result == PASS:
+                        result = upload_test_result_to_skia_gold(args, gold_session_manager,
+                                                                 gold_session, gold_properties,
+                                                                 screenshot_dir, test, artifacts)
+
+                    expected_result = SKIP if result == SKIP else PASS
+                    result_tests[test] = {'expected': expected_result, 'actual': result}
+                    if len(artifacts) > 0:
+                        result_tests[test]['artifacts'] = artifacts
+                    results['num_failures_by_type'][result] += 1
+
+            return results['num_failures_by_type'][FAIL] == 0
 
     rc = 0
+
     try:
         if IsWindows():
             args.test_suite = '.\\%s.exe' % args.test_suite
@@ -133,6 +408,9 @@ def main():
         if args.render_test_output_dir:
             if not run_tests(args, tests, extra_flags, env, args.render_test_output_dir):
                 rc = 1
+        elif 'ISOLATED_OUTDIR' in env:
+            if not run_tests(args, tests, extra_flags, env, env['ISOLATED_OUTDIR']):
+                rc = 1
         else:
             with temporary_dir('angle_trace_') as temp_dir:
                 if not run_tests(args, tests, extra_flags, env, temp_dir):
@@ -144,7 +422,7 @@ def main():
 
     if args.isolated_script_test_output:
         with open(args.isolated_script_test_output, 'w') as out_file:
-            out_file.write(json.dumps(results))
+            out_file.write(json.dumps(results, indent=2))
 
     if args.isolated_script_test_perf_output:
         with open(args.isolated_script_test_perf_output, 'w') as out_file:

src/tests/restricted_traces/skia_gold/__init__.py

+# Copyright 2020 The ANGLE Project Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+import sys
+
+d = os.path.dirname
+THIS_DIR = d(os.path.abspath(__file__))
+ANGLE_SRC_DIR = d(d(d(d(THIS_DIR))))
+sys.path.insert(0, os.path.join(ANGLE_SRC_DIR, 'build'))
+CHROMIUM_SRC_DIR = d(d(ANGLE_SRC_DIR))
+sys.path.insert(0, os.path.join(CHROMIUM_SRC_DIR, 'build'))

src/tests/restricted_traces/skia_gold/angle_skia_gold_properties.py

+# Copyright 2020 The ANGLE Project Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""ANGLE implementation of //build/skia_gold_common/skia_gold_properties.py."""
+
+import subprocess
+from skia_gold_common import skia_gold_properties
+
+
+class ANGLESkiaGoldProperties(skia_gold_properties.SkiaGoldProperties):
+
+    @staticmethod
+    def _GetGitOriginMasterHeadSha1():
+        try:
+            return subprocess.check_output(['git', 'rev-parse', 'origin/master'],
+                                           shell=_IsWin(),
+                                           cwd=ANGLE_SRC_DIR).strip()
+        except subprocess.CalledProcessError:
+            return None
+
+
+def _IsWin():
+    return sys.platform == 'win32'

src/tests/restricted_traces/skia_gold/angle_skia_gold_session.py

+# Copyright 2020 The ANGLE Project Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""ANGLE implementation of //build/skia_gold_common/skia_gold_session.py."""
+
+from skia_gold_common import output_managerless_skia_gold_session as omsgs
+
+
+class ANGLESkiaGoldSession(omsgs.OutputManagerlessSkiaGoldSession):
+
+    def _GetDiffGoldInstance(self):
+        return str(self._instance)

src/tests/restricted_traces/skia_gold/angle_skia_gold_session_manager.py

+# Copyright 2020 The ANGLE Project Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+"""ANGLE implementation of //build/skia_gold_common/skia_gold_session_manager.py."""
+
+from skia_gold_common import skia_gold_session_manager as sgsm
+import angle_skia_gold_session
+
+
+class ANGLESkiaGoldSessionManager(sgsm.SkiaGoldSessionManager):
+
+    @staticmethod
+    def _GetDefaultInstance():
+        return 'angle'
+
+    @staticmethod
+    def GetSessionClass():
+        return angle_skia_gold_session.ANGLESkiaGoldSession