From 11a9cb7029d77aef521a934645383780a732d67c Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Tue, 12 Dec 2023 20:09:13 -0800 Subject: [PATCH] Make tests more resilient to Skia gold failures and refactor flutter_goldens for extensive technical debt removal (#139549) * Remove all use of global variables. * Always pass in all dependencies, only create them in main or in tests. * Pass in the "print" primitive. * Make all network traffic retry (except when run locally, when it just auto-passes). * Enable tests to be run in random order. --- .../flutter_test/flutter_gold_expectation.txt | 6 - .../flutter_test/flutter_gold_test.dart | 18 + packages/flutter/pubspec.yaml | 4 +- .../flutter/test_private/test/pubspec.yaml | 4 +- packages/flutter_goldens/README | 9 + packages/flutter_goldens/dart_test.yaml | 5 - .../flutter_goldens/lib/flutter_goldens.dart | 353 +-- .../lib/skia_client.dart | 386 +-- packages/flutter_goldens/pubspec.yaml | 6 +- .../test/comparator_selection_test.dart | 168 ++ .../test/flutter_goldens_test.dart | 2099 ++++++++--------- .../test/skia_client_test.dart | 90 + packages/flutter_goldens_client/pubspec.yaml | 22 - 13 files changed, 1669 insertions(+), 1501 deletions(-) create mode 100644 packages/flutter_goldens/README delete mode 100644 packages/flutter_goldens/dart_test.yaml rename packages/{flutter_goldens_client => flutter_goldens}/lib/skia_client.dart (66%) create mode 100644 packages/flutter_goldens/test/comparator_selection_test.dart create mode 100644 packages/flutter_goldens/test/skia_client_test.dart delete mode 100644 packages/flutter_goldens_client/pubspec.yaml diff --git a/dev/automated_tests/flutter_test/flutter_gold_expectation.txt b/dev/automated_tests/flutter_test/flutter_gold_expectation.txt index 7de9b5a2a55..1008a501f5d 100644 --- a/dev/automated_tests/flutter_test/flutter_gold_expectation.txt +++ b/dev/automated_tests/flutter_test/flutter_gold_expectation.txt @@ -1,7 +1 @@ -[0-9]+:[0-9]+ [+]0: Local passes non-existent baseline for new test, null expectation * - *No expectations provided by Skia Gold for test: library.flutter.new_golden_test.1.png. This may be a new test. If this is an unexpected result, check https://flutter-gold.skia.org. - *Validate image output found at flutter/test/library/ -[0-9]+:[0-9]+ [+]1: Local passes non-existent baseline for new test, empty expectation * - *No expectations provided by Skia Gold for test: library.flutter.new_golden_test.2.png. This may be a new test. If this is an unexpected result, check https://flutter-gold.skia.org. - *Validate image output found at flutter/test/library/ [0-9]+:[0-9]+ [+]2: All tests passed! * diff --git a/dev/automated_tests/flutter_test/flutter_gold_test.dart b/dev/automated_tests/flutter_test/flutter_gold_test.dart index b12ac4efd16..bf0ba734497 100644 --- a/dev/automated_tests/flutter_test/flutter_gold_test.dart +++ b/dev/automated_tests/flutter_test/flutter_gold_test.dart @@ -21,6 +21,7 @@ const List _kFailPngBytes = [ ]; void main() { + final List log = []; final MemoryFileSystem fs = MemoryFileSystem(); final Directory basedir = fs.directory('flutter/test/library/') ..createSync(recursive: true); @@ -34,6 +35,7 @@ void main() { environment: {'FLUTTER_ROOT': '/flutter'}, operatingSystem: 'macos' ), + log: log.add, ); test('Local passes non-existent baseline for new test, null expectation', () async { @@ -44,6 +46,12 @@ void main() { ), isTrue, ); + expect(log, [ + // ignore: no_adjacent_strings_in_list + 'No expectations provided by Skia Gold for test: library.flutter.new_golden_test.1.png. ' + 'This may be a new test. If this is an unexpected result, check https://flutter-gold.skia.org.\n' + 'Validate image output found at flutter/test/library/' + ]); }); test('Local passes non-existent baseline for new test, empty expectation', () async { @@ -54,6 +62,16 @@ void main() { ), isTrue, ); + expect(log, [ + // ignore: no_adjacent_strings_in_list + 'No expectations provided by Skia Gold for test: library.flutter.new_golden_test.1.png. ' + 'This may be a new test. If this is an unexpected result, check https://flutter-gold.skia.org.\n' + 'Validate image output found at flutter/test/library/', + // ignore: no_adjacent_strings_in_list + 'No expectations provided by Skia Gold for test: library.flutter.new_golden_test.2.png. ' + 'This may be a new test. If this is an unexpected result, check https://flutter-gold.skia.org.\n' + 'Validate image output found at flutter/test/library/', + ]); }); } diff --git a/packages/flutter/pubspec.yaml b/packages/flutter/pubspec.yaml index 88cd5ba685c..b5be3d96c20 100644 --- a/packages/flutter/pubspec.yaml +++ b/packages/flutter/pubspec.yaml @@ -29,6 +29,7 @@ dev_dependencies: async: 2.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" boolean_selector: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" clock: 1.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + crypto: 3.0.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" file: 7.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" matcher: 0.12.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" path: 1.9.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" @@ -40,6 +41,7 @@ dev_dependencies: string_scanner: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" term_glyph: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" test_api: 0.6.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" vm_service: 13.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" -# PUBSPEC CHECKSUM: 8088 +# PUBSPEC CHECKSUM: bbe9 diff --git a/packages/flutter/test_private/test/pubspec.yaml b/packages/flutter/test_private/test/pubspec.yaml index f64064d4d38..ee384126455 100644 --- a/packages/flutter/test_private/test/pubspec.yaml +++ b/packages/flutter/test_private/test/pubspec.yaml @@ -38,8 +38,10 @@ dev_dependencies: sdk: flutter fake_async: 1.3.1 + crypto: 3.0.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" file: 7.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" platform: 3.1.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" process: 5.0.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" -# PUBSPEC CHECKSUM: db59 +# PUBSPEC CHECKSUM: 0fba diff --git a/packages/flutter_goldens/README b/packages/flutter_goldens/README new file mode 100644 index 00000000000..90bd254ad5d --- /dev/null +++ b/packages/flutter_goldens/README @@ -0,0 +1,9 @@ +This package is an internal implementation detail for our testing +infrastructure. It enables the framework to use the Skia Gold +infrastructure for tracking golden image tests. + +See also: + + * https://skia.org/docs/dev/testing/skiagold/ + * https://flutter-gold.skia.org/ + * https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter diff --git a/packages/flutter_goldens/dart_test.yaml b/packages/flutter_goldens/dart_test.yaml deleted file mode 100644 index fb3b8a21d3f..00000000000 --- a/packages/flutter_goldens/dart_test.yaml +++ /dev/null @@ -1,5 +0,0 @@ -tags: - # This tag tells the test framework to not shuffle the test order according to - # the --test-randomize-ordering-seed for the suites that have this tag. - no-shuffle: - allow_test_randomization: false diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index eef5ed39396..ab16ffc9ece 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -3,23 +3,28 @@ // found in the LICENSE file. import 'dart:async' show FutureOr; -import 'dart:io' as io show OSError, SocketException; +import 'dart:io' as io; import 'package:file/file.dart'; import 'package:file/local.dart'; import 'package:flutter/foundation.dart'; -import 'package:flutter_goldens_client/skia_client.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:platform/platform.dart'; +import 'package:process/process.dart'; -export 'package:flutter_goldens_client/skia_client.dart'; +import 'skia_client.dart'; +export 'skia_client.dart'; // If you are here trying to figure out how to use golden files in the Flutter // repo itself, consider reading this wiki page: // https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package%3Aflutter const String _kFlutterRootKey = 'FLUTTER_ROOT'; -final RegExp _kMainBranch = RegExp(r'master|main'); + +bool _isMainBranch(String? branch) { + return branch == 'main' + || branch == 'master'; +} /// Main method that can be used in a `flutter_test_config.dart` file to set /// [goldenFileComparator] to an instance of [FlutterGoldenFileComparator] that @@ -28,18 +33,51 @@ final RegExp _kMainBranch = RegExp(r'master|main'); /// /// When set, the `namePrefix` is prepended to the names of all gold images. Future testExecutable(FutureOr Function() testMain, {String? namePrefix}) async { + assert(goldenFileComparator is LocalFileComparator); const Platform platform = LocalPlatform(); - if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) { - goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix); - } else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) { - goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix); - } else if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) { - goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator( - 'Golden file testing is not executed on Cirrus, or LUCI environments outside of flutter/flutter.', - namePrefix: namePrefix + const FileSystem fs = LocalFileSystem(); + const ProcessManager process = LocalProcessManager(); + final io.HttpClient httpClient = io.HttpClient(); + if (FlutterPostSubmitFileComparator.isRecommendedForEnvironment(platform)) { + goldenFileComparator = await FlutterPostSubmitFileComparator.fromLocalFileComparator( + localFileComparator: goldenFileComparator as LocalFileComparator, + namePrefix: namePrefix, + platform: platform, + fs: fs, + process: process, + httpClient: httpClient, + log: print, + ); + } else if (FlutterPreSubmitFileComparator.isRecommendedForEnvironment(platform)) { + goldenFileComparator = await FlutterPreSubmitFileComparator.fromLocalFileComparator( + localFileComparator: goldenFileComparator as LocalFileComparator, + namePrefix: namePrefix, + platform: platform, + fs: fs, + process: process, + httpClient: httpClient, + log: print, + ); + } else if (FlutterSkippingFileComparator.isRecommendedForEnvironment(platform)) { + goldenFileComparator = FlutterSkippingFileComparator.fromLocalFileComparator( + localFileComparator: goldenFileComparator as LocalFileComparator, + namePrefix: namePrefix, + reason: 'Golden file testing is not executed on Cirrus, or LUCI environments outside of flutter/flutter.', + platform: platform, + fs: fs, + process: process, + httpClient: httpClient, + log: print, ); } else { - goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform); + goldenFileComparator = await FlutterLocalFileComparator.fromLocalFileComparator( + localFileComparator: goldenFileComparator as LocalFileComparator, + platform: platform, + fs: fs, + process: process, + httpClient: httpClient, + log: print, + ); } await testMain(); @@ -88,12 +126,12 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { /// /// The [fs] and [platform] parameters are useful in tests, where the default /// file system and platform can be replaced by mock instances. - @visibleForTesting FlutterGoldenFileComparator( this.basedir, this.skiaClient, { - this.fs = const LocalFileSystem(), - this.platform = const LocalPlatform(), + required this.fs, + required this.platform, + required this.log, this.namePrefix, }); @@ -113,6 +151,9 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { @visibleForTesting final Platform platform; + /// The logging function to use when reporting messages to the console. + final LogCallback log; + /// The prefix that is added to all golden names. final String? namePrefix; @@ -135,11 +176,11 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { @protected @visibleForTesting static Directory getBaseDirectory( - LocalFileComparator defaultComparator, - Platform platform, { + LocalFileComparator defaultComparator, { String? suffix, + required Platform platform, + required FileSystem fs, }) { - const FileSystem fs = LocalFileSystem(); final Directory flutterRoot = fs.directory(platform.environment[_kFlutterRootKey]); Directory comparisonRoot; @@ -214,34 +255,48 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator { FlutterPostSubmitFileComparator( super.basedir, super.skiaClient, { - super.fs, - super.platform, + required super.fs, + required super.platform, + required super.log, super.namePrefix, }); /// Creates a new [FlutterPostSubmitFileComparator] that mirrors the relative /// path resolution of the default [goldenFileComparator]. - /// - /// The [goldens] and [defaultComparator] parameters are visible for testing - /// purposes only. - static Future fromDefaultComparator( - final Platform platform, { - SkiaGoldClient? goldens, - LocalFileComparator? defaultComparator, + static Future fromLocalFileComparator({ + required LocalFileComparator localFileComparator, String? namePrefix, + required Platform platform, + required FileSystem fs, + required ProcessManager process, + required io.HttpClient httpClient, + required LogCallback log, }) async { - - defaultComparator ??= goldenFileComparator as LocalFileComparator; final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory( - defaultComparator, - platform, + localFileComparator, suffix: 'flutter_goldens_postsubmit.', + platform: platform, + fs: fs, ); baseDirectory.createSync(recursive: true); - goldens ??= SkiaGoldClient(baseDirectory); + final SkiaGoldClient goldens = SkiaGoldClient( + baseDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: httpClient, + log: log, + ); await goldens.auth(); - return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens, namePrefix: namePrefix); + return FlutterPostSubmitFileComparator( + baseDirectory.uri, + goldens, + fs: fs, + platform: platform, + log: log, + namePrefix: namePrefix, + ); } @override @@ -250,21 +305,17 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator { golden = _addPrefix(golden); await update(golden, imageBytes); final File goldenFile = getGoldenFile(golden); - - return skiaClient.imgtestAdd(golden.path, goldenFile); + skiaClient.imgtestAdd(golden.path, goldenFile); // throws if the result is false + return true; } /// Decides based on the current environment if goldens tests should be /// executed through Skia Gold. - static bool isAvailableForEnvironment(Platform platform) { - final bool luciPostSubmit = platform.environment.containsKey('SWARMING_TASK_ID') - && platform.environment.containsKey('GOLDCTL') - // Luci tryjob environments contain this value to inform the [FlutterPreSubmitComparator]. - && !platform.environment.containsKey('GOLD_TRYJOB') - // Only run on main branch. - && _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? ''); - - return luciPostSubmit; + static bool isRecommendedForEnvironment(Platform platform) { + return platform.environment.containsKey('SWARMING_TASK_ID') // Indicates LUCI environment. + && platform.environment.containsKey('GOLDCTL') // Needed to use Gold. + && !platform.environment.containsKey('GOLD_TRYJOB') // Indicates a pre-submit environment on LUCI. + && _isMainBranch(platform.environment['GIT_BRANCH']); } } @@ -291,41 +342,49 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { FlutterPreSubmitFileComparator( super.basedir, super.skiaClient, { - super.fs, - super.platform, + required super.fs, + required super.platform, + required super.log, super.namePrefix, }); /// Creates a new [FlutterPreSubmitFileComparator] that mirrors the /// relative path resolution of the default [goldenFileComparator]. - /// - /// The [goldens] and [defaultComparator] parameters are visible for testing - /// purposes only. - static Future fromDefaultComparator( - final Platform platform, { - SkiaGoldClient? goldens, - LocalFileComparator? defaultComparator, + static Future fromLocalFileComparator({ + required LocalFileComparator localFileComparator, Directory? testBasedir, String? namePrefix, + required Platform platform, + required FileSystem fs, + required ProcessManager process, + required io.HttpClient httpClient, + required LogCallback log, }) async { - - defaultComparator ??= goldenFileComparator as LocalFileComparator; final Directory baseDirectory = testBasedir ?? FlutterGoldenFileComparator.getBaseDirectory( - defaultComparator, - platform, + localFileComparator, suffix: 'flutter_goldens_presubmit.', + platform: platform, + fs: fs, ); - if (!baseDirectory.existsSync()) { baseDirectory.createSync(recursive: true); } - goldens ??= SkiaGoldClient(baseDirectory); - + final SkiaGoldClient goldens = SkiaGoldClient( + baseDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: httpClient, + log: log, + ); await goldens.auth(); return FlutterPreSubmitFileComparator( baseDirectory.uri, - goldens, platform: platform, + goldens, + fs: fs, + platform: platform, + log: log, namePrefix: namePrefix, ); } @@ -346,21 +405,19 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator { /// Decides based on the current environment if goldens tests should be /// executed as pre-submit tests with Skia Gold. - static bool isAvailableForEnvironment(Platform platform) { - final bool luciPreSubmit = platform.environment.containsKey('SWARMING_TASK_ID') - && platform.environment.containsKey('GOLDCTL') - && platform.environment.containsKey('GOLD_TRYJOB') - // Only run on the main branch - && _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? ''); - return luciPreSubmit; + static bool isRecommendedForEnvironment(Platform platform) { + return platform.environment.containsKey('SWARMING_TASK_ID') // Indicates LUCI environment. + && platform.environment.containsKey('GOLDCTL') // Needed to use Gold. + && platform.environment.containsKey('GOLD_TRYJOB') // Indicates a pre-submit environment on LUCI. + && _isMainBranch(platform.environment['GIT_BRANCH']); } } /// A [FlutterGoldenFileComparator] for testing conditions that do not execute /// golden file tests. /// -/// Currently, this comparator is used on Cirrus, or in Luci environments when executing tests -/// outside of the flutter/flutter repository. +/// Currently, this comparator is used on Cirrus, or in Luci environments when +/// executing tests outside of the flutter/flutter repository. /// /// See also: /// @@ -379,6 +436,9 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { super.basedir, super.skiaClient, this.reason, { + required super.fs, + required super.platform, + required super.log, super.namePrefix, }); @@ -387,25 +447,39 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { /// Creates a new [FlutterSkippingFileComparator] that mirrors the /// relative path resolution of the default [goldenFileComparator]. - static FlutterSkippingFileComparator fromDefaultComparator( - String reason, { - LocalFileComparator? defaultComparator, + static FlutterSkippingFileComparator fromLocalFileComparator({ + required LocalFileComparator localFileComparator, String? namePrefix, + required String reason, + required FileSystem fs, + required ProcessManager process, + required Platform platform, + required io.HttpClient httpClient, + required LogCallback log, }) { - defaultComparator ??= goldenFileComparator as LocalFileComparator; - const FileSystem fs = LocalFileSystem(); - final Uri basedir = defaultComparator.basedir; - final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir)); - return FlutterSkippingFileComparator(basedir, skiaClient, reason, namePrefix: namePrefix); + final Uri basedir = localFileComparator.basedir; + final SkiaGoldClient skiaClient = SkiaGoldClient( + fs.directory(basedir), + fs: fs, + process: process, + platform: platform, + httpClient: httpClient, + log: log, + ); + return FlutterSkippingFileComparator( + basedir, + skiaClient, + reason, + fs: fs, + platform: platform, + log: log, + namePrefix: namePrefix, + ); } @override Future compare(Uint8List imageBytes, Uri golden) async { - // Ideally we would use markTestSkipped here but in some situations, - // comparators are called outside of tests. - // See also: https://github.com/flutter/flutter/issues/91285 - // ignore: avoid_print - print('Skipping "$golden" test: $reason'); + log('Auto-passing "$golden" test without checking: $reason'); return true; } @@ -416,13 +490,10 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { /// used. /// /// If we are in a CI environment, LUCI or Cirrus, but are not using the other - /// comparators, we skip. - static bool isAvailableForEnvironment(Platform platform) { - return (platform.environment.containsKey('SWARMING_TASK_ID') - // Some builds are still being run on Cirrus, we should skip these. - || platform.environment.containsKey('CIRRUS_CI')) - // If we are in CI, skip on branches that are not main. - && !_kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? ''); + /// comparators (determined by checking this after the others), we skip. + static bool isRecommendedForEnvironment(Platform platform) { + return platform.environment.containsKey('SWARMING_TASK_ID') // Indicates LUCI environment. + || platform.environment.containsKey('CIRRUS_CI'); // Indicates Cirrus environment. } } @@ -432,7 +503,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator { /// This comparator utilizes the [SkiaGoldClient] to request baseline images for /// the given device under test for comparison. This comparator is initialized /// when conditions for all other [FlutterGoldenFileComparators] have not been -/// met, see the `isAvailableForEnvironment` method for each one listed below. +/// met, see the `isRecommendedForEnvironment` method for each one listed below. /// /// The [FlutterLocalFileComparator] is intended to run on local machines and /// serve as a smoke test during development. As such, it will not be able to @@ -461,52 +532,45 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC FlutterLocalFileComparator( super.basedir, super.skiaClient, { - super.fs, - super.platform, + required super.fs, + required super.platform, + required super.log, }); /// Creates a new [FlutterLocalFileComparator] that mirrors the /// relative path resolution of the default [goldenFileComparator]. - /// - /// The [goldens], [defaultComparator], and [baseDirectory] parameters are - /// visible for testing purposes only. - static Future fromDefaultComparator( - final Platform platform, { - SkiaGoldClient? goldens, - LocalFileComparator? defaultComparator, - Directory? baseDirectory, + static Future fromLocalFileComparator({ + required LocalFileComparator localFileComparator, + required Platform platform, + required FileSystem fs, + required ProcessManager process, + required io.HttpClient httpClient, + required LogCallback log, }) async { - defaultComparator ??= goldenFileComparator as LocalFileComparator; - baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory( - defaultComparator, - platform, + final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory( + localFileComparator, + platform: platform, + fs: fs, ); - if (!baseDirectory.existsSync()) { baseDirectory.createSync(recursive: true); } - goldens ??= SkiaGoldClient(baseDirectory); - try { - // Check if we can reach Gold. - await goldens.getExpectationForTest(''); - } on io.OSError catch (_) { - return FlutterSkippingFileComparator( - baseDirectory.uri, - goldens, - 'OSError occurred, could not reach Gold. ' - 'Switching to FlutterSkippingGoldenFileComparator.', - ); - } on io.SocketException catch (_) { - return FlutterSkippingFileComparator( - baseDirectory.uri, - goldens, - 'SocketException occurred, could not reach Gold. ' - 'Switching to FlutterSkippingGoldenFileComparator.', - ); - } - - return FlutterLocalFileComparator(baseDirectory.uri, goldens); + final SkiaGoldClient goldens = SkiaGoldClient( + baseDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: httpClient, + log: log, + ); + return FlutterLocalFileComparator( + baseDirectory.uri, + goldens, + fs: fs, + platform: platform, + log: log, + ); } @override @@ -514,21 +578,24 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC golden = _addPrefix(golden); final String testName = skiaClient.cleanTestName(golden.path); late String? testExpectation; - testExpectation = await skiaClient.getExpectationForTest(testName); - - if (testExpectation == null || testExpectation.isEmpty) { - // There is no baseline for this test. - // Ideally we would use markTestSkipped here but in some situations, - // comparators are called outside of tests. - // See also: https://github.com/flutter/flutter/issues/91285 - // ignore: avoid_print - print( - 'No expectations provided by Skia Gold for test: $golden. ' - 'This may be a new test. If this is an unexpected result, check ' - 'https://flutter-gold.skia.org.\n' - 'Validate image output found at $basedir' - ); - update(golden, imageBytes); + try { + testExpectation = await skiaClient.getExpectationForTest(testName); + if (testExpectation == null || testExpectation.isEmpty) { + log( + 'No expectations provided by Skia Gold for test: $golden. ' + 'This may be a new test. If this is an unexpected result, check ' + 'https://flutter-gold.skia.org.\n' + 'Validate image output found at $basedir' + ); + update(golden, imageBytes); + return true; + } + } on Exception catch (error) { + if (error is! io.SocketException && + error is! io.OSError) { + rethrow; // "uncaught error" + } + log('Auto-passing "$golden" test, ignoring network error when contacting Skia.'); return true; } diff --git a/packages/flutter_goldens_client/lib/skia_client.dart b/packages/flutter_goldens/lib/skia_client.dart similarity index 66% rename from packages/flutter_goldens_client/lib/skia_client.dart rename to packages/flutter_goldens/lib/skia_client.dart index e89a3068a9c..7f9cd44cbc5 100644 --- a/packages/flutter_goldens_client/lib/skia_client.dart +++ b/packages/flutter_goldens/lib/skia_client.dart @@ -7,7 +7,6 @@ import 'dart:io' as io; import 'package:crypto/crypto.dart'; import 'package:file/file.dart'; -import 'package:file/local.dart'; import 'package:path/path.dart' as path; import 'package:platform/platform.dart'; import 'package:process/process.dart'; @@ -21,6 +20,9 @@ const String _kGoldctlKey = 'GOLDCTL'; const String _kTestBrowserKey = 'FLUTTER_TEST_BROWSER'; const String _kWebRendererKey = 'FLUTTER_WEB_RENDERER'; +/// Signature of callbacks used to inject [print] replacements. +typedef LogCallback = void Function(String); + /// Exception thrown when an error is returned from the [SkiaClient]. class SkiaException implements Exception { /// Creates a new `SkiaException` with a required error [message]. @@ -40,16 +42,14 @@ class SkiaException implements Exception { /// Flutter Gold Dashboard. class SkiaGoldClient { /// Creates a [SkiaGoldClient] with the given [workDirectory]. - /// - /// All other parameters are optional. They may be provided in tests to - /// override the defaults for [fs], [process], [platform], and [httpClient]. SkiaGoldClient( this.workDirectory, { - this.fs = const LocalFileSystem(), - this.process = const LocalProcessManager(), - this.platform = const LocalPlatform(), - io.HttpClient? httpClient, - }) : httpClient = httpClient ?? io.HttpClient(); + required this.fs, + required this.process, + required this.platform, + required this.httpClient, + required this.log, + }); /// The file system to use for storing the local clone of the repository. /// @@ -57,12 +57,6 @@ class SkiaGoldClient { /// replaced by a memory file system. final FileSystem fs; - /// A wrapper for the [dart:io.Platform] API. - /// - /// This is useful in tests, where the system platform (the default) can be - /// replaced by a mock platform instance. - final Platform platform; - /// A controller for launching sub-processes. /// /// This is useful in tests, where the real process manager (the default) can @@ -70,9 +64,18 @@ class SkiaGoldClient { /// sub-processes. final ProcessManager process; + /// A wrapper for the [dart:io.Platform] API. + /// + /// This is useful in tests, where the system platform (the default) can be + /// replaced by a mock platform instance. + final Platform platform; + /// A client for making Http requests to the Flutter Gold dashboard. final io.HttpClient httpClient; + /// The logging function to use when reporting messages to the console. + final void Function(String message) log; + /// The local [Directory] within the [comparisonRoot] for the current test /// context. In this directory, the client will create image and JSON files /// for the goldctl tool to use. @@ -91,38 +94,109 @@ class SkiaGoldClient { /// Uses the [platform] environment in this implementation. String get _goldctl => platform.environment[_kGoldctlKey]!; + static void _indent(LogCallback writeln, String text) { + if (text.isEmpty) { + writeln(' '); + } else { + for (final String line in text.split('\n')) { + writeln(' $line'); + } + } + } + + static void _dump(LogCallback writeln, String data, String label) { + if (data.isNotEmpty) { + writeln(''); + writeln('$label:'); + _indent(writeln, data); + } + } + + Future _retry({ + required Future Function() task, + required String taskName, + bool Function(int exitCode, String stdout, String stderr)? success, + String? errorMessage, + }) async { + Duration delay = const Duration(seconds: 5); + while (true) { + final io.ProcessResult result = await task(); + final String resultStdout = result.stdout as String; + final String resultStderr = result.stderr as String; + + if (result.exitCode != 0 && resultStdout.contains('resulted in a 502: 502 Bad Gateway')) { + // Probably a transient error, try again. + // (See https://issues.skia.org/issues/40044713) + // + // This could have false-positives, because there's no standard format + // for the error messages from Skia gold. Maybe the test name is output + // and the test name contains the string above, who knows. For now it + // seems more likely that the server is flaking than that there's a + // false positive, and false positives seem less likely to be flaky so + // we're likely to catch them when they happen. + log('Transient failure (exit code ${result.exitCode}) from Skia Gold.'); + _dump(log, resultStdout, 'stdout from gold'); + _dump(log, resultStderr, 'stderr from gold'); + log(''); + log('Retrying in ${delay.inSeconds} seconds.'); + await Future.delayed(delay); + delay *= 2; + continue; // retry + } + + if (success == null) { + if (result.exitCode == 0) { + return; // success + } + } else if (success(result.exitCode, resultStdout, resultStderr)) { + return; // success + } + + final StringBuffer buffer = StringBuffer(); + if (errorMessage != null) { + buffer.writeln(errorMessage); + buffer.writeln(); + } + buffer.writeln('$taskName failed with exit code ${result.exitCode}.'); + _dump(buffer.writeln, resultStdout, 'stdout from gold'); + _dump(buffer.writeln, resultStderr, 'stderr from gold'); + final File resultFile = workDirectory.childFile('result-state.json'); + if (await resultFile.exists()) { + _dump(buffer.writeln, resultFile.readAsStringSync(), 'result-state.json contents'); + } + throw SkiaException(buffer.toString()); // failure + } + } + /// Prepares the local work space for golden file testing and calls the /// goldctl `auth` command. /// /// This ensures that the goldctl tool is authorized and ready for testing. /// Used by the [FlutterPostSubmitFileComparator] and the /// [FlutterPreSubmitFileComparator]. + /// + /// Does nothing if [clientIsAuthorized] returns true. Future auth() async { if (await clientIsAuthorized()) { return; } - final List authCommand = [ - _goldctl, - 'auth', - '--work-dir', workDirectory - .childDirectory('temp') - .path, - '--luci', - ]; - final io.ProcessResult result = await process.run(authCommand); - - if (result.exitCode != 0) { - final StringBuffer buf = StringBuffer() - ..writeln('Skia Gold authorization failed.') - ..writeln('Luci environments authenticate using the file provided ' - 'by LUCI_CONTEXT. There may be an error with this file or Gold ' - 'authentication.') - ..writeln('Debug information for Gold --------------------------------') - ..writeln('stdout: ${result.stdout}') - ..writeln('stderr: ${result.stderr}'); - throw SkiaException(buf.toString()); - } + await _retry( + task: () => process.run([ + _goldctl, + 'auth', + '--work-dir', workDirectory + .childDirectory('temp') + .path, + '--luci', + ]), + taskName: 'auth', + errorMessage: 'Skia Gold authorization failed.\n' + '\n' + 'Luci environments authenticate using the file provided by ' + 'LUCI_CONTEXT. There may be an error with this file or Gold ' + 'authentication.', + ); } /// Signals if this client is initialized for uploading images to the Gold @@ -138,6 +212,8 @@ class SkiaGoldClient { /// The `imgtest` command collects and uploads test results to the Skia Gold /// backend, the `init` argument initializes the current test. Used by the /// [FlutterPostSubmitFileComparator]. + /// + /// This function is idempotent. Future imgtestInit() async { // This client has already been initialized if (_initialized) { @@ -173,20 +249,11 @@ class SkiaGoldClient { throw SkiaException(buf.toString()); } - final io.ProcessResult result = await process.run(imgtestInitCommand); - - if (result.exitCode != 0) { - _initialized = false; - final StringBuffer buf = StringBuffer() - ..writeln('Skia Gold imgtest init failed.') - ..writeln('An error occurred when initializing golden file test with ') - ..writeln('goldctl.') - ..writeln() - ..writeln('Debug information for Gold --------------------------------') - ..writeln('stdout: ${result.stdout}') - ..writeln('stderr: ${result.stderr}'); - throw SkiaException(buf.toString()); - } + await _retry( + task: () => process.run(imgtestInitCommand), + taskName: 'imgtest init', + errorMessage: 'An error occurred when initializing golden file test with goldctl.', + ); _initialized = true; } @@ -197,53 +264,34 @@ class SkiaGoldClient { /// returned from the invocation of this command that indicates a pass or fail /// result. /// + /// If an unapproved image has made it to post-submit, this throws, to close + /// the tree. + /// /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated by the [FlutterPostSubmitFileComparator]. - Future imgtestAdd(String testName, File goldenFile) async { - final List imgtestCommand = [ - _goldctl, - 'imgtest', 'add', - '--work-dir', workDirectory - .childDirectory('temp') - .path, - '--test-name', cleanTestName(testName), - '--png-file', goldenFile.path, - '--passfail', - ..._getPixelMatchingArguments(), - ]; - - final io.ProcessResult result = await process.run(imgtestCommand); - - if (result.exitCode != 0) { - // If an unapproved image has made it to post-submit, throw to close the - // tree. - String? resultContents; - final File resultFile = workDirectory.childFile(fs.path.join( - 'result-state.json', - )); - if (await resultFile.exists()) { - resultContents = await resultFile.readAsString(); - } - - final StringBuffer buf = StringBuffer() - ..writeln('Skia Gold received an unapproved image in post-submit ') - ..writeln('testing. Golden file images in flutter/flutter are triaged ') - ..writeln('in pre-submit during code review for the given PR.') - ..writeln() - ..writeln('Visit https://flutter-gold.skia.org/ to view and approve ') - ..writeln('the image(s), or revert the associated change. For more ') - ..writeln('information, visit the wiki: ') - ..writeln('https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter') - ..writeln() - ..writeln('Debug information for Gold --------------------------------') - ..writeln('stdout: ${result.stdout}') - ..writeln('stderr: ${result.stderr}') - ..writeln() - ..writeln('result-state.json: ${resultContents ?? 'No result file found.'}'); - throw SkiaException(buf.toString()); - } - - return true; + Future imgtestAdd(String testName, File goldenFile) async { + await _retry( + task: () => process.run([ + _goldctl, + 'imgtest', 'add', + '--work-dir', workDirectory + .childDirectory('temp') + .path, + '--test-name', cleanTestName(testName), + '--png-file', goldenFile.path, + '--passfail', + ..._getPixelMatchingArguments(), + ]), + taskName: 'imgtest add', + errorMessage: 'Skia Gold received an unapproved image in post-submit ' + 'testing. Golden file images in flutter/flutter are triaged ' + 'in pre-submit during code review for the given PR.\n' + '\n' + 'Visit https://flutter-gold.skia.org/ to view and approve ' + 'the image(s), or revert the associated change. For more ' + 'information, visit the wiki:\n' + ' https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter' + ); } /// Signals if this client is initialized for uploading tryjobs to the Gold @@ -259,6 +307,8 @@ class SkiaGoldClient { /// The `imgtest` command collects and uploads test results to the Skia Gold /// backend, the `init` argument initializes the current tryjob. Used by the /// [FlutterPreSubmitFileComparator]. + /// + /// This function is idempotent. Future tryjobInit() async { // This client has already been initialized if (_tryjobInitialized) { @@ -297,20 +347,11 @@ class SkiaGoldClient { throw SkiaException(buf.toString()); } - final io.ProcessResult result = await process.run(imgtestInitCommand); - - if (result.exitCode != 0) { - _tryjobInitialized = false; - final StringBuffer buf = StringBuffer() - ..writeln('Skia Gold tryjobInit failure.') - ..writeln('An error occurred when initializing golden file tryjob with ') - ..writeln('goldctl.') - ..writeln() - ..writeln('Debug information for Gold --------------------------------') - ..writeln('stdout: ${result.stdout}') - ..writeln('stderr: ${result.stderr}'); - throw SkiaException(buf.toString()); - } + await _retry( + task: () => process.run(imgtestInitCommand), + taskName: 'imgtest init', + errorMessage: 'An error occurred when initializing golden file tryjob with goldctl.' + ); _tryjobInitialized = true; } @@ -324,42 +365,23 @@ class SkiaGoldClient { /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated by the [FlutterPreSubmitFileComparator]. Future tryjobAdd(String testName, File goldenFile) async { - final List imgtestCommand = [ - _goldctl, - 'imgtest', 'add', - '--work-dir', workDirectory - .childDirectory('temp') - .path, - '--test-name', cleanTestName(testName), - '--png-file', goldenFile.path, - ..._getPixelMatchingArguments(), - ]; - - final io.ProcessResult result = await process.run(imgtestCommand); - - final String/*!*/ resultStdout = result.stdout.toString(); - if (result.exitCode != 0 && - !(resultStdout.contains('Untriaged') || resultStdout.contains('negative image'))) { - String? resultContents; - final File resultFile = workDirectory.childFile(fs.path.join( - 'result-state.json', - )); - if (await resultFile.exists()) { - resultContents = await resultFile.readAsString(); - } - final StringBuffer buf = StringBuffer() - ..writeln('Unexpected Gold tryjobAdd failure.') - ..writeln('Tryjob execution for golden file test $testName failed for') - ..writeln('a reason unrelated to pixel comparison.') - ..writeln() - ..writeln('Debug information for Gold --------------------------------') - ..writeln('stdout: ${result.stdout}') - ..writeln('stderr: ${result.stderr}') - ..writeln() - ..writeln() - ..writeln('result-state.json: ${resultContents ?? 'No result file found.'}'); - throw SkiaException(buf.toString()); - } + await _retry( + task: () => process.run([ + _goldctl, + 'imgtest', 'add', + '--work-dir', workDirectory.childDirectory('temp').path, + '--test-name', cleanTestName(testName), + '--png-file', goldenFile.path, + ..._getPixelMatchingArguments(), + ]), + taskName: 'imgtest add', + success: (int exitCode, String resultStdout, String resultStderr) { + return exitCode == 0 + || resultStdout.contains('Untriaged') + || resultStdout.contains('negative image'); + }, + errorMessage: 'Golden test for "$testName" failed for a reason unrelated to pixel comparison.', + ); } // Constructs arguments for `goldctl` for controlling how pixels are compared. @@ -408,58 +430,45 @@ class SkiaGoldClient { } /// Returns the latest positive digest for the given test known to Flutter - /// Gold at head. + /// Gold at head. Throws without retrying if there's a network failure. Future getExpectationForTest(String testName) async { - late String? expectation; final String traceID = getTraceID(testName); - await io.HttpOverrides.runWithHttpOverrides>(() async { - final Uri requestForExpectations = Uri.parse( - 'https://flutter-gold.skia.org/json/v2/latestpositivedigest/$traceID' - ); - late String rawResponse; - try { - final io.HttpClientRequest request = await httpClient.getUrl(requestForExpectations); - final io.HttpClientResponse response = await request.close(); - rawResponse = await utf8.decodeStream(response); - final dynamic jsonResponse = json.decode(rawResponse); - if (jsonResponse is! Map) { - throw const FormatException('Skia gold expectations do not match expected format.'); - } - expectation = jsonResponse['digest'] as String?; - } on FormatException catch (error) { - // Ideally we'd use something like package:test's printOnError, but best reliability - // in getting logs on CI for now we're just using print. - // See also: https://github.com/flutter/flutter/issues/91285 - print( // ignore: avoid_print - 'Formatting error detected requesting expectations from Flutter Gold.\n' - 'error: $error\n' - 'url: $requestForExpectations\n' - 'response: $rawResponse' - ); - rethrow; - } - }, - SkiaGoldHttpOverrides(), + final Uri requestForExpectations = Uri.parse( + 'https://flutter-gold.skia.org/json/v2/latestpositivedigest/$traceID' ); - return expectation; + String? rawResponse; + try { + final io.HttpClientRequest request = await httpClient.getUrl(requestForExpectations); + final io.HttpClientResponse response = await request.close(); + rawResponse = await utf8.decodeStream(response); + final dynamic jsonResponse = json.decode(rawResponse); + if (jsonResponse is! Map) { + throw const FormatException('Skia gold expectations do not match expected format.'); + } + return jsonResponse['digest'] as String?; // success + } on FormatException catch (error) { + log( + 'Formatting error detected requesting expectations from Flutter Gold.\n' + 'error: $error\n' + 'url: $requestForExpectations\n' + 'response: $rawResponse' + ); + rethrow; // fail + } } /// Returns a list of bytes representing the golden image retrieved from the /// Flutter Gold dashboard. /// /// The provided image hash represents an expectation from Flutter Gold. - Future>getImageBytes(String imageHash) async { + Future> getImageBytes(String imageHash) async { final List imageBytes = []; - await io.HttpOverrides.runWithHttpOverrides>(() async { - final Uri requestForImage = Uri.parse( - 'https://flutter-gold.skia.org/img/images/$imageHash.png', - ); - final io.HttpClientRequest request = await httpClient.getUrl(requestForImage); - final io.HttpClientResponse response = await request.close(); - await response.forEach((List bytes) => imageBytes.addAll(bytes)); - }, - SkiaGoldHttpOverrides(), + final Uri requestForImage = Uri.parse( + 'https://flutter-gold.skia.org/img/images/$imageHash.png', ); + final io.HttpClientRequest request = await httpClient.getUrl(requestForImage); + final io.HttpClientResponse response = await request.close(); + await response.forEach((List bytes) => imageBytes.addAll(bytes)); return imageBytes; } @@ -512,7 +521,7 @@ class SkiaGoldClient { final File authFile = workDirectory.childFile(fs.path.join( 'temp', 'auth_opt.json', - ))/*!*/; + )); if (await authFile.exists()) { final String contents = await authFile.readAsString(); @@ -568,6 +577,3 @@ class SkiaGoldClient { return md5Sum; } } - -/// Used to make HttpRequests during testing. -class SkiaGoldHttpOverrides extends io.HttpOverrides { } diff --git a/packages/flutter_goldens/pubspec.yaml b/packages/flutter_goldens/pubspec.yaml index 2fbf003f43f..13c28dfc217 100644 --- a/packages/flutter_goldens/pubspec.yaml +++ b/packages/flutter_goldens/pubspec.yaml @@ -9,25 +9,23 @@ dependencies: sdk: flutter flutter_test: sdk: flutter - flutter_goldens_client: - path: ../flutter_goldens_client file: 7.0.0 meta: 1.11.0 platform: 3.1.3 process: 5.0.1 + crypto: 3.0.3 + path: 1.9.0 async: 2.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" boolean_selector: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" characters: 1.3.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" clock: 1.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" collection: 1.18.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" - crypto: 3.0.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" fake_async: 1.3.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" leak_tracker: 9.0.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" leak_tracker_testing: 1.0.5 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" matcher: 0.12.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" material_color_utilities: 0.8.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" - path: 1.9.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" source_span: 1.10.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" stack_trace: 1.11.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" stream_channel: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" diff --git a/packages/flutter_goldens/test/comparator_selection_test.dart b/packages/flutter_goldens/test/comparator_selection_test.dart new file mode 100644 index 00000000000..b537a185ac4 --- /dev/null +++ b/packages/flutter_goldens/test/comparator_selection_test.dart @@ -0,0 +1,168 @@ +// Copyright 2014 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 'package:flutter_goldens/flutter_goldens.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:platform/platform.dart'; + +enum _Comparator { post, pre, skip, local } + +_Comparator _testRecommendations({ + bool hasFlutterRoot = false, + bool hasLuci = false, + bool hasCirrus = false, + bool hasGold = false, + bool hasTryJob = false, + String branch = 'main', + String os = 'macos', +}) { + final Platform platform = FakePlatform( + environment: { + if (hasFlutterRoot) + 'FLUTTER_ROOT': '/flutter', + if (hasLuci) + 'SWARMING_TASK_ID': '8675309', + if (hasCirrus) + 'CIRRUS_CI': 'true', + if (hasCirrus) + 'CIRRUS_PR': '', + if (hasCirrus) + 'CIRRUS_BRANCH': branch, + if (hasGold) + 'GOLDCTL': 'goldctl', + if (hasGold && hasCirrus) + 'GOLD_SERVICE_ACCOUNT': 'service account...', + if (hasTryJob) + 'GOLD_TRYJOB': 'git/ref/12345/head', + 'GIT_BRANCH': branch, + }, + operatingSystem: os, + ); + if (FlutterPostSubmitFileComparator.isRecommendedForEnvironment(platform)) { + return _Comparator.post; + } + if (FlutterPreSubmitFileComparator.isRecommendedForEnvironment(platform)) { + return _Comparator.pre; + } + if (FlutterSkippingFileComparator.isRecommendedForEnvironment(platform)) { + return _Comparator.skip; + } + return _Comparator.local; +} + +void main() { + test('Comparator recommendations - main branch', () { + // If we're running locally (no CI), use a local comparator. + expect(_testRecommendations(), _Comparator.local); + expect(_testRecommendations(hasFlutterRoot: true), _Comparator.local); + expect(_testRecommendations(hasGold: true), _Comparator.local); + expect(_testRecommendations(hasFlutterRoot: true, hasGold: true), _Comparator.local); + + // If we don't have gold but are on CI, we skip regardless. + expect(_testRecommendations(hasLuci: true), _Comparator.skip); + expect(_testRecommendations(hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true), _Comparator.skip); + expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(hasFlutterRoot: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + + // On Luci, with Gold, post-submit. Flutter root and Cirrus variables should have no effect. + expect(_testRecommendations(hasGold: true, hasLuci: true), _Comparator.post); + expect(_testRecommendations(hasGold: true, hasLuci: true, hasCirrus: true), _Comparator.post); + expect(_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true), _Comparator.post); + expect(_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true), _Comparator.post); + + // On Luci, with Gold, pre-submit. Flutter root and Cirrus variables should have no effect. + expect(_testRecommendations(hasGold: true, hasLuci: true, hasTryJob: true), _Comparator.pre); + expect(_testRecommendations(hasGold: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.pre); + expect(_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.pre); + expect(_testRecommendations(hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.pre); + + // On Cirrus (with Gold and not on Luci), we skip regardless. + expect(_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true), _Comparator.skip); + expect(_testRecommendations(hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.skip); + }); + + test('Comparator recommendations - release branch', () { + // If we're running locally (no CI), use a local comparator. + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0'), _Comparator.local); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true), _Comparator.local); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true), _Comparator.local); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasGold: true), _Comparator.local); + + // If we don't have gold but are on CI, we skip regardless. + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasLuci: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasLuci: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasFlutterRoot: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + + // On Luci, with Gold, post-submit. Flutter root and Cirrus variables should have no effect. Branch should make us skip. + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasFlutterRoot: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true), _Comparator.skip); + + // On Luci, with Gold, pre-submit. Flutter root and Cirrus variables should have no effect. Branch should make us skip. + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + + // On Cirrus (with Gold and not on Luci), we skip regardless. + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasCirrus: true, hasGold: true, hasFlutterRoot: true), _Comparator.skip); + expect(_testRecommendations(branch: 'flutter-3.16-candidate.0', hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.skip); + }); + + test('Comparator recommendations - Linux', () { + // If we're running locally (no CI), use a local comparator. + expect(_testRecommendations(os: 'linux'), _Comparator.local); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true), _Comparator.local); + expect(_testRecommendations(os: 'linux', hasGold: true), _Comparator.local); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasGold: true), _Comparator.local); + + // If we don't have gold but are on CI, we skip regardless. + expect(_testRecommendations(os: 'linux', hasLuci: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasCirrus: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasFlutterRoot: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.skip); + + // On Luci, with Gold, post-submit. Flutter root and Cirrus variables should have no effect. + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true), _Comparator.post); + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasCirrus: true), _Comparator.post); + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasFlutterRoot: true), _Comparator.post); + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true), _Comparator.post); + + // On Luci, with Gold, pre-submit. Flutter root and Cirrus variables should have no effect. + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasTryJob: true), _Comparator.pre); + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasCirrus: true, hasTryJob: true), _Comparator.pre); + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.pre); + expect(_testRecommendations(os: 'linux', hasGold: true, hasLuci: true, hasFlutterRoot: true, hasCirrus: true, hasTryJob: true), _Comparator.pre); + + // On Cirrus (with Gold and not on Luci), we skip regardless. + expect(_testRecommendations(os: 'linux', hasCirrus: true, hasGold: true, hasFlutterRoot: true), _Comparator.skip); + expect(_testRecommendations(os: 'linux', hasCirrus: true, hasGold: true, hasFlutterRoot: true, hasTryJob: true), _Comparator.skip); + }); +} diff --git a/packages/flutter_goldens/test/flutter_goldens_test.dart b/packages/flutter_goldens/test/flutter_goldens_test.dart index 3e2c05ea9cf..c2f1a3c75b1 100644 --- a/packages/flutter_goldens/test/flutter_goldens_test.dart +++ b/packages/flutter_goldens/test/flutter_goldens_test.dart @@ -5,13 +5,14 @@ // See also dev/automated_tests/flutter_test/flutter_gold_test.dart import 'dart:convert'; -import 'dart:io' hide Directory; +import 'dart:io' as io; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter_goldens/flutter_goldens.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as path; import 'package:platform/platform.dart'; import 'package:process/process.dart'; @@ -27,1143 +28,967 @@ const List _kTestPngBytes = [ 78, 68, 174, 66, 96, 130, ]; -void main() { - late MemoryFileSystem fs; - late FakePlatform platform; - late FakeProcessManager process; - late FakeHttpClient fakeHttpClient; +FileSystem createFakeFileSystem() { + return MemoryFileSystem() + ..directory(_kFlutterRoot).createSync(recursive: true); +} - setUp(() { - fs = MemoryFileSystem(); - platform = FakePlatform( +(FileSystem, Directory) createFakeFileSystemWithWorkDirectory() { + final FileSystem fs = createFakeFileSystem(); + final Directory workDirectory = fs.directory('/workDirectory')..createSync(recursive: true); + return (fs, workDirectory); +} + +(FileSystem, Directory) createFakeFileSystemWithLibDirectory() { + final FileSystem fs = createFakeFileSystem(); + final Directory lib = fs.directory('$_kFlutterRoot/test/library/')..createSync(recursive: true); + return (fs, lib); +} + +void main() { + test('SkiaGoldClient - web HTML test', () async { + final Platform platform = FakePlatform( + environment: { + 'GOLDCTL': 'goldctl', + 'FLUTTER_ROOT': _kFlutterRoot, + 'FLUTTER_TEST_BROWSER': 'Chrome', + 'FLUTTER_WEB_RENDERER': 'html', + }, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final io.HttpClient httpClient = ThrowingHttpClient(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: httpClient, + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + final File goldenFile = workDirectory.childFile('temp/golden_file_test.png') + ..createSync(recursive: true); + + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'add', + '--work-dir', '/workDirectory/temp', + '--test-name', 'golden_file_test', + '--png-file', '/workDirectory/temp/golden_file_test.png', + '--passfail', + '--add-test-optional-key', 'image_matching_algorithm:fuzzy', + '--add-test-optional-key', 'fuzzy_max_different_pixels:20', + '--add-test-optional-key', 'fuzzy_pixel_delta_threshold:4', + ], + null, + ); + process.processResults[goldctlInvocation] = io.ProcessResult(123, 0, '', ''); + await skiaClient.imgtestAdd('golden_file_test.png', goldenFile); + }); + + test('SkiaGoldClient - web CanvasKit test', () async { + final Platform platform = FakePlatform( + environment: { + 'GOLDCTL': 'goldctl', + 'FLUTTER_ROOT': _kFlutterRoot, + 'FLUTTER_TEST_BROWSER': 'Chrome', + 'FLUTTER_WEB_RENDERER': 'canvaskit', + }, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final io.HttpClient httpClient = FakeHttpClient(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: httpClient, + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + final File goldenFile = workDirectory.childFile('temp/golden_file_test.png') + ..createSync(recursive: true); + + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'add', + '--work-dir', '/workDirectory/temp', + '--test-name', 'golden_file_test', + '--png-file', '/workDirectory/temp/golden_file_test.png', + '--passfail', + ], + null, + ); + process.processResults[goldctlInvocation] = io.ProcessResult(123, 0, '', ''); + + await skiaClient.imgtestAdd('golden_file_test.png', goldenFile); + }); + + test('SkiaGoldClient - auth performs minimal work if already authorized', () async { + final Platform platform = FakePlatform( environment: {'FLUTTER_ROOT': _kFlutterRoot}, operatingSystem: 'macos' ); - process = FakeProcessManager(); - fakeHttpClient = FakeHttpClient(); - fs.directory(_kFlutterRoot).createSync(recursive: true); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final File authFile = workDirectory.childFile('temp/auth_opt.json') + ..createSync(recursive: true); + authFile.writeAsStringSync(authTemplate()); + process.fallbackProcessResult = io.ProcessResult(123, 0, '', ''); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + await skiaClient.auth(); + expect(process.workingDirectories, isEmpty); }); - group('SkiaGoldClient', () { - late SkiaGoldClient skiaClient; - late Directory workDirectory; + test('SkiaGoldClient - gsutil is checked when authorization file is present', () async { + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final ProcessManager process = FakeProcessManager(); + final File authFile = workDirectory.childFile('temp/auth_opt.json') + ..createSync(recursive: true); + authFile.writeAsStringSync(authTemplate(gsutil: true)); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + expect( + await skiaClient.clientIsAuthorized(), + isFalse, + ); + }); - setUp(() { - workDirectory = fs.directory('/workDirectory') - ..createSync(recursive: true); - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - }); + test('SkiaGoldClient - throws for error state from auth', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLD_SERVICE_ACCOUNT': 'Service Account', + 'GOLDCTL': 'goldctl', + }, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); + expect( + skiaClient.auth(), + throwsException, + ); + }); - test('web HTML test', () async { - platform = FakePlatform( + test('SkiaGoldClient - throws for error state from init', () { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + }, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + const RunInvocation gitInvocation = RunInvocation( + ['git', 'rev-parse', 'HEAD'], + '/flutter', + ); + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'init', + '--instance', 'flutter', + '--work-dir', '/workDirectory/temp', + '--commit', '12345678', + '--keys-file', '/workDirectory/keys.json', + '--failure-file', '/workDirectory/failures.json', + '--passfail', + ], + null, + ); + process.processResults[gitInvocation] = io.ProcessResult(12345678, 0, '12345678', ''); + process.processResults[goldctlInvocation] = io.ProcessResult(123, 1, 'Expected failure', 'Expected failure'); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); + + expect( + skiaClient.imgtestInit(), + throwsException, + ); + }); + + test('SkiaGoldClient - Only calls init once', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + }, + operatingSystem: 'macos', + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + const RunInvocation gitInvocation = RunInvocation( + ['git', 'rev-parse', 'HEAD'], + '/flutter', + ); + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'init', + '--instance', 'flutter', + '--work-dir', '/workDirectory/temp', + '--commit', '1234', + '--keys-file', '/workDirectory/keys.json', + '--failure-file', '/workDirectory/failures.json', + '--passfail', + ], + null, + ); + process.processResults[gitInvocation] = io.ProcessResult(1234, 0, '1234', ''); + process.processResults[goldctlInvocation] = io.ProcessResult(5678, 0, '5678', ''); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); + + // First call + await skiaClient.imgtestInit(); + + // Remove fake process result. + // If the init call is executed again, the fallback process will throw. + process.processResults.remove(goldctlInvocation); + + // Second call + await skiaClient.imgtestInit(); + }); + + test('SkiaGoldClient - Only calls tryjob init once', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + 'SWARMING_TASK_ID': '4ae997b50dfd4d11', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', + 'GOLD_TRYJOB': 'refs/pull/49815/head', + }, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final FakeProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + const RunInvocation gitInvocation = RunInvocation( + ['git', 'rev-parse', 'HEAD'], + '/flutter', + ); + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'init', + '--instance', 'flutter', + '--work-dir', '/workDirectory/temp', + '--commit', '1234', + '--keys-file', '/workDirectory/keys.json', + '--failure-file', '/workDirectory/failures.json', + '--passfail', + '--crs', 'github', + '--patchset_id', '1234', + '--changelist', '49815', + '--cis', 'buildbucket', + '--jobid', '8885996262141582672', + ], + null, + ); + process.processResults[gitInvocation] = io.ProcessResult(1234, 0, '1234', ''); + process.processResults[goldctlInvocation] = io.ProcessResult(5678, 0, '5678', ''); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); + + // First call + await skiaClient.tryjobInit(); + + // Remove fake process result. + // If the init call is executed again, the fallback process will throw. + process.processResults.remove(goldctlInvocation); + + // Second call + await skiaClient.tryjobInit(); + }); + + test('SkiaGoldClient - throws for error state from imgtestAdd', () { + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final File goldenFile = workDirectory.childFile('temp/golden_file_test.png') + ..createSync(recursive: true); + final FakeProcessManager process = FakeProcessManager(); + final Platform platform = FakePlatform( environment: { + 'FLUTTER_ROOT': _kFlutterRoot, 'GOLDCTL': 'goldctl', - 'FLUTTER_ROOT': _kFlutterRoot, - 'FLUTTER_TEST_BROWSER': 'Chrome', - 'FLUTTER_WEB_RENDERER': 'html', }, - operatingSystem: 'macos' - ); - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); + operatingSystem: 'macos', + ); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); - final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png') - ..createSync(recursive: true); + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'add', + '--work-dir', '/workDirectory/temp', + '--test-name', 'golden_file_test', + '--png-file', '/workDirectory/temp/golden_file_test.png', + '--passfail', + ], + null, + ); + process.processResults[goldctlInvocation] = io.ProcessResult(123, 1, 'Expected failure', 'Expected failure'); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - const RunInvocation goldctlInvocation = RunInvocation( + expect( + skiaClient.imgtestAdd('golden_file_test', goldenFile), + throwsException, + ); + }); + + test('SkiaGoldClient - correctly inits tryjob for luci', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + 'SWARMING_TASK_ID': '4ae997b50dfd4d11', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', + 'GOLD_TRYJOB': 'refs/pull/49815/head', + }, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final ProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + final List ciArguments = skiaClient.getCIArguments(); + + expect( + ciArguments, + equals( [ - 'goldctl', - 'imgtest', 'add', - '--work-dir', '/workDirectory/temp', - '--test-name', 'golden_file_test', - '--png-file', '/workDirectory/temp/golden_file_test.png', - '--passfail', - '--add-test-optional-key', 'image_matching_algorithm:fuzzy', - '--add-test-optional-key', 'fuzzy_max_different_pixels:20', - '--add-test-optional-key', 'fuzzy_pixel_delta_threshold:4', - ], - null, - ); - process.processResults[goldctlInvocation] = ProcessResult(123, 0, '', ''); - - expect( - await skiaClient.imgtestAdd('golden_file_test.png', goldenFile), - isTrue, - ); - }); - - test('web CanvasKit test', () async { - platform = FakePlatform( - environment: { - 'GOLDCTL': 'goldctl', - 'FLUTTER_ROOT': _kFlutterRoot, - 'FLUTTER_TEST_BROWSER': 'Chrome', - 'FLUTTER_WEB_RENDERER': 'canvaskit', - }, - operatingSystem: 'macos' - ); - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png') - ..createSync(recursive: true); - - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'add', - '--work-dir', '/workDirectory/temp', - '--test-name', 'golden_file_test', - '--png-file', '/workDirectory/temp/golden_file_test.png', - '--passfail', - ], - null, - ); - process.processResults[goldctlInvocation] = ProcessResult(123, 0, '', ''); - - expect( - await skiaClient.imgtestAdd('golden_file_test.png', goldenFile), - isTrue, - ); - }); - - test('auth performs minimal work if already authorized', () async { - final File authFile = fs.file('/workDirectory/temp/auth_opt.json') - ..createSync(recursive: true); - authFile.writeAsStringSync(authTemplate()); - process.fallbackProcessResult = ProcessResult(123, 0, '', ''); - await skiaClient.auth(); - - expect(process.workingDirectories, isEmpty); - }); - - test('gsutil is checked when authorization file is present', () async { - final File authFile = fs.file('/workDirectory/temp/auth_opt.json') - ..createSync(recursive: true); - authFile.writeAsStringSync(authTemplate(gsutil: true)); - expect( - await skiaClient.clientIsAuthorized(), - isFalse, - ); - }); - - test('throws for error state from auth', () async { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLD_SERVICE_ACCOUNT' : 'Service Account', - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - expect( - skiaClient.auth(), - throwsException, - ); - }); - - test('throws for error state from init', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - const RunInvocation gitInvocation = RunInvocation( - ['git', 'rev-parse', 'HEAD'], - '/flutter', - ); - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'init', - '--instance', 'flutter', - '--work-dir', '/workDirectory/temp', - '--commit', '12345678', - '--keys-file', '/workDirectory/keys.json', - '--failure-file', '/workDirectory/failures.json', - '--passfail', - ], - null, - ); - process.processResults[gitInvocation] = ProcessResult(12345678, 0, '12345678', ''); - process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'Expected failure', 'Expected failure'); - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - expect( - skiaClient.imgtestInit(), - throwsException, - ); - }); - - test('Only calls init once', () async { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - const RunInvocation gitInvocation = RunInvocation( - ['git', 'rev-parse', 'HEAD'], - '/flutter', - ); - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'init', - '--instance', 'flutter', - '--work-dir', '/workDirectory/temp', - '--commit', '1234', - '--keys-file', '/workDirectory/keys.json', - '--failure-file', '/workDirectory/failures.json', - '--passfail', - ], - null, - ); - process.processResults[gitInvocation] = ProcessResult(1234, 0, '1234', ''); - process.processResults[goldctlInvocation] = ProcessResult(5678, 0, '5678', ''); - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - // First call - await skiaClient.imgtestInit(); - - // Remove fake process result. - // If the init call is executed again, the fallback process will throw. - process.processResults.remove(goldctlInvocation); - - // Second call - await skiaClient.imgtestInit(); - }); - - test('Only calls tryjob init once', () async { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - 'SWARMING_TASK_ID' : '4ae997b50dfd4d11', - 'LOGDOG_STREAM_PREFIX' : 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', - 'GOLD_TRYJOB' : 'refs/pull/49815/head', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - const RunInvocation gitInvocation = RunInvocation( - ['git', 'rev-parse', 'HEAD'], - '/flutter', - ); - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'init', - '--instance', 'flutter', - '--work-dir', '/workDirectory/temp', - '--commit', '1234', - '--keys-file', '/workDirectory/keys.json', - '--failure-file', '/workDirectory/failures.json', - '--passfail', - '--crs', 'github', - '--patchset_id', '1234', '--changelist', '49815', '--cis', 'buildbucket', '--jobid', '8885996262141582672', ], - null, - ); - process.processResults[gitInvocation] = ProcessResult(1234, 0, '1234', ''); - process.processResults[goldctlInvocation] = ProcessResult(5678, 0, '5678', ''); - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - // First call - await skiaClient.tryjobInit(); - - // Remove fake process result. - // If the init call is executed again, the fallback process will throw. - process.processResults.remove(goldctlInvocation); - - // Second call - await skiaClient.tryjobInit(); - }); - - test('throws for error state from imgtestAdd', () { - final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png') - ..createSync(recursive: true); - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'add', - '--work-dir', '/workDirectory/temp', - '--test-name', 'golden_file_test', - '--png-file', '/workDirectory/temp/golden_file_test.png', - '--passfail', - ], - null, - ); - process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'Expected failure', 'Expected failure'); - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - expect( - skiaClient.imgtestAdd('golden_file_test', goldenFile), - throwsException, - ); - }); - - test('correctly inits tryjob for luci', () async { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - 'SWARMING_TASK_ID' : '4ae997b50dfd4d11', - 'LOGDOG_STREAM_PREFIX' : 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', - 'GOLD_TRYJOB' : 'refs/pull/49815/head', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - final List ciArguments = skiaClient.getCIArguments(); - - expect( - ciArguments, - equals( - [ - '--changelist', '49815', - '--cis', 'buildbucket', - '--jobid', '8885996262141582672', - ], - ), - ); - }); - - test('Creates traceID correctly', () async { - String traceID; - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - 'SWARMING_TASK_ID' : '4ae997b50dfd4d11', - 'LOGDOG_STREAM_PREFIX' : 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', - 'GOLD_TRYJOB' : 'refs/pull/49815/head', - }, - operatingSystem: 'linux' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - traceID = skiaClient.getTraceID('flutter.golden.1'); - expect( - traceID, - equals('ae18c7a6aa48e0685525dfe8fdf79003'), - ); - - // Browser - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - 'SWARMING_TASK_ID' : '4ae997b50dfd4d11', - 'LOGDOG_STREAM_PREFIX' : 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', - 'GOLD_TRYJOB' : 'refs/pull/49815/head', - 'FLUTTER_TEST_BROWSER' : 'chrome', - }, - operatingSystem: 'linux' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - traceID = skiaClient.getTraceID('flutter.golden.1'); - expect( - traceID, - equals('e9d5c296c48e7126808520e9cc191243'), - ); - - // Locally - should defer to luci traceID - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - traceID = skiaClient.getTraceID('flutter.golden.1'); - expect( - traceID, - equals('9968695b9ae78cdb77cbb2be621ca2d6'), - ); - }); - - test('throws for error state from imgtestAdd', () { - final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png') - ..createSync(recursive: true); - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'add', - '--work-dir', '/workDirectory/temp', - '--test-name', 'golden_file_test', - '--png-file', '/workDirectory/temp/golden_file_test.png', - '--passfail', - ], - null, - ); - process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'Expected failure', 'Expected failure'); - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - expect( - skiaClient.imgtestAdd('golden_file_test', goldenFile), - throwsA( - isA().having((SkiaException error) => error.message, - 'message', - contains('result-state.json'), - ), - ), - ); - }); - - test('throws for error state from tryjobAdd', () { - final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png') - ..createSync(recursive: true); - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos' - ); - - skiaClient = SkiaGoldClient( - workDirectory, - fs: fs, - process: process, - platform: platform, - httpClient: fakeHttpClient, - ); - - const RunInvocation goldctlInvocation = RunInvocation( - [ - 'goldctl', - 'imgtest', 'add', - '--work-dir', '/workDirectory/temp', - '--test-name', 'golden_file_test', - '--png-file', '/workDirectory/temp/golden_file_test.png', - '--passfail', - ], - null, - ); - process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'Expected failure', 'Expected failure'); - process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - - expect( - skiaClient.tryjobAdd('golden_file_test', goldenFile), - throwsA( - isA().having((SkiaException error) => error.message, - 'message', - contains('result-state.json'), - ), - ), - ); - }); - - group('Request Handling', () { - const String expectation = '55109a4bed52acc780530f7a9aeff6c0'; - - test('image bytes are processed properly', () async { - final Uri imageUrl = Uri.parse( - 'https://flutter-gold.skia.org/img/images/$expectation.png' - ); - final FakeHttpClientRequest fakeImageRequest = FakeHttpClientRequest(); - final FakeHttpImageResponse fakeImageResponse = FakeHttpImageResponse( - imageResponseTemplate() - ); - - fakeHttpClient.request = fakeImageRequest; - fakeImageRequest.response = fakeImageResponse; - - final List masterBytes = await skiaClient.getImageBytes(expectation); - - expect(fakeHttpClient.lastUri, imageUrl); - expect(masterBytes, equals(_kTestPngBytes)); - }); - }); + ), + ); }); - group('FlutterGoldenFileComparator', () { - late FlutterGoldenFileComparator comparator; + test('SkiaGoldClient - Creates traceID correctly - Linux', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + 'SWARMING_TASK_ID': '4ae997b50dfd4d11', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', + 'GOLD_TRYJOB': 'refs/pull/49815/head', + }, + operatingSystem: 'linux', + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final ProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + expect( + skiaClient.getTraceID('flutter.golden.1'), + equals('ae18c7a6aa48e0685525dfe8fdf79003'), + ); + }); - setUp(() { - final Directory basedir = fs.directory('flutter/test/library/') - ..createSync(recursive: true); - comparator = FlutterPostSubmitFileComparator( - basedir.uri, - FakeSkiaGoldClient(), - fs: fs, - platform: platform, - ); - }); + test('SkiaGoldClient - Creates traceID correctly - Linux web', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + 'SWARMING_TASK_ID': '4ae997b50dfd4d11', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', + 'GOLD_TRYJOB': 'refs/pull/49815/head', + 'FLUTTER_TEST_BROWSER': 'chrome', // flips browser bit + }, + operatingSystem: 'linux', + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final ProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + expect( + skiaClient.getTraceID('flutter.golden.1'), + equals('e9d5c296c48e7126808520e9cc191243'), + ); + }); - test('calculates the basedir correctly from defaultComparator for local testing', () async { - final FakeLocalFileComparator defaultComparator = FakeLocalFileComparator(); - final Directory flutterRoot = fs.directory(platform.environment['FLUTTER_ROOT']) - ..createSync(recursive: true); - defaultComparator.basedir = flutterRoot.childDirectory('baz').uri; + test('SkiaGoldClient - Creates traceID correctly - Linux', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + 'SWARMING_TASK_ID': '4ae997b50dfd4d11', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/8885996262141582672', + 'GOLD_TRYJOB': 'refs/pull/49815/head', + }, + operatingSystem: 'macos', // different operating system + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final ProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + expect( + skiaClient.getTraceID('flutter.golden.1'), + equals('9968695b9ae78cdb77cbb2be621ca2d6'), + ); + }); - final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory( - defaultComparator, - platform, - ); - expect( - basedir.uri, - fs.directory('/flutter/bin/cache/pkg/skia_goldens/baz').uri, - ); - }); + test('SkiaGoldClient - throws for error state from imgtestAdd', () { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + }, + operatingSystem: 'macos', + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final File goldenFile = workDirectory.childFile('temp/golden_file_test.png') + ..createSync(recursive: true); + final FakeProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); - test('ignores version number', () { - final Uri key = comparator.getTestUri(Uri.parse('foo.png'), 1); - expect(key, Uri.parse('foo.png')); - }); + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'add', + '--work-dir', '/workDirectory/temp', + '--test-name', 'golden_file_test', + '--png-file', '/workDirectory/temp/golden_file_test.png', + '--passfail', + ], + null, + ); + process.processResults[goldctlInvocation] = io.ProcessResult(123, 1, 'Expected failure', 'Expected failure'); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); - test('adds namePrefix', () async { - const String libraryName = 'sidedishes'; - const String namePrefix = 'tomatosalad'; - const String fileName = 'lettuce.png'; - final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); - final Directory basedir = fs.directory('flutter/test/$libraryName/') - ..createSync(recursive: true); - final FlutterGoldenFileComparator comparator = FlutterPostSubmitFileComparator( - basedir.uri, - fakeSkiaClient, - fs: fs, - platform: platform, - namePrefix: namePrefix, - ); + expect( + skiaClient.imgtestAdd('golden_file_test', goldenFile), + throwsA( + isA().having((SkiaException error) => error.message, + 'message', + 'Skia Gold received an unapproved image in post-submit testing. ' + 'Golden file images in flutter/flutter are triaged in pre-submit during code review for the given PR.\n' + '\n' + 'Visit https://flutter-gold.skia.org/ to view and approve the image(s), or revert the associated change. ' + 'For more information, visit the wiki:\n' + ' https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter\n' + '\n' + 'imgtest add failed with exit code 1.\n' + '\n' + 'stdout from gold:\n' + ' Fallback failure\n' + '\n' + 'stderr from gold:\n' + ' Fallback failure\n', + ), + ), + ); + }); + + test('SkiaGoldClient - throws for error state from tryjobAdd', () { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'goldctl', + }, + operatingSystem: 'macos', + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final File goldenFile = workDirectory.childFile('temp/golden_file_test.png') + ..createSync(recursive: true); + final FakeProcessManager process = FakeProcessManager(); + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + + const RunInvocation goldctlInvocation = RunInvocation( + [ + 'goldctl', + 'imgtest', 'add', + '--work-dir', '/workDirectory/temp', + '--test-name', 'golden_file_test', + '--png-file', '/workDirectory/temp/golden_file_test.png', + '--passfail', + ], + null, + ); + process.processResults[goldctlInvocation] = io.ProcessResult(123, 1, 'Expected failure', 'Expected failure'); + process.fallbackProcessResult = io.ProcessResult(123, 1, 'Fallback failure', 'Fallback failure'); + + expect( + skiaClient.tryjobAdd('golden_file_test', goldenFile), + throwsA( + isA().having((SkiaException error) => error.message, + 'message', + 'Golden test for "golden_file_test" failed for a reason unrelated to pixel comparison.\n' + '\n' + 'imgtest add failed with exit code 1.\n' + '\n' + 'stdout from gold:\n' + ' Fallback failure\n' + '\n' + 'stderr from gold:\n' + ' Fallback failure\n', + ), + ), + ); + }); + + test('SkiaGoldClient - Request Handling - image bytes are processed properly', () async { + const String expectation = '55109a4bed52acc780530f7a9aeff6c0'; + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory workDirectory) = createFakeFileSystemWithWorkDirectory(); + final ProcessManager process = FakeProcessManager(); + final Uri imageUrl = Uri.parse( + 'https://flutter-gold.skia.org/img/images/$expectation.png' + ); + final FakeHttpClient fakeHttpClient = FakeHttpClient(); + final FakeHttpClientRequest fakeImageRequest = FakeHttpClientRequest(); + final FakeHttpImageResponse fakeImageResponse = FakeHttpImageResponse( + imageResponseTemplate() + ); + + fakeHttpClient.request = fakeImageRequest; + fakeImageRequest.response = fakeImageResponse; + + final SkiaGoldClient skiaClient = SkiaGoldClient( + workDirectory, + fs: fs, + process: process, + platform: platform, + httpClient: fakeHttpClient, + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + final List masterBytes = await skiaClient.getImageBytes(expectation); + + expect(fakeHttpClient.lastUri, imageUrl); + expect(masterBytes, equals(_kTestPngBytes)); + }); + + test('FlutterGoldenFileComparator - calculates the basedir correctly from defaultComparator for local testing', () async { + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + final FileSystem fs = createFakeFileSystem(); + final FakeLocalFileComparator defaultComparator = FakeLocalFileComparator(); + final Directory flutterRoot = fs.directory(platform.environment['FLUTTER_ROOT']) + ..createSync(recursive: true); + defaultComparator.basedir = flutterRoot.childDirectory('baz').uri; + + final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory( + defaultComparator, + platform: platform, + fs: fs, + ); + expect( + basedir.uri, + fs.directory('/flutter/bin/cache/pkg/skia_goldens/baz').uri, + ); + }); + + test('FlutterGoldenFileComparator - ignores version number', () { + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final List log = []; + final FlutterGoldenFileComparator comparator = FlutterPostSubmitFileComparator( + libDirectory.uri, + FakeSkiaGoldClient(), + fs: fs, + platform: platform, + log: log.add, + ); + final Uri key = comparator.getTestUri(Uri.parse('foo.png'), 1); + expect(key, Uri.parse('foo.png')); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - adds namePrefix', () async { + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + const String libraryName = 'sidedishes'; + const String namePrefix = 'tomatosalad'; + const String fileName = 'lettuce.png'; + final FileSystem fs = createFakeFileSystem(); + final Directory basedir = fs.directory('flutter/test/$libraryName/') + ..createSync(recursive: true); + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final List log = []; + final FlutterGoldenFileComparator comparator = FlutterPostSubmitFileComparator( + basedir.uri, + fakeSkiaClient, + fs: fs, + platform: platform, + log: log.add, + namePrefix: namePrefix, + ); + await comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse(fileName), + ); + expect(fakeSkiaClient.testNames.single, '$namePrefix.$libraryName.$fileName'); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Post-Submit - asserts .png format', () async { + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final List log = []; + final FlutterGoldenFileComparator comparator = FlutterPostSubmitFileComparator( + libDirectory.uri, + fakeSkiaClient, + fs: fs, + platform: platform, + log: log.add, + ); + await expectLater( + () async { + return comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse('flutter.golden_test.1'), + ); + }, + throwsA( + isA().having((AssertionError error) => error.toString(), + 'description', + contains( + 'Golden files in the Flutter framework must end with the file ' + 'extension .png.' + ), + ), + ), + ); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Post-Submit - calls init during compare', () { + final Platform platform = FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos' + ); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final List log = []; + final FlutterGoldenFileComparator comparator = FlutterPostSubmitFileComparator( + libDirectory.uri, + fakeSkiaClient, + fs: fs, + platform: platform, + log: log.add, + ); + expect(fakeSkiaClient.initCalls, 0); + comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse('flutter.golden_test.1.png'), + ); + expect(fakeSkiaClient.initCalls, 1); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Post-Submit - does not call init in during construction', () { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'testctl', + }, + operatingSystem: 'macos' + ); + final FileSystem fs = createFakeFileSystem(); + final List log = []; + FlutterPostSubmitFileComparator.fromLocalFileComparator( + localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix), + platform: platform, + fs: fs, + process: LoggingProcessManager(log), + httpClient: ThrowingHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Pre-Submit - asserts .png format', () async { + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final List log = []; + final FlutterGoldenFileComparator comparator = FlutterPreSubmitFileComparator( + libDirectory.uri, + fakeSkiaClient, + fs: fs, + platform: FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos', + ), + log: log.add, + ); + await expectLater( + () async { + return comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse('flutter.golden_test.1'), + ); + }, + throwsA( + isA().having((AssertionError error) => error.toString(), + 'description', + contains( + 'Golden files in the Flutter framework must end with the file ' + 'extension .png.' + ), + ), + ), + ); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Pre-Submit - calls init during compare', () { + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final List log = []; + final FlutterGoldenFileComparator comparator = FlutterPreSubmitFileComparator( + libDirectory.uri, + fakeSkiaClient, + fs: fs, + platform: FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos', + ), + log: log.add, + ); + expect(fakeSkiaClient.tryInitCalls, 0); + comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse('flutter.golden_test.1.png'), + ); + expect(fakeSkiaClient.tryInitCalls, 1); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Pre-Submit - does not call init in during construction', () async { + final Platform platform = FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + 'GOLDCTL': 'testctl', + }, + operatingSystem: 'macos', + ); + final FileSystem fs = createFakeFileSystem(); + final List log = []; + await FlutterPostSubmitFileComparator.fromLocalFileComparator( + localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix), + platform: platform, + fs: fs, + process: LoggingProcessManager(log), + httpClient: FakeHttpClient(), + log: (String message) => fail('skia gold client printed unexpected output: "$message"'), + ); + expect(log, ['testctl auth --work-dir /.tmp_rand0/flutter_goldens_postsubmit.rand0/../temp --luci']); + }); + + test('FlutterGoldenFileComparator - Local - asserts .png format', () async { + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final List log = []; + final FlutterLocalFileComparator comparator = FlutterLocalFileComparator( + libDirectory.uri, + fakeSkiaClient, + fs: fs, + platform: FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos', + ), + log: log.add, + ); + + const String hash = '55109a4bed52acc780530f7a9aeff6c0'; + fakeSkiaClient.expectationForTestValues['flutter.golden_test.1'] = hash; + fakeSkiaClient.imageBytesValues[hash] =_kTestPngBytes; + fakeSkiaClient.cleanTestNameValues['library.flutter.golden_test.1.png'] = 'flutter.golden_test.1'; + await expectLater( + () async { + return comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse('flutter.golden_test.1'), + ); + }, + throwsA( + isA().having((AssertionError error) => error.toString(), + 'description', + contains( + 'Golden files in the Flutter framework must end with the file ' + 'extension .png.' + ), + ), + ), + ); + expect(log, isEmpty); + }); + + test('FlutterGoldenFileComparator - Local - passes when bytes match', () async { + final List log = []; + final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); + final (FileSystem fs, Directory libDirectory) = createFakeFileSystemWithLibDirectory(); + final FlutterLocalFileComparator comparator = FlutterLocalFileComparator( + libDirectory.uri, + fakeSkiaClient, + fs: fs, + platform: FakePlatform( + environment: {'FLUTTER_ROOT': _kFlutterRoot}, + operatingSystem: 'macos', + ), + log: log.add, + ); + + const String hash = '55109a4bed52acc780530f7a9aeff6c0'; + fakeSkiaClient.expectationForTestValues['flutter.golden_test.1'] = hash; + fakeSkiaClient.imageBytesValues[hash] =_kTestPngBytes; + fakeSkiaClient.cleanTestNameValues['library.flutter.golden_test.1.png'] = 'flutter.golden_test.1'; + expect( await comparator.compare( Uint8List.fromList(_kTestPngBytes), - Uri.parse(fileName), - ); - expect(fakeSkiaClient.testNames.single, '$namePrefix.$libraryName.$fileName'); - }); + Uri.parse('flutter.golden_test.1.png'), + ), + isTrue, + ); + expect(log, isEmpty); + }); - group('Post-Submit', () { - late FakeSkiaGoldClient fakeSkiaClient; - - setUp(() { - fakeSkiaClient = FakeSkiaGoldClient(); - final Directory basedir = fs.directory('flutter/test/library/') - ..createSync(recursive: true); - comparator = FlutterPostSubmitFileComparator( - basedir.uri, - fakeSkiaClient, - fs: fs, - platform: platform, - ); - }); - - test('asserts .png format', () async { - await expectLater( - () async { - return comparator.compare( - Uint8List.fromList(_kTestPngBytes), - Uri.parse('flutter.golden_test.1'), - ); - }, - throwsA( - isA().having((AssertionError error) => error.toString(), - 'description', - contains( - 'Golden files in the Flutter framework must end with the file ' - 'extension .png.' - ), - ), - ), - ); - }); - - test('calls init during compare', () { - expect(fakeSkiaClient.initCalls, 0); - comparator.compare( - Uint8List.fromList(_kTestPngBytes), - Uri.parse('flutter.golden_test.1.png'), - ); - expect(fakeSkiaClient.initCalls, 1); - }); - - test('does not call init in during construction', () { - expect(fakeSkiaClient.initCalls, 0); - FlutterPostSubmitFileComparator.fromDefaultComparator( - platform, - goldens: fakeSkiaClient, - ); - expect(fakeSkiaClient.initCalls, 0); - }); - - group('correctly determines testing environment', () { - test('returns true for configured Luci', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '12345678990', - 'GOLDCTL' : 'goldctl', - 'GIT_BRANCH' : 'master', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns false on release branches in postsubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GIT_BRANCH' : 'flutter-3.16-candidate.0', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns true on master branch in postsubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GIT_BRANCH' : 'master', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns true on main branch in postsubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GIT_BRANCH' : 'main', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns false - GOLDCTL not present', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '12345678990', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns false - GOLD_TRYJOB active', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '12345678990', - 'GOLDCTL' : 'goldctl', - 'GOLD_TRYJOB' : 'git/ref/12345/head', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns false - on Cirrus', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'CIRRUS_CI': 'true', - 'CIRRUS_PR': '', - 'CIRRUS_BRANCH': 'master', - 'GOLD_SERVICE_ACCOUNT': 'service account...', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - }); - }); - - group('Pre-Submit', () { - late FakeSkiaGoldClient fakeSkiaClient; - - setUp(() { - fakeSkiaClient = FakeSkiaGoldClient(); - final Directory basedir = fs.directory('flutter/test/library/') - ..createSync(recursive: true); - comparator = FlutterPreSubmitFileComparator( - basedir.uri, - fakeSkiaClient, - fs: fs, - platform: platform, - ); - }); - - test('asserts .png format', () async { - await expectLater( - () async { - return comparator.compare( - Uint8List.fromList(_kTestPngBytes), - Uri.parse('flutter.golden_test.1'), - ); - }, - throwsA( - isA().having((AssertionError error) => error.toString(), - 'description', - contains( - 'Golden files in the Flutter framework must end with the file ' - 'extension .png.' - ), - ), - ), - ); - }); - - test('calls init during compare', () { - expect(fakeSkiaClient.tryInitCalls, 0); - comparator.compare( - Uint8List.fromList(_kTestPngBytes), - Uri.parse('flutter.golden_test.1.png'), - ); - expect(fakeSkiaClient.tryInitCalls, 1); - }); - - test('does not call init in during construction', () { - expect(fakeSkiaClient.tryInitCalls, 0); - FlutterPostSubmitFileComparator.fromDefaultComparator( - platform, - goldens: fakeSkiaClient, - ); - expect(fakeSkiaClient.tryInitCalls, 0); - }); - - group('correctly determines testing environment', () { - test('returns false on release branches in presubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GOLD_TRYJOB' : 'true', - 'GIT_BRANCH' : 'flutter-3.16-candidate.0', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns true on master branch in presubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GOLD_TRYJOB' : 'true', - 'GIT_BRANCH' : 'master', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns true on main branch in presubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GOLD_TRYJOB' : 'true', - 'GIT_BRANCH' : 'main', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns true for Luci', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '12345678990', - 'GOLDCTL' : 'goldctl', - 'GOLD_TRYJOB' : 'git/ref/12345/head', - 'GIT_BRANCH' : 'master', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns false - not on Luci', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns false - GOLDCTL missing', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '12345678990', - 'GOLD_TRYJOB' : 'git/ref/12345/head', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns false - GOLD_TRYJOB missing', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '12345678990', - 'GOLDCTL' : 'goldctl', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - - test('returns false - on Cirrus', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'CIRRUS_CI': 'true', - 'CIRRUS_PR': '', - 'CIRRUS_BRANCH': 'master', - 'GOLD_SERVICE_ACCOUNT': 'service account...', - }, - operatingSystem: 'macos', - ); - expect( - FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - }); - }); - - group('Skipping', () { - group('correctly determines testing environment', () { - test('returns true on release branches in presubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GOLD_TRYJOB' : 'true', - 'GIT_BRANCH' : 'flutter-3.16-candidate.0', - }, - operatingSystem: 'macos', - ); - expect( - FlutterSkippingFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns true on release branches in postsubmit', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : 'sweet task ID', - 'GOLDCTL' : 'some/path', - 'GIT_BRANCH' : 'flutter-3.16-candidate.0', - }, - operatingSystem: 'macos', - ); - expect( - FlutterSkippingFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns true on Cirrus builds', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'CIRRUS_CI' : 'yep', - }, - operatingSystem: 'macos', - ); - expect( - FlutterSkippingFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns true on irrelevant LUCI builds', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - 'SWARMING_TASK_ID' : '1234567890', - }, - operatingSystem: 'macos' - ); - expect( - FlutterSkippingFileComparator.isAvailableForEnvironment(platform), - isTrue, - ); - }); - - test('returns false - not in CI', () { - platform = FakePlatform( - environment: { - 'FLUTTER_ROOT': _kFlutterRoot, - }, - operatingSystem: 'macos', - ); - expect( - FlutterSkippingFileComparator.isAvailableForEnvironment(platform), - isFalse, - ); - }); - }); - }); - - group('Local', () { - late FlutterLocalFileComparator comparator; - final FakeSkiaGoldClient fakeSkiaClient = FakeSkiaGoldClient(); - - setUp(() async { - final Directory basedir = fs.directory('flutter/test/library/') - ..createSync(recursive: true); - comparator = FlutterLocalFileComparator( - basedir.uri, - fakeSkiaClient, - fs: fs, - platform: FakePlatform( - environment: {'FLUTTER_ROOT': _kFlutterRoot}, - operatingSystem: 'macos', - ), - ); - - const String hash = '55109a4bed52acc780530f7a9aeff6c0'; - fakeSkiaClient.expectationForTestValues['flutter.golden_test.1'] = hash; - fakeSkiaClient.imageBytesValues[hash] =_kTestPngBytes; - fakeSkiaClient.cleanTestNameValues['library.flutter.golden_test.1.png'] = 'flutter.golden_test.1'; - }); - - test('asserts .png format', () async { - await expectLater( - () async { - return comparator.compare( - Uint8List.fromList(_kTestPngBytes), - Uri.parse('flutter.golden_test.1'), - ); - }, - throwsA( - isA().having((AssertionError error) => error.toString(), - 'description', - contains( - 'Golden files in the Flutter framework must end with the file ' - 'extension .png.' - ), - ), - ), - ); - }); - - test('passes when bytes match', () async { - expect( - await comparator.compare( - Uint8List.fromList(_kTestPngBytes), - Uri.parse('flutter.golden_test.1.png'), - ), - isTrue, - ); - }); - - test('returns FlutterSkippingGoldenFileComparator when network connection is unavailable', () async { - final FakeDirectory fakeDirectory = FakeDirectory(); - fakeDirectory.existsSyncValue = true; - fakeDirectory.uri = Uri.parse('/flutter'); - - fakeSkiaClient.getExpectationForTestThrowable = const OSError("Can't reach Gold"); - - FlutterGoldenFileComparator comparator = await FlutterLocalFileComparator.fromDefaultComparator( - platform, - goldens: fakeSkiaClient, - baseDirectory: fakeDirectory, - ); - expect(comparator.runtimeType, FlutterSkippingFileComparator); - - fakeSkiaClient.getExpectationForTestThrowable = const SocketException("Can't reach Gold"); - - comparator = await FlutterLocalFileComparator.fromDefaultComparator( - platform, - goldens: fakeSkiaClient, - baseDirectory: fakeDirectory, - ); - expect(comparator.runtimeType, FlutterSkippingFileComparator); - // reset property or it will carry on to other tests - fakeSkiaClient.getExpectationForTestThrowable = null; - }); - }); + test('FlutterGoldenFileComparator - Local - skips when network connection is unavailable', () async { + final FileSystem fs = createFakeFileSystem(); + final FakeProcessManager process = FakeProcessManager() + ..fallbackProcessResult = io.ProcessResult(123, 1, 'test resulted in a 502: 502 Bad Gateway\n', ''); + final List log = []; + final FlutterGoldenFileComparator comparator = await FlutterLocalFileComparator.fromLocalFileComparator( + localFileComparator: LocalFileComparator(Uri.parse('/test'), pathStyle: path.Style.posix), + platform: FakePlatform( + environment: { + 'FLUTTER_ROOT': _kFlutterRoot, + }, + operatingSystem: 'macos', + ), + fs: fs, + process: process, + httpClient: ThrowingHttpClient(), + log: log.add, + ); + expect( + await comparator.compare( + Uint8List.fromList(_kTestPngBytes), + Uri.parse('flutter.golden_test.1.png'), + ), + isTrue, + ); + expect(log, [ + 'Auto-passing "pkg.flutter.golden_test.1.png" test, ignoring network error when contacting Skia.' + ]); }); } @@ -1207,28 +1032,27 @@ class RunInvocation { } class FakeProcessManager extends Fake implements ProcessManager { - Map processResults = {}; + Map processResults = {}; /// Used if [processResults] does not contain a matching invocation. - ProcessResult? fallbackProcessResult; + io.ProcessResult? fallbackProcessResult; final List workingDirectories = []; @override - Future run( + Future run( List command, { String? workingDirectory, Map? environment, bool includeParentEnvironment = true, bool runInShell = false, - Encoding? stdoutEncoding = systemEncoding, - Encoding? stderrEncoding = systemEncoding, + Encoding? stdoutEncoding, + Encoding? stderrEncoding, }) async { workingDirectories.add(workingDirectory); - final ProcessResult? result = processResults[RunInvocation(command.cast(), workingDirectory)]; + final io.ProcessResult? result = processResults[RunInvocation(command.cast(), workingDirectory)]; if (result == null && fallbackProcessResult == null) { - printOnFailure('ProcessManager.run was called with $command ($workingDirectory) unexpectedly - $processResults.'); - fail('See above.'); + fail('ProcessManager.run was called with $command ($workingDirectory) unexpectedly - $processResults.'); } return result ?? fallbackProcessResult!; } @@ -1280,36 +1104,34 @@ class FakeLocalFileComparator extends Fake implements LocalFileComparator { late Uri basedir; } -class FakeDirectory extends Fake implements Directory { - late bool existsSyncValue; +class ThrowingHttpClient extends Fake implements io.HttpClient { @override - bool existsSync() => existsSyncValue; - - @override - late Uri uri; + Future getUrl(Uri url) async { + throw const io.SocketException('test error'); + } } -class FakeHttpClient extends Fake implements HttpClient { +class FakeHttpClient extends Fake implements io.HttpClient { late Uri lastUri; late FakeHttpClientRequest request; @override - Future getUrl(Uri url) async { + Future getUrl(Uri url) async { lastUri = url; return request; } } -class FakeHttpClientRequest extends Fake implements HttpClientRequest { +class FakeHttpClientRequest extends Fake implements io.HttpClientRequest { late FakeHttpImageResponse response; @override - Future close() async { + Future close() async { return response; } } -class FakeHttpImageResponse extends Fake implements HttpClientResponse { +class FakeHttpImageResponse extends Fake implements io.HttpClientResponse { FakeHttpImageResponse(this.response); final List> response; @@ -1319,3 +1141,22 @@ class FakeHttpImageResponse extends Fake implements HttpClientResponse { response.forEach(action); } } + +class LoggingProcessManager extends Fake implements ProcessManager { + LoggingProcessManager(this.log); + + final List log; + + @override + Future run(List command, { + Map? environment, + bool includeParentEnvironment = true, + bool runInShell = false, + Encoding? stderrEncoding, + Encoding? stdoutEncoding, + String? workingDirectory, + }) async { + log.add(command.join(' ')); + return io.ProcessResult(0, 0, '200', ''); + } +} diff --git a/packages/flutter_goldens/test/skia_client_test.dart b/packages/flutter_goldens/test/skia_client_test.dart new file mode 100644 index 00000000000..ac5f273ae5b --- /dev/null +++ b/packages/flutter_goldens/test/skia_client_test.dart @@ -0,0 +1,90 @@ +// Copyright 2014 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' show HttpClient, ProcessResult; + +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_goldens/skia_client.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:platform/platform.dart'; +import 'package:process/process.dart'; + +void main() { + test('502 retry', () async { + final List log = []; + await runZoned( + zoneSpecification: ZoneSpecification( + print: (Zone self, ZoneDelegate parent, Zone zone, String line) { + fail('unexpected print: "$line"'); + }, + createTimer: (Zone self, ZoneDelegate parent, Zone zone, Duration duration, void Function() f) { + log.add('CREATED TIMER: $duration'); + return parent.createTimer(zone, Duration.zero, f); + }, + ), + () async { + final FileSystem fs; + final SkiaGoldClient skiaClient = SkiaGoldClient( + fs: fs = MemoryFileSystem(), + process: FakeProcessManager(log), + platform: FakePlatform( + environment: const { + 'GOLDCTL': 'goldctl', + }, + ), + httpClient: FakeHttpClient(), + log: log.add, + fs.directory('/'), + ); + log.add('START'); // ignore: avoid_print + await skiaClient.tryjobAdd('test', fs.file('golden')); + log.add('END'); // ignore: avoid_print + expect(log, [ + 'START', + 'EXEC: goldctl imgtest add --work-dir /temp --test-name t --png-file golden', + 'Transient failure (exit code 1) from Skia Gold.', + '', + 'stdout from gold:', + ' test resulted in a 502: 502 Bad Gateway', + ' ', + '', + 'Retrying in 5 seconds.', + 'CREATED TIMER: 0:00:05.000000', + 'EXEC: goldctl imgtest add --work-dir /temp --test-name t --png-file golden', + 'END', + ]); + }, + ); + }); +} + +class FakeProcessManager extends Fake implements ProcessManager { + FakeProcessManager(this.log); + + final List log; + int _index = 0; + + @override + Future run(List command, { + Map? environment, + bool includeParentEnvironment = true, + bool runInShell = false, + Encoding? stderrEncoding, + Encoding? stdoutEncoding, + String? workingDirectory, + }) async { + log.add('EXEC: ${command.join(' ')}'); + _index += 1; + switch (_index) { + case 1: return ProcessResult(0, 1, 'test resulted in a 502: 502 Bad Gateway\n', ''); + case 2: return ProcessResult(0, 0, '200', ''); + default: throw StateError('unexpected call to run'); + } + } +} + +class FakeHttpClient extends Fake implements HttpClient { } diff --git a/packages/flutter_goldens_client/pubspec.yaml b/packages/flutter_goldens_client/pubspec.yaml deleted file mode 100644 index 0942c63a34f..00000000000 --- a/packages/flutter_goldens_client/pubspec.yaml +++ /dev/null @@ -1,22 +0,0 @@ -name: flutter_goldens_client - -environment: - sdk: '>=3.2.0-0 <4.0.0' - -dependencies: - # To update these, use "flutter update-packages --force-upgrade". - crypto: 3.0.3 - file: 7.0.0 - platform: 3.1.3 - process: 5.0.1 - - collection: 1.18.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" - meta: 1.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" - path: 1.9.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" - typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" - -dartdoc: - # Exclude this package from the hosted API docs. - nodoc: true - -# PUBSPEC CHECKSUM: 652c