Add a basic e2e-test for `skia_gold_client`. (#51078) Basically, I take blank images of red/blue/green squares, like this:  ... and use Image Magick to annotate it with the current git hash: ```sh $ drt ./testing/skia_gold_client/tool/generate.dart Writing annotation "8069cb4ca1" on images in testing/skia_gold_client/tool/source_images and saving them in testing/skia_gold_client/tool/e2e_fixtures. Writing to testing/skia_gold_client/tool/e2e_fixtures/solid_red_square.png Writing to testing/skia_gold_client/tool/e2e_fixtures/solid_green_square.png Writing to testing/skia_gold_client/tool/e2e_fixtures/solid_blue_square.png Done: wrote 3 image. ```  ... then, I upload the digests very similar to how we do it in `scenario_app`. --- The idea here is to have a way for me to know if Skia gold is working the way I/we expect, independent of a more complicated test suite with various race-y or flake-y conditions. We could also augment it with tests of "dimensions" properties.
diff --git a/ci/builders/linux_android_emulator.json b/ci/builders/linux_android_emulator.json index 305608f..918c8e6 100644 --- a/ci/builders/linux_android_emulator.json +++ b/ci/builders/linux_android_emulator.json
@@ -60,6 +60,12 @@ }, { "language": "dart", + "name": "skia_gold_client/e2e_test", + "script": "flutter/testing/skia_gold_client/tool/e2e_test.dart", + "max_attempts": 1 + }, + { + "language": "dart", "name": "Android Scenario App Integration Tests (Impeller/Vulkan)", "test_timeout_secs": 900, "max_attempts": 2,
diff --git a/testing/skia_gold_client/pubspec.yaml b/testing/skia_gold_client/pubspec.yaml index a7ba15d..d119b11 100644 --- a/testing/skia_gold_client/pubspec.yaml +++ b/testing/skia_gold_client/pubspec.yaml
@@ -22,10 +22,16 @@ process: any dev_dependencies: + args: any + git_repo_tools: any litetest: any process_fakes: any dependency_overrides: + args: + path: ../../../third_party/dart/third_party/pkg/args + async: + path: ../../../third_party/dart/third_party/pkg/async async_helper: path: ../../../third_party/dart/pkg/async_helper collection: @@ -38,6 +44,8 @@ path: ../../../third_party/dart/pkg/expect file: path: ../../../third_party/dart/third_party/pkg/file/packages/file + git_repo_tools: + path: ../../tools/pkg/git_repo_tools litetest: path: ../../testing/litetest meta: @@ -50,6 +58,8 @@ path: ../../third_party/pkg/process process_fakes: path: ../../tools/pkg/process_fakes + process_runner: + path: ../../third_party/pkg/process_runner smith: path: ../../../third_party/dart/pkg/smith typed_data:
diff --git a/testing/skia_gold_client/test/skia_gold_client_test.dart b/testing/skia_gold_client/test/skia_gold_client_test.dart index 1d094d0..0c76212 100644 --- a/testing/skia_gold_client/test/skia_gold_client_test.dart +++ b/testing/skia_gold_client/test/skia_gold_client_test.dart
@@ -1,3 +1,7 @@ +// 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. + import 'dart:async'; import 'dart:convert'; import 'dart:io' as io;
diff --git a/testing/skia_gold_client/tool/e2e_fixtures/solid_blue_square.png b/testing/skia_gold_client/tool/e2e_fixtures/solid_blue_square.png new file mode 100644 index 0000000..e15f0c8 --- /dev/null +++ b/testing/skia_gold_client/tool/e2e_fixtures/solid_blue_square.png Binary files differ
diff --git a/testing/skia_gold_client/tool/e2e_fixtures/solid_green_square.png b/testing/skia_gold_client/tool/e2e_fixtures/solid_green_square.png new file mode 100644 index 0000000..0b6f149 --- /dev/null +++ b/testing/skia_gold_client/tool/e2e_fixtures/solid_green_square.png Binary files differ
diff --git a/testing/skia_gold_client/tool/e2e_fixtures/solid_red_square.png b/testing/skia_gold_client/tool/e2e_fixtures/solid_red_square.png new file mode 100644 index 0000000..53388f4 --- /dev/null +++ b/testing/skia_gold_client/tool/e2e_fixtures/solid_red_square.png Binary files differ
diff --git a/testing/skia_gold_client/tool/e2e_test.dart b/testing/skia_gold_client/tool/e2e_test.dart new file mode 100644 index 0000000..79e23e6 --- /dev/null +++ b/testing/skia_gold_client/tool/e2e_test.dart
@@ -0,0 +1,143 @@ +// 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. + +import 'dart:io'; + +import 'package:engine_repo_tools/engine_repo_tools.dart'; +import 'package:path/path.dart' as path; +import 'package:skia_gold_client/skia_gold_client.dart'; + +/// An E2E test for the Skia Gold client. +/// +/// Attempts to answer the question: "Does the Skia Gold client, and our custom +/// integration with it (GitHub, LUCI, etc), work as expected?" in an automated +/// way. +/// +/// In a sibling directory, `e2e_fixtures`, there are static (checked-in) files +/// that represent (fake) generated output to be uploaded and verified by the +/// Skia Gold client. This test, when run on CI, will use these fixtures to +/// simulate the process of uploading and verifying real output. +/// +/// For example, try changing the contents of a fixture file, and then uploading +/// a PR with this change. The CI will run this test, and it should fail (on +/// pre-submit), since the fixture file no longer matches the expected output. +/// +/// Next, after the PR is merged, the CI will run this test again, and it should +/// pass (on post-submit), since the fixture file now matches the expected +/// output. +/// +/// There are also tests for the "dimensions" feature of the Skia Gold client, +/// which live in `e2e_fixtures/dimensions`. These tests are similar to the +/// regular tests, but experiment with different fake dimensions to ensure that +/// our CI environment is correctly handling this feature. +void main() async { + // If the client is not available, we can't run the test. + if (!SkiaGoldClient.isAvailable()) { + stderr.writeln('Skia gold is unavailable in this environment.'); + exitCode = 1; + return; + } + + // If we're not in an engine repo, we can't run the test. + final Engine? engine = Engine.tryFindWithin(); + if (engine == null) { + stderr.writeln('Must run within the engine repo.'); + exitCode = 1; + return; + } + + // Create a client. + final SkiaGoldClient skiaGoldClient = SkiaGoldClient( + engine.flutterDir, + ); + + // Authenticate the client. + await skiaGoldClient.auth(); + + const String prefix = 'SkiaGoldClientE2ETest'; + const List<_Digest> digests = <_Digest>[ + _Digest( + name: '${prefix}_SolidBlueSquare', + source: 'e2e_fixtures/solid_blue_square.png', + pixelCount: 512 * 512, + ), + _Digest( + name: '${prefix}_SolidRedSquare', + source: 'e2e_fixtures/solid_red_square.png', + pixelCount: 768 * 768, + ), + _Digest( + name: '${prefix}_SolidGreenSquare', + source: 'e2e_fixtures/solid_green_square.png', + pixelCount: 1200 * 1200, + ), + ]; + + // Upload the digests to Skia Gold. + final Set<_Digest> comparisonsFailed = <_Digest>{}; + for (final _Digest digest in digests) { + final String digestPath = digest.source; + final String digestName = digest.name; + final File digestFile = File(path.join( + engine.flutterDir.path, + 'testing', + 'skia_gold_client', + 'tool', + digestPath, + )); + if (!digestFile.existsSync()) { + stderr.writeln('The digest file "$digestPath" does not exist.'); + exitCode = 1; + return; + } + + print('Uploading digest: $digestName ($digestPath): ${digest.pixelCount} pixels...'); + + try { + await skiaGoldClient.addImg( + digestName, + digestFile, + screenshotSize: 0, + ); + stderr.writeln('Comparison success: $digestName'); + } on Exception catch (e) { + stderr.writeln('Comparison failure: $digestName: $e'); + comparisonsFailed.add(digest); + } + } + + if (comparisonsFailed.isNotEmpty) { + stdout.writeln('${comparisonsFailed.length} digest(s) failed.'); + for (final _Digest digest in comparisonsFailed) { + stderr.writeln(' ${digest.name} (${digest.source})'); + } + exitCode = 1; + return; + } +} + +final class _Digest { + const _Digest({ + required this.name, + required this.source, + required this.pixelCount, + }); + + /// The name of the digest/test. + final String name; + + /// The source of the digest (e.g. the path to the image file). + final String source; + + /// The number of pixels in the image. + final int pixelCount; + + @override + int get hashCode => source.hashCode; + + @override + bool operator ==(Object other) { + return other is _Digest && other.source == source; + } +}
diff --git a/testing/skia_gold_client/tool/generate.dart b/testing/skia_gold_client/tool/generate.dart new file mode 100644 index 0000000..a9753ae --- /dev/null +++ b/testing/skia_gold_client/tool/generate.dart
@@ -0,0 +1,120 @@ +// 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. + +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:engine_repo_tools/engine_repo_tools.dart'; +import 'package:git_repo_tools/git_repo_tools.dart'; +import 'package:path/path.dart' as path; + +/// Takes the images in `source_images`, writes text on them (i.e. git hash) +/// and saves them in `e2e_fixtures`. By default, no arguments are needed but +/// ImageMagick must be installed. +void main(List<String> args) async { + final Engine? engine = Engine.tryFindWithin(); + + final ArgParser parser = ArgParser() + ..addFlag( + 'help', + abbr: 'h', + help: 'Prints usage information.', + negatable: false, + ) + ..addOption( + 'image-magick-convert-bin', + help: 'The path to the ImageMagick `convert` executable.', + defaultsTo: 'convert', + hide: true, + ) + ..addOption( + 'annotation', + abbr: 'a', + help: 'The text to write on the images.', + defaultsTo: engine == null + ? null + : await GitRepo.fromRoot(engine.flutterDir).headSha(short: true), + ) + ..addOption( + 'source', + abbr: 's', + help: 'The directory containing the images to be modified.', + defaultsTo: engine == null + ? null + : path.join( + engine.flutterDir.path, + 'testing', + 'skia_gold_client', + 'tool', + 'source_images', + ), + ) + ..addOption( + 'output', + abbr: 'o', + help: 'The directory to save the modified images in.', + defaultsTo: engine == null + ? null + : path.join( + engine.flutterDir.path, + 'testing', + 'skia_gold_client', + 'tool', + 'e2e_fixtures', + ), + ); + + final ArgResults results = parser.parse(args); + if (results['help'] as bool) { + print(parser.usage); + return; + } + + final String relativeDir = engine?.flutterDir.path ?? ''; + final String imageMagickConvertBin = results['image-magick-convert-bin'] as String; + final String annotation = results['annotation'] as String; + final String source = results['source'] as String; + final String output = results['output'] as String; + + print( + 'Writing annotation "$annotation" on images in ' + '${path.relative(source, from: relativeDir)} and saving them in ' + '${path.relative(output, from: relativeDir)}.', + ); + + final List<String> sourceImages = Directory(source) + .listSync() + .whereType<File>() + .map((File file) => file.path) + .toList(); + + // For each source image, write the annotation and save it in the output directory. + for (final String sourceImage in sourceImages) { + final String outputImage = path.join( + output, + '${path.basenameWithoutExtension(sourceImage)}.png', + ); + print('Writing to ${path.relative(outputImage, from: relativeDir)}'); + await Process.run( + imageMagickConvertBin, + <String>[ + sourceImage, + '-fill', + 'white', + '-undercolor', + 'black', + '-gravity', + 'SouthEast', + '-pointsize', + '24', + '-annotate', + '+10+10', + annotation, + outputImage, + ], + ); + } + + print('Done: wrote ${sourceImages.length} image.'); +}
diff --git a/testing/skia_gold_client/tool/source_images/solid_blue_square.png b/testing/skia_gold_client/tool/source_images/solid_blue_square.png new file mode 100644 index 0000000..424d663 --- /dev/null +++ b/testing/skia_gold_client/tool/source_images/solid_blue_square.png Binary files differ
diff --git a/testing/skia_gold_client/tool/source_images/solid_green_square.png b/testing/skia_gold_client/tool/source_images/solid_green_square.png new file mode 100644 index 0000000..162932f --- /dev/null +++ b/testing/skia_gold_client/tool/source_images/solid_green_square.png Binary files differ
diff --git a/testing/skia_gold_client/tool/source_images/solid_red_square.png b/testing/skia_gold_client/tool/source_images/solid_red_square.png new file mode 100644 index 0000000..5cf7bbf --- /dev/null +++ b/testing/skia_gold_client/tool/source_images/solid_red_square.png Binary files differ
diff --git a/tools/pkg/git_repo_tools/lib/git_repo_tools.dart b/tools/pkg/git_repo_tools/lib/git_repo_tools.dart index 40fd077..db28e6d 100644 --- a/tools/pkg/git_repo_tools/lib/git_repo_tools.dart +++ b/tools/pkg/git_repo_tools/lib/git_repo_tools.dart
@@ -43,6 +43,17 @@ /// getter will return the same result. late final Future<List<io.File>> changedFiles = _changedFiles(); + /// Returns the SHA of the current HEAD commit. + Future<String> headSha({bool short = false}) async { + final ProcessRunnerResult result = await ProcessRunner( + defaultWorkingDirectory: root, + processManager: _processManager, + ).runProcess( + <String>['git', 'rev-parse', if (short) '--short' ,'HEAD'], + ); + return result.stdout.trim(); + } + Future<List<io.File>> _changedFiles() async { final ProcessRunner processRunner = ProcessRunner( defaultWorkingDirectory: root,
