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.

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! *

dev/automated_tests/flutter_test/flutter_gold_test.dart

@@ -21,6 +21,7 @@ const List<int> _kFailPngBytes = <int>[
 ];
 
 void main() {
+  final List<String> log = <String>[];
   final MemoryFileSystem fs = MemoryFileSystem();
   final Directory basedir = fs.directory('flutter/test/library/')
     ..createSync(recursive: true);
@@ -34,6 +35,7 @@ void main() {
       environment: <String, String>{'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, <String>[
+      // 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, <String>[
+      // 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/',
+    ]);
   });
 }
 

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

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

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

packages/flutter_goldens/dart_test.yaml

@@ -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

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<void> testExecutable(FutureOr<void> Function() testMain, {String? namePrefix}) async {
+  assert(goldenFileComparator is LocalFileComparator);
   const Platform platform = LocalPlatform();
-  if (FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform)) {
+  const FileSystem fs = LocalFileSystem();
-    goldenFileComparator = await FlutterPostSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix);
+  const ProcessManager process = LocalProcessManager();
-  } else if (FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform)) {
+  final io.HttpClient httpClient = io.HttpClient();
-    goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform, namePrefix: namePrefix);
+  if (FlutterPostSubmitFileComparator.isRecommendedForEnvironment(platform)) {
-  } else if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
+    goldenFileComparator = await FlutterPostSubmitFileComparator.fromLocalFileComparator(
-    goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
+      localFileComparator: goldenFileComparator as LocalFileComparator,
-      'Golden file testing is not executed on Cirrus, or LUCI environments outside of flutter/flutter.',
+      namePrefix: namePrefix,
-        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(),
+    required this.fs,
-    this.platform = const LocalPlatform(),
+    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,
+    LocalFileComparator defaultComparator, {
-    Platform platform, {
     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,
+    required super.fs,
-    super.platform,
+    required super.platform,
+    required super.log,
     super.namePrefix,
   });
 
   /// Creates a new [FlutterPostSubmitFileComparator] that mirrors the relative
   /// path resolution of the default [goldenFileComparator].
-  ///
+  static Future<FlutterPostSubmitFileComparator> fromLocalFileComparator({
-  /// The [goldens] and [defaultComparator] parameters are visible for testing
+    required LocalFileComparator localFileComparator,
-  /// purposes only.
-  static Future<FlutterPostSubmitFileComparator> fromDefaultComparator(
-    final Platform platform, {
-    SkiaGoldClient? goldens,
-    LocalFileComparator? defaultComparator,
     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,
+      localFileComparator,
-      platform,
       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);
-
+    skiaClient.imgtestAdd(golden.path, goldenFile); // throws if the result is false
-    return skiaClient.imgtestAdd(golden.path, goldenFile);
+    return true;
   }
 
   /// Decides based on the current environment if goldens tests should be
   /// executed through Skia Gold.
-  static bool isAvailableForEnvironment(Platform platform) {
+  static bool isRecommendedForEnvironment(Platform platform) {
-    final bool luciPostSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
+    return platform.environment.containsKey('SWARMING_TASK_ID') // Indicates LUCI environment.
-      && platform.environment.containsKey('GOLDCTL')
+        && platform.environment.containsKey('GOLDCTL') // Needed to use Gold.
-      // Luci tryjob environments contain this value to inform the [FlutterPreSubmitComparator].
+        && !platform.environment.containsKey('GOLD_TRYJOB') // Indicates a pre-submit environment on LUCI.
-      && !platform.environment.containsKey('GOLD_TRYJOB')
+        && _isMainBranch(platform.environment['GIT_BRANCH']);
-      // Only run on main branch.
-      && _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
-
-    return luciPostSubmit;
   }
 }
 
@@ -291,41 +342,49 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
   FlutterPreSubmitFileComparator(
     super.basedir,
     super.skiaClient, {
-    super.fs,
+    required super.fs,
-    super.platform,
+    required super.platform,
+    required super.log,
     super.namePrefix,
   });
 
   /// Creates a new [FlutterPreSubmitFileComparator] that mirrors the
   /// relative path resolution of the default [goldenFileComparator].
-  ///
+  static Future<FlutterGoldenFileComparator> fromLocalFileComparator({
-  /// The [goldens] and [defaultComparator] parameters are visible for testing
+    required LocalFileComparator localFileComparator,
-  /// purposes only.
-  static Future<FlutterGoldenFileComparator> fromDefaultComparator(
-    final Platform platform, {
-    SkiaGoldClient? goldens,
-    LocalFileComparator? defaultComparator,
     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,
+      localFileComparator,
-      platform,
       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) {
+  static bool isRecommendedForEnvironment(Platform platform) {
-    final bool luciPreSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
+    return platform.environment.containsKey('SWARMING_TASK_ID') // Indicates LUCI environment.
-      && platform.environment.containsKey('GOLDCTL')
+        && platform.environment.containsKey('GOLDCTL') // Needed to use Gold.
-      && platform.environment.containsKey('GOLD_TRYJOB')
+        && platform.environment.containsKey('GOLD_TRYJOB') // Indicates a pre-submit environment on LUCI.
-      // Only run on the main branch
+        && _isMainBranch(platform.environment['GIT_BRANCH']);
-      && _kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
-    return luciPreSubmit;
   }
 }
 
 /// 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
+/// Currently, this comparator is used on Cirrus, or in Luci environments when
-/// outside of the flutter/flutter repository.
+/// 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(
+  static FlutterSkippingFileComparator fromLocalFileComparator({
-    String reason, {
+    required LocalFileComparator localFileComparator,
-    LocalFileComparator? defaultComparator,
     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;
+    final Uri basedir = localFileComparator.basedir;
-    const FileSystem fs = LocalFileSystem();
+    final SkiaGoldClient skiaClient = SkiaGoldClient(
-    final Uri basedir = defaultComparator.basedir;
+      fs.directory(basedir),
-    final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir));
+      fs: fs,
-    return FlutterSkippingFileComparator(basedir, skiaClient, reason, namePrefix: namePrefix);
+      process: process,
+      platform: platform,
+      httpClient: httpClient,
+      log: log,
+    );
+    return FlutterSkippingFileComparator(
+      basedir,
+      skiaClient,
+      reason,
+      fs: fs,
+      platform: platform,
+      log: log,
+      namePrefix: namePrefix,
+    );
   }
 
   @override
   Future<bool> compare(Uint8List imageBytes, Uri golden) async {
-    // Ideally we would use markTestSkipped here but in some situations,
+    log('Auto-passing "$golden" test without checking: $reason');
-    // comparators are called outside of tests.
-    // See also: https://github.com/flutter/flutter/issues/91285
-    // ignore: avoid_print
-    print('Skipping "$golden" test: $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.
+  /// comparators (determined by checking this after the others), we skip.
-  static bool isAvailableForEnvironment(Platform platform) {
+  static bool isRecommendedForEnvironment(Platform platform) {
-    return (platform.environment.containsKey('SWARMING_TASK_ID')
+    return platform.environment.containsKey('SWARMING_TASK_ID') // Indicates LUCI environment.
-      // Some builds are still being run on Cirrus, we should skip these.
+        || platform.environment.containsKey('CIRRUS_CI'); // Indicates Cirrus environment.
-      || platform.environment.containsKey('CIRRUS_CI'))
-      // If we are in CI, skip on branches that are not main.
-      && !_kMainBranch.hasMatch(platform.environment['GIT_BRANCH'] ?? '');
   }
 }
 
@@ -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,
+    required super.fs,
-    super.platform,
+    required super.platform,
+    required super.log,
   });
 
   /// Creates a new [FlutterLocalFileComparator] that mirrors the
   /// relative path resolution of the default [goldenFileComparator].
-  ///
+  static Future<FlutterGoldenFileComparator> fromLocalFileComparator({
-  /// The [goldens], [defaultComparator], and [baseDirectory] parameters are
+    required LocalFileComparator localFileComparator,
-  /// visible for testing purposes only.
+    required Platform platform,
-  static Future<FlutterGoldenFileComparator> fromDefaultComparator(
+    required FileSystem fs,
-    final Platform platform, {
+    required ProcessManager process,
-    SkiaGoldClient? goldens,
+    required io.HttpClient httpClient,
-    LocalFileComparator? defaultComparator,
+    required LogCallback log,
-    Directory? baseDirectory,
   }) async {
-    defaultComparator ??= goldenFileComparator as LocalFileComparator;
+    final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
-    baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
+      localFileComparator,
-      defaultComparator,
+      platform: platform,
-      platform,
+      fs: fs,
     );
-
     if (!baseDirectory.existsSync()) {
       baseDirectory.createSync(recursive: true);
     }
 
-    goldens ??= SkiaGoldClient(baseDirectory);
+    final SkiaGoldClient goldens = SkiaGoldClient(
-    try {
+      baseDirectory,
-      // Check if we can reach Gold.
+      fs: fs,
-      await goldens.getExpectationForTest('');
+      process: process,
-    } on io.OSError catch (_) {
+      platform: platform,
-      return FlutterSkippingFileComparator(
+      httpClient: httpClient,
-        baseDirectory.uri,
+      log: log,
-        goldens,
+    );
-        'OSError occurred, could not reach Gold. '
+    return FlutterLocalFileComparator(
-          'Switching to FlutterSkippingGoldenFileComparator.',
+      baseDirectory.uri,
-      );
+      goldens,
-    } on io.SocketException catch (_) {
+      fs: fs,
-      return FlutterSkippingFileComparator(
+      platform: platform,
-        baseDirectory.uri,
+      log: log,
-        goldens,
+    );
-        'SocketException occurred, could not reach Gold. '
-          'Switching to FlutterSkippingGoldenFileComparator.',
-      );
-    }
-
-    return FlutterLocalFileComparator(baseDirectory.uri, goldens);
   }
 
   @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);
+    try {
-
+      testExpectation = await skiaClient.getExpectationForTest(testName);
-    if (testExpectation == null || testExpectation.isEmpty) {
+      if (testExpectation == null || testExpectation.isEmpty) {
-      // There is no baseline for this test.
+        log(
-      // Ideally we would use markTestSkipped here but in some situations,
+          'No expectations provided by Skia Gold for test: $golden. '
-      // comparators are called outside of tests.
+          'This may be a new test. If this is an unexpected result, check '
-      // See also: https://github.com/flutter/flutter/issues/91285
+          'https://flutter-gold.skia.org.\n'
-      // ignore: avoid_print
+          'Validate image output found at $basedir'
-      print(
+        );
-        'No expectations provided by Skia Gold for test: $golden. '
+        update(golden, imageBytes);
-        'This may be a new test. If this is an unexpected result, check '
+        return true;
-        'https://flutter-gold.skia.org.\n'
+      }
-        'Validate image output found at $basedir'
+    } on Exception catch (error) {
-      );
+      if (error is! io.SocketException &&
-      update(golden, imageBytes);
+          error is! io.OSError) {
+        rethrow; // "uncaught error"
+      }
+      log('Auto-passing "$golden" test, ignoring network error when contacting Skia.');
       return true;
     }
 

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(),
+    required this.fs,
-    this.process = const LocalProcessManager(),
+    required this.process,
-    this.platform = const LocalPlatform(),
+    required this.platform,
-    io.HttpClient? httpClient,
+    required this.httpClient,
-  }) : httpClient = httpClient ?? io.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('  <empty>');
+    } 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<void> _retry({
+    required Future<io.ProcessResult> 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<void>.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<void> auth() async {
     if (await clientIsAuthorized()) {
       return;
     }
-    final List<String> authCommand = <String>[
-      _goldctl,
-      'auth',
-      '--work-dir', workDirectory
-        .childDirectory('temp')
-        .path,
-      '--luci',
-    ];
 
-    final io.ProcessResult result = await process.run(authCommand);
+    await _retry(
-
+      task: () => process.run(<String>[
-    if (result.exitCode != 0) {
+        _goldctl,
-      final StringBuffer buf = StringBuffer()
+        'auth',
-        ..writeln('Skia Gold authorization failed.')
+        '--work-dir', workDirectory
-        ..writeln('Luci environments authenticate using the file provided '
+          .childDirectory('temp')
-          'by LUCI_CONTEXT. There may be an error with this file or Gold '
+          .path,
-          'authentication.')
+        '--luci',
-        ..writeln('Debug information for Gold --------------------------------')
+      ]),
-        ..writeln('stdout: ${result.stdout}')
+      taskName: 'auth',
-        ..writeln('stderr: ${result.stderr}');
+      errorMessage: 'Skia Gold authorization failed.\n'
-      throw SkiaException(buf.toString());
+                    '\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<void> 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);
+    await _retry(
-
+      task: () => process.run(imgtestInitCommand),
-    if (result.exitCode != 0) {
+      taskName: 'imgtest init',
-      _initialized = false;
+      errorMessage: 'An error occurred when initializing golden file test with goldctl.',
-      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());
-    }
     _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<bool> imgtestAdd(String testName, File goldenFile) async {
+  Future<void> imgtestAdd(String testName, File goldenFile) async {
-    final List<String> imgtestCommand = <String>[
+    await _retry(
-      _goldctl,
+      task: () => process.run(<String>[
-      'imgtest', 'add',
+        _goldctl,
-      '--work-dir', workDirectory
+        'imgtest', 'add',
-        .childDirectory('temp')
+        '--work-dir', workDirectory
-        .path,
+          .childDirectory('temp')
-      '--test-name', cleanTestName(testName),
+          .path,
-      '--png-file', goldenFile.path,
+        '--test-name', cleanTestName(testName),
-      '--passfail',
+        '--png-file', goldenFile.path,
-      ..._getPixelMatchingArguments(),
+        '--passfail',
-    ];
+        ..._getPixelMatchingArguments(),
-
+      ]),
-    final io.ProcessResult result = await process.run(imgtestCommand);
+      taskName: 'imgtest add',
-
+      errorMessage: 'Skia Gold received an unapproved image in post-submit '
-    if (result.exitCode != 0) {
+                    'testing. Golden file images in flutter/flutter are triaged '
-      // If an unapproved image has made it to post-submit, throw to close the
+                    'in pre-submit during code review for the given PR.\n'
-      // tree.
+                    '\n'
-      String? resultContents;
+                    'Visit https://flutter-gold.skia.org/ to view and approve '
-      final File resultFile = workDirectory.childFile(fs.path.join(
+                    'the image(s), or revert the associated change. For more '
-        'result-state.json',
+                    'information, visit the wiki:\n'
-      ));
+                    '  https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter'
-      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;
   }
 
   /// 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<void> 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);
+    await _retry(
-
+      task: () => process.run(imgtestInitCommand),
-    if (result.exitCode != 0) {
+      taskName: 'imgtest init',
-      _tryjobInitialized = false;
+      errorMessage: 'An error occurred when initializing golden file tryjob with goldctl.'
-      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());
-    }
     _tryjobInitialized = true;
   }
 
@@ -324,42 +365,23 @@ class SkiaGoldClient {
   /// The [testName] and [goldenFile] parameters reference the current
   /// comparison being evaluated by the [FlutterPreSubmitFileComparator].
   Future<void> tryjobAdd(String testName, File goldenFile) async {
-    final List<String> imgtestCommand = <String>[
+    await _retry(
-      _goldctl,
+      task: () => process.run(<String>[
-      'imgtest', 'add',
+        _goldctl,
-      '--work-dir', workDirectory
+        'imgtest', 'add',
-        .childDirectory('temp')
+        '--work-dir', workDirectory.childDirectory('temp').path,
-        .path,
+        '--test-name', cleanTestName(testName),
-      '--test-name', cleanTestName(testName),
+        '--png-file', goldenFile.path,
-      '--png-file', goldenFile.path,
+        ..._getPixelMatchingArguments(),
-      ..._getPixelMatchingArguments(),
+      ]),
-    ];
+      taskName: 'imgtest add',
-
+      success: (int exitCode, String resultStdout, String resultStderr) {
-    final io.ProcessResult result = await process.run(imgtestCommand);
+        return exitCode == 0
-
+            || resultStdout.contains('Untriaged')
-    final String/*!*/ resultStdout = result.stdout.toString();
+            || resultStdout.contains('negative image');
-    if (result.exitCode != 0 &&
+      },
-      !(resultStdout.contains('Untriaged') || resultStdout.contains('negative image'))) {
+      errorMessage: 'Golden test for "$testName" failed for a reason unrelated to pixel comparison.',
-      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());
-    }
   }
 
   // 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<String?> getExpectationForTest(String testName) async {
-    late String? expectation;
     final String traceID = getTraceID(testName);
-    await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
+    final Uri requestForExpectations = Uri.parse(
-      final Uri requestForExpectations = Uri.parse(
+      'https://flutter-gold.skia.org/json/v2/latestpositivedigest/$traceID'
-        '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<String, dynamic>) {
-          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(),
     );
-    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<String, dynamic>) {
+        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<List<int>>getImageBytes(String imageHash) async {
+  Future<List<int>> getImageBytes(String imageHash) async {
     final List<int> imageBytes = <int>[];
-    await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
+    final Uri requestForImage = Uri.parse(
-      final Uri requestForImage = Uri.parse(
+      'https://flutter-gold.skia.org/img/images/$imageHash.png',
-        '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<int> bytes) => imageBytes.addAll(bytes));
-    },
-      SkiaGoldHttpOverrides(),
     );
+    final io.HttpClientRequest request = await httpClient.getUrl(requestForImage);
+    final io.HttpClientResponse response = await request.close();
+    await response.forEach((List<int> 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 { }

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"

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: <String, String>{
+      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);
+  });
+}

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<String> log = <String>[];
+    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 <String, String>{
+              '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, <String>[
+          '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<String> log;
+  int _index = 0;
+
+  @override
+  Future<ProcessResult> run(List<Object> command, {
+    Map<String, String>? 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 { }

packages/flutter_goldens_client/pubspec.yaml

@@ -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