Move Skia gold client to shared location (flutter#33672)

Author: Emmanuel Garcia

lib/web_ui/dev/steps/run_tests_step.dart

@@ -7,6 +7,7 @@ import 'dart:io' as io;
 import 'package:path/path.dart' as pathlib;
 // TODO(yjbanov): remove hacks when this is fixed:
 //                https://github.com/dart-lang/test/issues/1521
+import 'package:skia_gold_client/skia_gold_client.dart';
 import 'package:test_api/src/backend/group.dart' as hack;
 import 'package:test_api/src/backend/live_test.dart' as hack;
 import 'package:test_api/src/backend/runtime.dart' as hack;
@@ -15,7 +16,6 @@ import 'package:test_core/src/runner/configuration/reporters.dart' as hack;
 import 'package:test_core/src/runner/engine.dart' as hack;
 import 'package:test_core/src/runner/hack_register_platform.dart' as hack;
 import 'package:test_core/src/runner/reporter.dart' as hack;
-import 'package:web_test_utils/skia_client.dart';
 
 import '../browser.dart';
 import '../common.dart';
@@ -86,7 +86,7 @@ class RunTestsStep implements PipelineStep {
   Future<SkiaGoldClient?> _createSkiaClient() async {
     final SkiaGoldClient skiaClient = SkiaGoldClient(
       environment.webUiSkiaGoldDirectory,
-      browserName: browserName,
+      dimensions: <String, String> {'Browser': browserName},
     );
 
     if (await _checkSkiaClient(skiaClient)) {
@@ -104,7 +104,7 @@ class RunTestsStep implements PipelineStep {
   Future<bool> _checkSkiaClient(SkiaGoldClient skiaClient) async {
     // Now let's check whether Skia Gold is reachable or not.
     if (isLuci) {
-      if (SkiaGoldClient.isAvailable) {
+      if (isSkiaGoldClientAvailable) {
         try {
           await skiaClient.auth();
           return true;

lib/web_ui/dev/test_platform.dart

@@ -19,6 +19,7 @@ import 'package:shelf/shelf_io.dart' as shelf_io;
 import 'package:shelf_packages_handler/shelf_packages_handler.dart';
 import 'package:shelf_static/shelf_static.dart';
 import 'package:shelf_web_socket/shelf_web_socket.dart';
+import 'package:skia_gold_client/skia_gold_client.dart';
 import 'package:stream_channel/stream_channel.dart';
 
 import 'package:test_api/src/backend/runtime.dart';
@@ -35,7 +36,6 @@ import 'package:test_core/src/util/stack_trace_mapper.dart';
 import 'package:web_socket_channel/web_socket_channel.dart';
 import 'package:web_test_utils/goldens.dart';
 import 'package:web_test_utils/image_compare.dart';
-import 'package:web_test_utils/skia_client.dart';
 
 import 'browser.dart';
 import 'common.dart';

lib/web_ui/pubspec.yaml

@@ -28,3 +28,5 @@ dev_dependencies:
       url: https://github.com/flutter/web_installers.git
       path: packages/simulators/
       ref: 9afed28b771da1c4e82a3382c4a2b31344c04522
+  skia_gold_client:
+    path: ../../testing/skia_gold_client

testing/skia_gold_client/README.md

@@ -0,0 +1,56 @@
+# skia_gold_client
+
+This package allows to create a Skia gold client in the engine repo.
+
+The client uses the `goldctl` tool on LUCI builders to upload screenshots,
+and verify if a new screenshot matches the baseline screenshot.
+
+The web UI is available on https://flutter-engine-gold.skia.org/.
+
+## Using the client
+
+1. In `.ci.yaml`, ensure that the task has a dependency on `goldctl`:
+
+```yaml
+  dependencies: [{"dependency": "goldctl"}]
+```
+
+2. Add dependency in `pubspec.yaml`:
+
+```yaml
+dependencies:
+  skia_gold_client:
+    path: <relative-path>/testing/skia_gold_client
+```
+
+3. Use the client:
+
+```dart
+import 'package:skia_gold_client/skia_gold_client.dart';
+
+Future<void> main() {
+    final Directory tmpDirectory = Directory.current.createTempSync('skia_gold_wd');
+    final SkiaGoldClient client = SkiaGoldClient(
+        tmpDirectory,
+        dimensions: <String, String> {'<attribute-name>': '<attribute-value>'},
+    );
+
+    try {
+        if (isSkiaGoldClientAvailable) {
+            await client.auth();
+
+            await client.addImg(
+                '<file-name>',
+                File('gold-file.png'),
+                screenshotSize: 1024,
+            );
+        }
+    } catch (error) {
+        // Failed to authenticate or compare pixels.
+        stderr.write(error.toString());
+        rethrow;
+    } finally {
+        tmpDirectory.deleteSync(recursive: true);
+    }
+}
+```

web_sdk/web_test_utils/lib/skia_client.dart renamed to testing/skia_gold_client/lib/skia_gold_client.dart

@@ -9,35 +9,37 @@ import 'package:crypto/crypto.dart';
 import 'package:path/path.dart' as path;
 import 'package:process/process.dart';
 
-import 'environment.dart';
-
 const String _kGoldctlKey = 'GOLDCTL';
+const String _kPresubmitEnvName = 'GOLD_TRYJOB';
+const String _kLuciEnvName = 'LUCI_CONTEXT';
 
 const String _skiaGoldHost = 'https://flutter-engine-gold.skia.org';
 const String _instance = 'flutter-engine';
 
-/// The percentage of accepted pixels to be wrong.
-///
-/// This should be a double between 0.0 and 1.0. A value of 0.0 means we don't
-/// accept any pixel to be different. A value of 1.0 means we accept 100% of
-/// pixels to be different.
-const double kMaxDifferentPixelsRate = 0.1;
+/// Whether the Skia Gold client is available and can be used in this
+/// environment.
+bool get isSkiaGoldClientAvailable => Platform.environment.containsKey(_kGoldctlKey);
+
+/// Returns true if the current environment is a LUCI builder.
+bool get isLuciEnv => Platform.environment.containsKey(_kLuciEnvName);
+
+/// Whether the current task is run during a presubmit check.
+bool get _isPresubmit => isLuciEnv && isSkiaGoldClientAvailable && Platform.environment.containsKey(_kPresubmitEnvName);
+
+/// Whether the current task is run during a postsubmit check.
+bool get _isPostsubmit => isLuciEnv && isSkiaGoldClientAvailable && !Platform.environment.containsKey(_kPresubmitEnvName);
 
 /// A client for uploading image tests and making baseline requests to the
 /// Flutter Gold Dashboard.
 class SkiaGoldClient {
   /// Creates a [SkiaGoldClient] with the given [workDirectory].
   ///
-  /// The [browserName] parameter is the name of the browser that generated the
-  /// screenshots.
-  SkiaGoldClient(this.workDirectory, { required this.browserName });
+  /// [dimensions] allows to add attributes about the environment
+  /// used to generate the screenshots.
+  SkiaGoldClient(this.workDirectory, { this.dimensions });
 
-  /// Whether the Skia Gold client is available and can be used in this
-  /// environment.
-  static bool get isAvailable => Platform.environment.containsKey(_kGoldctlKey);
-
-  /// The name of the browser running the tests.
-  final String browserName;
+  /// Allows to add attributes about the environment used to generate the screenshots.
+  final Map<String, String>? dimensions;
 
   /// A controller for launching sub-processes.
   final ProcessManager process = const LocalProcessManager();
@@ -73,7 +75,7 @@ class SkiaGoldClient {
   /// The path to the local [Directory] where the `goldctl` tool is hosted.
   String get _goldctl {
     assert(
-      isAvailable,
+      isSkiaGoldClientAvailable,
       'Trying to use `goldctl` in an environment where it is not available',
     );
     return Platform.environment[_kGoldctlKey]!;
@@ -165,6 +167,42 @@ class SkiaGoldClient {
     _isInitialized = true;
   }
 
+  /// Executes the `imgtest add` command in the `goldctl` tool.
+  ///
+  /// The `imgtest` command collects and uploads test results to the Skia Gold
+  /// backend, the `add` argument uploads the current image test.
+  ///
+  /// Throws an exception for try jobs that failed to pass the pixel comparison.
+  ///
+  /// The [testName] and [goldenFile] parameters reference the current
+  /// comparison being evaluated.
+  ///
+  /// [pixelColorDelta] defines maximum acceptable difference in RGB channels of each pixel,
+  /// such that:
+  ///
+  /// ```
+  /// abs(r(image) - r(golden)) + abs(g(image) - g(golden)) + abs(b(image) - b(golden)) <= pixelDeltaThreshold
+  /// ```
+  ///
+  /// [differentPixelsRate] is the fraction of accepted pixels to be wrong in the range [0.0, 1.0].
+  /// Defaults to 0.1. A value of 0.1 means that 10% of the pixels are allowed to change.
+  Future<void> addImg(
+    String testName,
+    File goldenFile, {
+    double differentPixelsRate = 0.1,
+    int pixelColorDelta = 0,
+    required int screenshotSize,
+  }) async {
+    assert(_isPresubmit || _isPostsubmit);
+
+    if (_isPresubmit) {
+      await _tryjobAdd(testName, goldenFile, screenshotSize, pixelColorDelta, differentPixelsRate);
+    }
+    if (_isPostsubmit) {
+      await _imgtestAdd(testName, goldenFile, screenshotSize, pixelColorDelta, differentPixelsRate);
+    }
+  }
+
   /// Executes the `imgtest add` command in the `goldctl` tool.
   ///
   /// The `imgtest` command collects and uploads test results to the Skia Gold
@@ -174,11 +212,12 @@ class SkiaGoldClient {
   ///
   /// The [testName] and [goldenFile] parameters reference the current
   /// comparison being evaluated.
-  Future<bool> imgtestAdd(
+  Future<void> _imgtestAdd(
     String testName,
     File goldenFile,
     int screenshotSize,
-    bool isCanvaskitTest,
+    int pixelDeltaThreshold,
+    double maxDifferentPixelsRate,
   ) async {
     await _imgtestInit();
 
@@ -188,7 +227,7 @@ class SkiaGoldClient {
       '--work-dir', _tempPath,
       '--test-name', cleanTestName(testName),
       '--png-file', goldenFile.path,
-      ..._getMatchingArguments(testName, screenshotSize, isCanvaskitTest),
+      ..._getMatchingArguments(testName, screenshotSize, pixelDeltaThreshold, maxDifferentPixelsRate),
     ];
 
     final ProcessResult result = await _runCommand(imgtestCommand);
@@ -200,8 +239,6 @@ class SkiaGoldClient {
       print('goldctl imgtest add stdout: ${result.stdout}');
       print('goldctl imgtest add stderr: ${result.stderr}');
     }
-
-    return true;
   }
 
   /// Executes the `imgtest init` command in the `goldctl` tool for tryjobs.
@@ -268,11 +305,12 @@ class SkiaGoldClient {
   ///
   /// The [testName] and [goldenFile] parameters reference the current
   /// comparison being evaluated.
-  Future<void> tryjobAdd(
+  Future<void> _tryjobAdd(
     String testName,
     File goldenFile,
     int screenshotSize,
-    bool isCanvaskitTest,
+    int pixelDeltaThreshold,
+    double differentPixelsRate,
   ) async {
     await _tryjobInit();
 
@@ -282,7 +320,7 @@ class SkiaGoldClient {
       '--work-dir', _tempPath,
       '--test-name', cleanTestName(testName),
       '--png-file', goldenFile.path,
-      ..._getMatchingArguments(testName, screenshotSize, isCanvaskitTest),
+      ..._getMatchingArguments(testName, screenshotSize, pixelDeltaThreshold, differentPixelsRate),
     ];
 
     final ProcessResult result = await _runCommand(tryjobCommand);
@@ -306,7 +344,8 @@ class SkiaGoldClient {
   List<String> _getMatchingArguments(
     String testName,
     int screenshotSize,
-    bool isCanvaskitTest,
+    int pixelDeltaThreshold,
+    double differentPixelsRate,
   ) {
     // The algorithm to be used when matching images. The available options are:
     // - "fuzzy": Allows for customizing the thresholds of pixel differences.
@@ -318,22 +357,7 @@ class SkiaGoldClient {
     // baseline. It's okay for this to be a slightly high number like 10% of the
     // image size because those wrong pixels are constrained by
     // `pixelDeltaThreshold` below.
-    final int maxDifferentPixels = (screenshotSize * kMaxDifferentPixelsRate).toInt();
-
-    // The maximum acceptable difference in RGB channels of each pixel.
-    //
-    // ```
-    // abs(r(image) - r(golden)) + abs(g(image) - g(golden)) + abs(b(image) - b(golden)) <= pixelDeltaThreshold
-    // ```
-    final String pixelDeltaThreshold;
-    if (isCanvaskitTest) {
-      pixelDeltaThreshold = '21';
-    } else if (browserName == 'ios-safari') {
-      pixelDeltaThreshold = '15';
-    } else {
-      pixelDeltaThreshold = '3';
-    }
-
+    final int maxDifferentPixels = (screenshotSize * differentPixelsRate).toInt();
     return <String>[
       '--add-test-optional-key', 'image_matching_algorithm:$algorithm',
       '--add-test-optional-key', 'fuzzy_max_different_pixels:$maxDifferentPixels',
@@ -396,19 +420,15 @@ class SkiaGoldClient {
 
   /// Returns the current commit hash of the engine repository.
   Future<String> _getCurrentCommit() async {
-    final Directory webUiRoot = environment.webUiRootDir;
-    if (!webUiRoot.existsSync()) {
-      throw Exception('Web Engine root could not be found: $webUiRoot\n');
-    } else {
-      final ProcessResult revParse = await process.run(
-        <String>['git', 'rev-parse', 'HEAD'],
-        workingDirectory: webUiRoot.path,
-      );
-      if (revParse.exitCode != 0) {
-        throw Exception('Current commit of Web Engine can not be found.');
-      }
-      return (revParse.stdout as String).trim();
+    final File currentScript = File.fromUri(Platform.script);
+    final ProcessResult revParse = await process.run(
+      <String>['git', 'rev-parse', 'HEAD'],
+      workingDirectory: currentScript.path,
+    );
+    if (revParse.exitCode != 0) {
+      throw Exception('Current commit of the engine can not be found from path ${currentScript.path}.');
     }
+    return (revParse.stdout as String).trim();
   }
 
   /// Returns a Map of key value pairs used to uniquely identify the
@@ -417,11 +437,14 @@ class SkiaGoldClient {
   /// Currently, the only key value pairs being tracked are the platform and
   /// browser the image was rendered on.
   Map<String, dynamic> _getKeys() {
-    return <String, dynamic>{
-      'Browser': browserName,
+    final Map<String, dynamic> initialKeys = <String, dynamic>{
       'CI': 'luci',
       'Platform': Platform.operatingSystem,
     };
+    if (dimensions != null) {
+      initialKeys.addAll(dimensions!);
+    }
+    return initialKeys;
   }
 
   /// Same as [_getKeys] but encodes it in a JSON string.

testing/skia_gold_client/pubspec.yaml

@@ -0,0 +1,38 @@
+# Copyright 2013 The Flutter Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+name: skia_gold_client
+publish_to: none
+environment:
+  sdk: '>=2.12.0 <3.0.0'
+
+# Do not add any dependencies that require more than what is provided in
+# //third_party/dart/pkg, //third_party/dart/third_party/pkg, or
+# //third_party/pkg. In particular, package:test is not usable here.
+
+# If you do add packages here, make sure you can run `pub get --offline`, and
+# check the .packages and .package_config to make sure all the paths are
+# relative to this directory into //third_party/dart, or //third_party/pkg
+dependencies:
+  crypto: any
+  path: any
+  process: any
+
+dependency_overrides:
+  collection:
+    path: ../../../third_party/dart/third_party/pkg/collection
+  crypto:
+    path: ../../../third_party/dart/third_party/pkg/crypto
+  file:
+    path: ../../../third_party/pkg/file/packages/file
+  meta:
+    path: ../../../third_party/dart/pkg/meta
+  path:
+    path: ../../../third_party/dart/third_party/pkg/path
+  platform:
+    path: ../../../third_party/pkg/platform
+  process:
+    path: ../../../third_party/pkg/process
+  typed_data:
+    path: ../../../third_party/dart/third_party/pkg/typed_data