diff --git a/engine/src/flutter/lib/web_ui/dev/steps/run_tests_step.dart b/engine/src/flutter/lib/web_ui/dev/steps/run_tests_step.dart index e06768573a2..43c16f2e2e5 100644 --- a/engine/src/flutter/lib/web_ui/dev/steps/run_tests_step.dart +++ b/engine/src/flutter/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 _createSkiaClient() async { final SkiaGoldClient skiaClient = SkiaGoldClient( environment.webUiSkiaGoldDirectory, - browserName: browserName, + dimensions: {'Browser': browserName}, ); if (await _checkSkiaClient(skiaClient)) { @@ -104,7 +104,7 @@ class RunTestsStep implements PipelineStep { Future _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; diff --git a/engine/src/flutter/lib/web_ui/dev/test_platform.dart b/engine/src/flutter/lib/web_ui/dev/test_platform.dart index 834787cb554..c104301817f 100644 --- a/engine/src/flutter/lib/web_ui/dev/test_platform.dart +++ b/engine/src/flutter/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'; diff --git a/engine/src/flutter/lib/web_ui/pubspec.yaml b/engine/src/flutter/lib/web_ui/pubspec.yaml index 06f48ff7240..b5627e62a7c 100644 --- a/engine/src/flutter/lib/web_ui/pubspec.yaml +++ b/engine/src/flutter/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 diff --git a/engine/src/flutter/testing/skia_gold_client/README.md b/engine/src/flutter/testing/skia_gold_client/README.md new file mode 100644 index 00000000000..c9d6a8ce564 --- /dev/null +++ b/engine/src/flutter/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: /testing/skia_gold_client +``` + +3. Use the client: + +```dart +import 'package:skia_gold_client/skia_gold_client.dart'; + +Future main() { + final Directory tmpDirectory = Directory.current.createTempSync('skia_gold_wd'); + final SkiaGoldClient client = SkiaGoldClient( + tmpDirectory, + dimensions: {'': ''}, + ); + + try { + if (isSkiaGoldClientAvailable) { + await client.auth(); + + await client.addImg( + '', + File('gold-file.png'), + screenshotSize: 1024, + ); + } + } catch (error) { + // Failed to authenticate or compare pixels. + stderr.write(error.toString()); + rethrow; + } finally { + tmpDirectory.deleteSync(recursive: true); + } +} +``` diff --git a/engine/src/flutter/web_sdk/web_test_utils/lib/skia_client.dart b/engine/src/flutter/testing/skia_gold_client/lib/skia_gold_client.dart similarity index 80% rename from engine/src/flutter/web_sdk/web_test_utils/lib/skia_client.dart rename to engine/src/flutter/testing/skia_gold_client/lib/skia_gold_client.dart index 025f509ddd1..83197c31483 100644 --- a/engine/src/flutter/web_sdk/web_test_utils/lib/skia_client.dart +++ b/engine/src/flutter/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? 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 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 imgtestAdd( + Future _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 tryjobAdd( + Future _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 _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 [ '--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 _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( - ['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( + ['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 _getKeys() { - return { - 'Browser': browserName, + final Map initialKeys = { 'CI': 'luci', 'Platform': Platform.operatingSystem, }; + if (dimensions != null) { + initialKeys.addAll(dimensions!); + } + return initialKeys; } /// Same as [_getKeys] but encodes it in a JSON string. diff --git a/engine/src/flutter/testing/skia_gold_client/pubspec.yaml b/engine/src/flutter/testing/skia_gold_client/pubspec.yaml new file mode 100644 index 00000000000..bd6d3082d7c --- /dev/null +++ b/engine/src/flutter/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 diff --git a/engine/src/flutter/web_sdk/web_test_utils/lib/image_compare.dart b/engine/src/flutter/web_sdk/web_test_utils/lib/image_compare.dart index b36a32cab0e..29de42db075 100644 --- a/engine/src/flutter/web_sdk/web_test_utils/lib/image_compare.dart +++ b/engine/src/flutter/web_sdk/web_test_utils/lib/image_compare.dart @@ -6,16 +6,10 @@ import 'dart:io'; import 'package:image/image.dart'; import 'package:path/path.dart' as p; +import 'package:skia_gold_client/skia_gold_client.dart'; import 'environment.dart'; import 'goldens.dart'; -import 'skia_client.dart'; - -/// Whether this code is running on LUCI. -bool _isLuci = Platform.environment.containsKey('SWARMING_TASK_ID') && Platform.environment.containsKey('GOLDCTL'); -bool _isPreSubmit = _isLuci && Platform.environment.containsKey('GOLD_TRYJOB'); -bool _isPostSubmit = _isLuci && !_isPreSubmit; - /// Compares a screenshot taken through a test with its golden. /// @@ -43,12 +37,26 @@ Future compareImage( await screenshotFile.create(recursive: true); await screenshotFile.writeAsBytes(encodePng(screenshot), flush: true); - if (_isLuci) { + if (isLuciEnv) { // This is temporary to get started by uploading existing screenshots to // Skia Gold. The next step would be to actually use Skia Gold for // comparison. final int screenshotSize = screenshot.width * screenshot.height; - await _uploadToSkiaGold(skiaClient, screenshotFile, screenshotSize, filename, isCanvaskitTest); + + late int pixelColorDelta; + if (isCanvaskitTest) { + pixelColorDelta = 21; + } else if (skiaClient.dimensions != null && skiaClient.dimensions!['Browser'] == 'ios-safari') { + pixelColorDelta = 15; + } else { + pixelColorDelta = 3; + } + skiaClient.addImg( + filename, + screenshotFile, + screenshotSize: screenshotSize, + pixelColorDelta: pixelColorDelta, + ); return 'OK'; } @@ -148,43 +156,3 @@ Future _getGolden(String filename) { String _getFullScreenshotPath(String filename) { return p.join(environment.webUiSkiaGoldDirectory.path, filename); } - -Future _uploadToSkiaGold( - SkiaGoldClient skiaClient, - File screenshotFile, - int screenshotSize, - String filename, - bool isCanvaskitTest, -) async { - // Can't upload to Gold Skia unless running in LUCI. - assert(_isLuci); - - if (_isPreSubmit) { - return _uploadInPreSubmit(skiaClient, filename, screenshotFile, screenshotSize, isCanvaskitTest); - } - if (_isPostSubmit) { - return _uploadInPostSubmit(skiaClient, filename, screenshotFile, screenshotSize, isCanvaskitTest); - } -} - -Future _uploadInPreSubmit( - SkiaGoldClient skiaClient, - String filename, - File screenshotFile, - int screenshotSize, - bool isCanvaskitTest, -) { - assert(_isPreSubmit); - return skiaClient.tryjobAdd(filename, screenshotFile, screenshotSize, isCanvaskitTest); -} - -Future _uploadInPostSubmit( - SkiaGoldClient skiaClient, - String filename, - File screenshotFile, - int screenshotSize, - bool isCanvaskitTest, -) { - assert(_isPostSubmit); - return skiaClient.imgtestAdd(filename, screenshotFile, screenshotSize, isCanvaskitTest); -} diff --git a/engine/src/flutter/web_sdk/web_test_utils/pubspec.yaml b/engine/src/flutter/web_sdk/web_test_utils/pubspec.yaml index 360966b4bc1..592663a1592 100644 --- a/engine/src/flutter/web_sdk/web_test_utils/pubspec.yaml +++ b/engine/src/flutter/web_sdk/web_test_utils/pubspec.yaml @@ -12,5 +12,7 @@ dependencies: meta: 1.3.0 path: 1.8.0 process: 4.2.3 + skia_gold_client: + path: ../../testing/skia_gold_client typed_data: 1.3.0 yaml: 3.0.0