Gold Pre-submit flow for contributors without permissions (#47551)

packages/flutter/lib/src/animation/animation_controller.dart

@@ -248,7 +248,8 @@ class AnimationController extends Animation<double>
     _internalSetValue(value ?? lowerBound);
   }
 
-  /// Creates an animation controller with no upper or lower bound for its value.
+  /// Creates an animation controller with no upper or lower bound for its
+  /// value.
   ///
   /// * [value] is the initial value of the animation.
   ///

packages/flutter_goldens/lib/flutter_goldens.dart

@@ -268,10 +268,6 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
 ///  * [FlutterLocalFileComparator], another
 ///    [FlutterGoldenFileComparator] that tests golden images locally on your
 ///    current machine.
-// TODO(Piinks): Better handling for first-time contributors that cannot decrypt
-// the service account is needed. Gold has a new feature `goldctl imgtest check`
-// that could work. There is also the previous implementation that did not use
-// goldctl for this edge case. https://github.com/flutter/flutter/issues/46687
 class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
   /// Creates a [FlutterPreSubmitFileComparator] that will test golden file
   /// images against baselines requested from Flutter Gold.
@@ -299,41 +295,132 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
     final Platform platform, {
     SkiaGoldClient goldens,
     LocalFileComparator defaultComparator,
+    final Directory testBasedir,
   }) async {
 
     defaultComparator ??= goldenFileComparator as LocalFileComparator;
-    final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
+    final Directory baseDirectory = testBasedir ?? FlutterGoldenFileComparator.getBaseDirectory(
       defaultComparator,
       platform,
       suffix: '${math.Random().nextInt(10000)}',
     );
-    baseDirectory.createSync(recursive: true);
+
+    if (!baseDirectory.existsSync())
+      baseDirectory.createSync(recursive: true);
 
     goldens ??= SkiaGoldClient(baseDirectory);
-    await goldens.auth();
-    await goldens.tryjobInit();
-    return FlutterPreSubmitFileComparator(baseDirectory.uri, goldens);
+
+    final bool hasWritePermission = !platform.environment['GOLD_SERVICE_ACCOUNT'].startsWith('ENCRYPTED');
+    if (hasWritePermission) {
+      await goldens.auth();
+      await goldens.tryjobInit();
+      return _AuthorizedFlutterPreSubmitComparator(
+        baseDirectory.uri,
+        goldens,
+        platform: platform,
+      );
+    }
+
+    goldens.emptyAuth();
+    return _UnauthorizedFlutterPreSubmitComparator(
+      baseDirectory.uri,
+      goldens,
+      platform: platform,
+    );
   }
 
   @override
   Future<bool> compare(Uint8List imageBytes, Uri golden) async {
-    golden = _addPrefix(golden);
-    await update(golden, imageBytes);
-    final File goldenFile = getGoldenFile(golden);
-
-    return skiaClient.tryjobAdd(golden.path, goldenFile);
+    assert(
+      false,
+      'The FlutterPreSubmitFileComparator has been used to execute a golden '
+      'file test; this should never happen. Presubmit golden file testing '
+      'should be executed by either the _AuthorizedFlutterPreSubmitComparator '
+      'or the _UnauthorizedFlutterPreSubmitComparator based on contributor '
+      'permissions.'
+    );
+    return false;
   }
 
   /// Decides based on the current environment whether goldens tests should be
   /// performed as pre-submit tests with Skia Gold.
   static bool isAvailableForEnvironment(Platform platform) {
     final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
-    final bool hasWritePermission = platform.environment['CIRRUS_USER_PERMISSION'] == 'admin'
-      || platform.environment['CIRRUS_USER_PERMISSION'] == 'write';
     return platform.environment.containsKey('CIRRUS_CI')
       && cirrusPR.isNotEmpty
-      && platform.environment.containsKey('GOLD_SERVICE_ACCOUNT')
-      && hasWritePermission;
+      && platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
+  }
+}
+
+class _AuthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileComparator {
+  _AuthorizedFlutterPreSubmitComparator(
+    final Uri basedir,
+    final SkiaGoldClient skiaClient, {
+    final FileSystem fs = const LocalFileSystem(),
+    final Platform platform = const LocalPlatform(),
+  }) : super(
+    basedir,
+    skiaClient,
+    fs: fs,
+    platform: platform,
+  );
+
+  @override
+  Future<bool> compare(Uint8List imageBytes, Uri golden) async {
+    golden = _addPrefix(golden);
+    await update(golden, imageBytes);
+    final File goldenFile = getGoldenFile(golden);
+
+    return skiaClient.tryjobAdd(golden.path, goldenFile);
+  }
+}
+
+class _UnauthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileComparator {
+  _UnauthorizedFlutterPreSubmitComparator(
+    final Uri basedir,
+    final SkiaGoldClient skiaClient, {
+    final FileSystem fs = const LocalFileSystem(),
+    final Platform platform = const LocalPlatform(),
+  }) : super(
+    basedir,
+    skiaClient,
+    fs: fs,
+    platform: platform,
+  );
+
+  @override
+  Future<bool> compare(Uint8List imageBytes, Uri golden) async {
+    golden = _addPrefix(golden);
+    await update(golden, imageBytes);
+    final File goldenFile = getGoldenFile(golden);
+
+    // Check for match to existing baseline.
+    if (await skiaClient.imgtestCheck(golden.path, goldenFile))
+      return true;
+
+    // We do not have a matching image, so we need to check a few things
+    // manually. We wait until this point to do this work so request traffic
+    // low.
+    skiaClient.getExpectations();
+    final String testName = skiaClient.cleanTestName(golden.path);
+    final List<String> testExpectations = skiaClient.expectations[testName];
+    if (testExpectations == null) {
+      // This is a new test.
+      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'
+      );
+      return true;
+    }
+
+    // Contributors without the proper permissions to execute a tryjob can make
+    // a golden file change through Gold's ignore feature instead.
+    final bool ignoreResult = await skiaClient.testIsIgnoredForPullRequest(
+      platform.environment['CIRRUS_PR'] ?? '',
+      golden.path,
+    );
+    // If true, this is an intended change.
+    return ignoreResult;
   }
 }
 
@@ -399,12 +486,8 @@ class FlutterSkippingGoldenFileComparator extends FlutterGoldenFileComparator {
   /// Decides based on the current environment whether this comparator should be
   /// used.
   static bool isAvailableForEnvironment(Platform platform) {
-    return (platform.environment.containsKey('SWARMING_TASK_ID')
-      || platform.environment.containsKey('CIRRUS_CI'))
-      // A service account means that this is a Gold shard. At this point, it
-      // means we don't have permission to use the account, so we will pass
-      // through to the [FlutterLocalFileComparator].
-      && !platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
+    return platform.environment.containsKey('SWARMING_TASK_ID')
+      || platform.environment.containsKey('CIRRUS_CI');
   }
 }
 

packages/flutter_goldens/test/flutter_goldens_test.dart

@@ -156,12 +156,14 @@ void main() {
 
     group('Request Handling', () {
       String testName;
+      String pullRequestNumber;
       String expectation;
       Uri url;
       MockHttpClientRequest mockHttpRequest;
 
       setUp(() {
         testName = 'flutter.golden_test.1.png';
+        pullRequestNumber = '1234';
         expectation = '55109a4bed52acc780530f7a9aeff6c0';
         mockHttpRequest = MockHttpClientRequest();
       });
@@ -282,6 +284,120 @@ void main() {
         expect(masterBytes, equals(_kTestPngBytes));
       });
 
+      group('ignores', () {
+        Uri url;
+        MockHttpClientRequest mockHttpRequest;
+        MockHttpClientResponse mockHttpResponse;
+
+        setUp(() {
+          url = Uri.parse('https://flutter-gold.skia.org/json/ignores');
+          mockHttpRequest = MockHttpClientRequest();
+          mockHttpResponse = MockHttpClientResponse(utf8.encode(
+            ignoreResponseTemplate(
+              pullRequestNumber: pullRequestNumber,
+              expires: DateTime.now()
+                .add(const Duration(days: 1))
+                .toString(),
+              otherTestName: 'unrelatedTest.1'
+            )
+          ));
+          when(mockHttpClient.getUrl(url))
+            .thenAnswer((_) => Future<MockHttpClientRequest>.value(mockHttpRequest));
+          when(mockHttpRequest.close())
+            .thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
+        });
+
+        test('returns true for ignored test and ignored pull request number', () async {
+          expect(
+            await skiaClient.testIsIgnoredForPullRequest(
+              pullRequestNumber,
+              testName,
+            ),
+            isTrue,
+          );
+        });
+
+        test('returns true for ignored test and not ignored pull request number', () async {
+          expect(
+            await skiaClient.testIsIgnoredForPullRequest(
+              '5678',
+              testName,
+            ),
+            isTrue,
+          );
+        });
+
+        test('returns false for not ignored test and ignored pull request number', () async {
+          expect(
+            await skiaClient.testIsIgnoredForPullRequest(
+              pullRequestNumber,
+              'failure.png',
+            ),
+            isFalse,
+          );
+        });
+
+        test('throws exception for expired ignore', () async {
+          mockHttpResponse = MockHttpClientResponse(utf8.encode(
+            ignoreResponseTemplate(
+              pullRequestNumber: pullRequestNumber,
+            )
+          ));
+          when(mockHttpRequest.close())
+            .thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
+          final Future<bool> test = skiaClient.testIsIgnoredForPullRequest(
+            pullRequestNumber,
+            testName,
+          );
+          expect(
+            test,
+            throwsException,
+          );
+        });
+
+        test('throws exception for first expired ignore among multiple', () async {
+          mockHttpResponse = MockHttpClientResponse(utf8.encode(
+            ignoreResponseTemplate(
+              pullRequestNumber: pullRequestNumber,
+              otherExpires: DateTime.now()
+                .add(const Duration(days: 1))
+                .toString(),
+            )
+          ));
+          when(mockHttpRequest.close())
+            .thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
+          final Future<bool> test = skiaClient.testIsIgnoredForPullRequest(
+            pullRequestNumber,
+            testName,
+          );
+          expect(
+            test,
+            throwsException,
+          );
+        });
+
+        test('throws exception for later expired ignore among multiple', () async {
+          mockHttpResponse = MockHttpClientResponse(utf8.encode(
+            ignoreResponseTemplate(
+              pullRequestNumber: pullRequestNumber,
+              expires: DateTime.now()
+                .add(const Duration(days: 1))
+                .toString(),
+            )
+          ));
+          when(mockHttpRequest.close())
+            .thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
+          final Future<bool> test = skiaClient.testIsIgnoredForPullRequest(
+            pullRequestNumber,
+            testName,
+          );
+          expect(
+            test,
+            throwsException,
+          );
+        });
+      });
+
       group('digest parsing', () {
         Uri url;
         MockHttpClientRequest mockHttpRequest;
@@ -463,6 +579,9 @@ void main() {
     });
 
     group('Pre-Submit', () {
+      FlutterGoldenFileComparator comparator;
+      final MockSkiaGoldClient mockSkiaClient = MockSkiaGoldClient();
+
       group('correctly determines testing environment', () {
         test('returns true', () {
           platform = FakePlatform(
@@ -471,7 +590,6 @@ void main() {
               'CIRRUS_CI': 'true',
               'CIRRUS_PR': '1234',
               'GOLD_SERVICE_ACCOUNT' : 'service account...',
-              'CIRRUS_USER_PERMISSION' : 'write',
             },
             operatingSystem: 'macos'
           );
@@ -524,72 +642,110 @@ void main() {
             isFalse,
           );
         });
+      });
 
-        test('returns true - admin privileges', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...',
-              'CIRRUS_USER_PERMISSION' : 'admin',
-            },
-            operatingSystem: 'macos'
+      group('_Authorized', () {
+        setUp(() async {
+          final Directory basedir = fs.directory('flutter/test/library/')
+            ..createSync(recursive: true);
+          comparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(
+            FakePlatform(
+              environment: <String, String>{
+                'FLUTTER_ROOT': _kFlutterRoot,
+                'CIRRUS_CI' : 'true',
+                'CIRRUS_PR' : '1234',
+                'GOLD_SERVICE_ACCOUNT' : 'service account...',
+                'CIRRUS_USER_PERMISSION' : 'admin',
+              },
+              operatingSystem: 'macos'
+            ),
+            goldens: mockSkiaClient,
+            testBasedir: basedir,
           );
+        });
+
+        test('fromDefaultComparator chooses correct comparator', () async {
           expect(
-            FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
-            isTrue,
+            comparator.runtimeType.toString(),
+            '_AuthorizedFlutterPreSubmitComparator',
           );
         });
+      });
 
-        test('returns true - write privileges', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...',
-              'CIRRUS_USER_PERMISSION' : 'write',
-            },
-            operatingSystem: 'macos'
+      group('_UnAuthorized', () {
+        setUp(() async {
+          final Directory basedir = fs.directory('flutter/test/library/')
+            ..createSync(recursive: true);
+          comparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(
+            FakePlatform(
+              environment: <String, String>{
+                'FLUTTER_ROOT': _kFlutterRoot,
+                'CIRRUS_CI' : 'true',
+                'CIRRUS_PR' : '1234',
+                'GOLD_SERVICE_ACCOUNT' : 'ENCRYPTED[...]',
+                'CIRRUS_USER_PERMISSION' : 'none',
+              },
+              operatingSystem: 'macos'
+            ),
+            goldens: mockSkiaClient,
+            testBasedir: basedir,
           );
+          when(mockSkiaClient.cleanTestName('library.flutter.golden_test.1.png'))
+            .thenReturn('flutter.golden_test.1');
+          when(mockSkiaClient.expectations)
+            .thenReturn(expectationsTemplate());
+        });
+
+        test('fromDefaultComparator chooses correct comparator', () async {
           expect(
-            FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
-            isTrue,
+            comparator.runtimeType.toString(),
+            '_UnauthorizedFlutterPreSubmitComparator',
           );
         });
 
-        test('returns false - read privileges', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...',
-              'CIRRUS_USER_PERMISSION' : 'read',
-            },
-            operatingSystem: 'macos'
+        test('comparison passes test that is ignored for this PR', () async {
+          when(mockSkiaClient.imgtestCheck(any, any))
+            .thenAnswer((_) => Future<bool>.value(false));
+          when(mockSkiaClient.testIsIgnoredForPullRequest(
+            '1234',
+            'library.flutter.golden_test.1.png',
+          ))
+            .thenAnswer((_) => Future<bool>.value(true));
+          expect(
+            await comparator.compare(
+              Uint8List.fromList(_kFailPngBytes),
+              Uri.parse('flutter.golden_test.1.png'),
+            ),
+            isTrue,
           );
+        });
+
+        test('fails test that is not ignored', () async {
+          when(mockSkiaClient.getImageBytes('55109a4bed52acc780530f7a9aeff6c0'))
+            .thenAnswer((_) => Future<List<int>>.value(_kTestPngBytes));
+          when(mockSkiaClient.testIsIgnoredForPullRequest(
+            '1234',
+            'library.flutter.golden_test.1.png',
+          ))
+            .thenAnswer((_) => Future<bool>.value(false));
           expect(
-            FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
+            await comparator.compare(
+              Uint8List.fromList(_kFailPngBytes),
+              Uri.parse('flutter.golden_test.1.png'),
+            ),
             isFalse,
           );
         });
 
-        test('returns false - no privileges', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...',
-              'CIRRUS_USER_PERMISSION' : 'none',
-            },
-            operatingSystem: 'macos'
-          );
+        test('passes non-existent baseline for new test', () async {
+          when(mockSkiaClient.cleanTestName('library.flutter.new_golden_test.1.png'))
+            .thenReturn('flutter.new_golden_test.1');
           expect(
-            FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
-            isFalse,
+            await comparator.compare(
+              Uint8List.fromList(_kFailPngBytes),
+              Uri.parse('flutter.new_golden_test.1.png'),
+            ),
+            isTrue,
           );
         });
       });
@@ -637,22 +793,6 @@ void main() {
             isFalse,
           );
         });
-
-        test('returns false - permission pass through', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI' : 'yep',
-              'GOLD_SERVICE_ACCOUNT' : 'This is a Gold shard!',
-            },
-            operatingSystem: 'macos'
-          );
-          expect(
-            FlutterSkippingGoldenFileComparator.isAvailableForEnvironment(
-              platform),
-            isFalse,
-          );
-        });
       });
     });
 

packages/flutter_goldens/test/json_templates.dart

@@ -169,6 +169,37 @@ String digestResponseTemplate({
   ''';
 }
 
+/// Json response template for Skia Gold ignore request:
+/// https://flutter-gold.skia.org/json/ignores
+String ignoreResponseTemplate({
+  String pullRequestNumber = '0000',
+  String testName = 'flutter.golden_test.1',
+  String otherTestName = 'flutter.golden_test.1',
+  String expires = '2019-09-06T21:28:18.815336Z',
+  String otherExpires = '2019-09-06T21:28:18.815336Z',
+}) {
+  return '''
+    [
+      {
+        "id": "7579425228619212078",
+        "name": "contributor@getMail.com",
+        "updatedBy": "contributor@getMail.com",
+        "expires": "$expires",
+        "query": "ext=png&name=$testName",
+        "note": "https://github.com/flutter/flutter/pull/$pullRequestNumber"
+      },
+      {
+        "id": "7579425228619212078",
+        "name": "contributor@getMail.com",
+        "updatedBy": "contributor@getMail.com",
+        "expires": "$otherExpires",
+        "query": "ext=png&name=$otherTestName",
+        "note": "https://github.com/flutter/flutter/pull/99999"
+      }
+    ]
+  ''';
+}
+
 /// Json response template for Skia Gold image request:
 /// https://flutter-gold.skia.org/img/images/[imageHash].png
 List<List<int>> imageResponseTemplate() {

packages/flutter_goldens_client/lib/skia_client.dart

@@ -97,18 +97,16 @@ class SkiaGoldClient {
   /// This ensures that the goldctl tool is authorized and ready for testing. It
   /// will only be called once for each instance of
   /// [FlutterSkiaGoldFileComparator].
-  ///
-  /// The [workDirectory] parameter specifies the current directory that golden
-  /// tests are executing in, relative to the library of the given test. It is
-  /// informed by the basedir of the [FlutterSkiaGoldFileComparator].
   Future<void> auth() async {
     if (_clientIsAuthorized())
       return;
 
     if (_serviceAccount.isEmpty) {
       final StringBuffer buf = StringBuffer()
-        ..writeln('Gold service account is unavailable.');
-      throw NonZeroExitCode(1, buf.toString());
+        ..writeln('The Gold service account is unavailable.')
+        ..writeln('Without a service account, Gold can not be authorized.')
+        ..writeln('Please check your user permissions and current comparator.');
+      throw Exception(buf.toString());
     }
 
     final File authorization = workDirectory.childFile('serviceAccount.json');
@@ -129,10 +127,44 @@ class SkiaGoldClient {
 
     if (result.exitCode != 0) {
       final StringBuffer buf = StringBuffer()
-        ..writeln('Skia Gold auth failed.')
+        ..writeln('Skia Gold authorization failed.')
+        ..writeln('This could be caused by incorrect user permissions, if the ')
+        ..writeln('debug information below contains ENCRYPTED, the wrong ')
+        ..writeln('comparator was chosen for the test case.')
+        ..writeln()
+        ..writeln('Debug information for Gold:')
+        ..writeln('stdout: ${result.stdout}')
+        ..writeln('stderr: ${result.stderr}');
+      throw Exception(buf.toString());
+    }
+  }
+
+  /// Prepares the local work space for an unauthorized client to lookup golden
+  /// file expectations using [imgtestCheck].
+  ///
+  /// It will only be called once for each instance of an
+  /// [_UnauthorizedFlutterPreSubmitComparator].
+  Future<void> emptyAuth() async {
+    final List<String> authArguments = <String>[
+      'auth',
+      '--work-dir', workDirectory
+        .childDirectory('temp')
+        .path,
+    ];
+
+    final io.ProcessResult result = await io.Process.run(
+      _goldctl,
+      authArguments,
+    );
+
+    if (result.exitCode != 0) {
+      final StringBuffer buf = StringBuffer()
+        ..writeln('Skia Gold emptyAuth failed.')
+        ..writeln()
+        ..writeln('Debug information for Gold:')
         ..writeln('stdout: ${result.stdout}')
         ..writeln('stderr: ${result.stderr}');
-      throw NonZeroExitCode(1, buf.toString());
+      throw Exception(buf.toString());
     }
   }
 
@@ -162,9 +194,11 @@ class SkiaGoldClient {
 
     if (imgtestInitArguments.contains(null)) {
       final StringBuffer buf = StringBuffer()
-        ..writeln('Null argument for Skia Gold imgtest init:');
+        ..writeln('A null argument was provided for Skia Gold imgtest init.')
+        ..writeln('Please confirm the settings of your golden file test.')
+        ..writeln('Arguments provided:');
       imgtestInitArguments.forEach(buf.writeln);
-      throw NonZeroExitCode(1, buf.toString());
+      throw Exception(buf.toString());
     }
 
     final io.ProcessResult result = await io.Process.run(
@@ -175,9 +209,13 @@ class SkiaGoldClient {
     if (result.exitCode != 0) {
       final StringBuffer buf = StringBuffer()
         ..writeln('Skia Gold imgtest init failed.')
+        ..writeln('An error occured when initializing golden file test with ')
+        ..writeln('goldctl.')
+        ..writeln()
+        ..writeln('Debug information for Gold:')
         ..writeln('stdout: ${result.stdout}')
         ..writeln('stderr: ${result.stderr}');
-      throw NonZeroExitCode(1, buf.toString());
+      throw Exception(buf.toString());
     }
   }
 
@@ -188,8 +226,8 @@ class SkiaGoldClient {
   /// returned from the invocation of this command that indicates a pass or fail
   /// result.
   ///
-  /// The testName and goldenFile parameters reference the current comparison
-  /// being evaluated by the [FlutterSkiaGoldFileComparator].
+  /// The [testName] and [goldenFile] parameters reference the current
+  /// comparison being evaluated by the [FlutterSkiaGoldFileComparator].
   Future<bool> imgtestAdd(String testName, File goldenFile) async {
     assert(testName != null);
     assert(goldenFile != null);
@@ -209,6 +247,9 @@ class SkiaGoldClient {
     );
 
     if (result.exitCode != 0) {
+      // We do not want to throw for non-zero exit codes here, as an intentional
+      // change or new golden file test expect non-zero exit codes. Logging here
+      // is meant to inform when an unexpected result occurs.
       print('goldctl imgtest add stdout: ${result.stdout}');
       print('goldctl imgtest add stderr: ${result.stderr}');
     }
@@ -250,9 +291,11 @@ class SkiaGoldClient {
 
     if (imgtestInitArguments.contains(null)) {
       final StringBuffer buf = StringBuffer()
-        ..writeln('Null argument for Skia Gold tryjobInit:');
+        ..writeln('A null argument was provided for Skia Gold tryjob init.')
+        ..writeln('Please confirm the settings of your golden file test.')
+        ..writeln('Arguments provided:');
       imgtestInitArguments.forEach(buf.writeln);
-      throw NonZeroExitCode(1, buf.toString());
+      throw Exception(buf.toString());
     }
 
     final io.ProcessResult result = await io.Process.run(
@@ -263,9 +306,13 @@ class SkiaGoldClient {
     if (result.exitCode != 0) {
       final StringBuffer buf = StringBuffer()
         ..writeln('Skia Gold tryjobInit failure.')
+        ..writeln('An error occured when initializing golden file tryjob with ')
+        ..writeln('goldctl.')
+        ..writeln()
+        ..writeln('Debug information for Gold:')
         ..writeln('stdout: ${result.stdout}')
         ..writeln('stderr: ${result.stderr}');
-      throw NonZeroExitCode(1, buf.toString());
+      throw Exception(buf.toString());
     }
   }
 
@@ -276,8 +323,8 @@ class SkiaGoldClient {
   /// returned from the invocation of this command that indicates a pass or fail
   /// result for the tryjob.
   ///
-  /// The testName and goldenFile parameters reference the current comparison
-  /// being evaluated by the [FlutterSkiaGoldFileComparator].
+  /// The [testName] and [goldenFile] parameters reference the current
+  /// comparison being evaluated by the [_AuthorizedFlutterPreSubmitComparator].
   Future<bool> tryjobAdd(String testName, File goldenFile) async {
     assert(testName != null);
     assert(goldenFile != null);
@@ -297,12 +344,71 @@ class SkiaGoldClient {
     );
 
     if (result.exitCode != 0) {
-      final StringBuffer buf = StringBuffer()
-        ..writeln('Skia Gold tryjobAdd failure.')
-        ..writeln('stdout: ${result.stdout}')
-        ..writeln('stderr: ${result.stderr}\n');
-      throw NonZeroExitCode(1, buf.toString());
+      final String resultStdout = result.stdout.toString();
+      if (resultStdout.contains('Untriaged') || resultStdout.contains('negative image')) {
+        final List<String> failureLinks = await workDirectory.childFile('failures.json').readAsLines();
+
+        final StringBuffer buf = StringBuffer()
+          ..writeln('The golden file "$testName" ')
+          ..writeln('did not match the expected image.')
+          ..writeln('To view the closest matching image, the actual image generated, ')
+          ..writeln('and the visual difference, visit: ')
+          ..writeln(failureLinks.last)
+          ..writeln('There you can also triage this image (e.g. because this ')
+          ..writeln('is an intentional change).')
+          ..writeln();
+        throw Exception(buf.toString());
+      } else {
+        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();
+        throw Exception(buf.toString());
+      }
     }
+
+    return result.exitCode == 0;
+  }
+
+  /// Executes the `imgtest check` command in the goldctl tool for unauthorized
+  /// clients.
+  ///
+  /// Using the `check` command hashes the current test images and checks that
+  /// hash against Gold's known expectation hashes. A response is returned from
+  /// the invocation of this command that indicates a pass or fail result,
+  /// indicating if Gold has seen this image before.
+  ///
+  /// This will not allow for state change on the Gold dashboard, it is
+  /// essentially a lookup function. If an unauthorized change needs to be made,
+  /// use Gold's ignore feature.
+  ///
+  /// The [testName] and [goldenFile] parameters reference the current
+  /// comparison being evaluated by the
+  /// [_UnauthorizedFlutterPreSubmitComparator].
+  Future<bool> imgtestCheck(String testName, File goldenFile) async {
+    assert(testName != null);
+    assert(goldenFile != null);
+
+    final List<String> imgtestArguments = <String>[
+      'imgtest', 'check',
+      '--work-dir', workDirectory
+        .childDirectory('temp')
+        .path,
+      '--test-name', cleanTestName(testName),
+      '--png-file', goldenFile.path,
+      '--instance', 'flutter',
+    ];
+
+    final io.ProcessResult result = await io.Process.run(
+      _goldctl,
+      imgtestArguments,
+    );
+
     return result.exitCode == 0;
   }
 
@@ -359,6 +465,69 @@ class SkiaGoldClient {
     return imageBytes;
   }
 
+  /// Returns a boolean value for whether or not the given test and current pull
+  /// request are ignored on Flutter Gold.
+  ///
+  /// This is only relevant when used by the [FlutterPreSubmitFileComparator]
+  /// when a golden file test fails. In order to land a change to an existing
+  /// golden file, an ignore must be set up in Flutter Gold. This will serve as
+  /// a flag to permit the change to land, protect against any unwanted changes,
+  /// and ensure that changes that have landed are triaged.
+  Future<bool> testIsIgnoredForPullRequest(String pullRequest, String testName) async {
+    bool ignoreIsActive = false;
+    testName = cleanTestName(testName);
+    String rawResponse;
+    await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
+      final Uri requestForIgnores = Uri.parse(
+        'https://flutter-gold.skia.org/json/ignores'
+      );
+
+      try {
+        final io.HttpClientRequest request = await httpClient.getUrl(requestForIgnores);
+        final io.HttpClientResponse response = await request.close();
+        rawResponse = await utf8.decodeStream(response);
+        final List<dynamic> ignores = json.decode(rawResponse) as List<dynamic>;
+        for(final dynamic ignore in ignores) {
+          final List<String> ignoredQueries = (ignore['query'] as String).split('&');
+          final String ignoredPullRequest = (ignore['note'] as String).split('/').last;
+          final DateTime expiration = DateTime.parse(ignore['expires'] as String);
+          // The currently failing test is in the process of modification.
+          if (ignoredQueries.contains('name=$testName')) {
+            if (expiration.isAfter(DateTime.now())) {
+              ignoreIsActive = true;
+            } else {
+              // If any ignore is expired for the given test, throw with
+              // guidance.
+              final StringBuffer buf = StringBuffer()
+                ..writeln('This test has an expired ignore in place, and the')
+                ..writeln('change has not been triaged.')
+                ..writeln('The associated pull request is:')
+                ..writeln('https://github.com/flutter/flutter/pull/$ignoredPullRequest');
+              throw Exception(buf.toString());
+            }
+          }
+        }
+      } on FormatException catch(_) {
+        if (rawResponse.contains('stream timeout')) {
+          final StringBuffer buf = StringBuffer()
+            ..writeln('Stream timeout on /ignores api.')
+            ..writeln('This may be caused by a failure to triage a change.')
+            ..writeln('Check https://flutter-gold.skia.org/ignores, or')
+            ..writeln('https://flutter-gold.skia.org/?query=source_type%3Dflutter')
+            ..writeln('for untriaged golden files.');
+          throw Exception(buf.toString());
+        } else {
+          print('Formatting error detected requesting /ignores from Flutter Gold.'
+            '\nrawResponse: $rawResponse');
+          rethrow;
+        }
+      }
+    },
+      SkiaGoldHttpOverrides(),
+    );
+    return ignoreIsActive;
+  }
+
   /// The [_expectations] retrieved from Flutter Gold do not include the
   /// parameters of the given test. This function queries the Flutter Gold
   /// details api to determine if the given expectation for a test matches the
@@ -383,8 +552,8 @@ class SkiaGoldClient {
       } on FormatException catch(_) {
         if (rawResponse.contains('stream timeout')) {
           final StringBuffer buf = StringBuffer()
-            ..writeln('Stream timeout on /details api.');
-          throw NonZeroExitCode(1, buf.toString());
+            ..writeln('Stream timeout on Gold\'s /details api.');
+          throw Exception(buf.toString());
         } else {
           print('Formatting error detected requesting /ignores from Flutter Gold.'
             '\nrawResponse: $rawResponse');
@@ -402,7 +571,7 @@ class SkiaGoldClient {
     if (!_flutterRoot.existsSync()) {
       final StringBuffer buf = StringBuffer()
         ..writeln('Flutter root could not be found: $_flutterRoot');
-      throw NonZeroExitCode(1, buf.toString());
+      throw Exception(buf.toString());
     } else {
       final io.ProcessResult revParse = await process.run(
         <String>['git', 'rev-parse', 'HEAD'],
@@ -490,24 +659,5 @@ class SkiaGoldDigest {
          || paramSet['Browser'] == platform.environment[_kTestBrowserKey])
       && testName == name
       && status == 'positive';
-    }
   }
-
-/// Exception that signals a process' exit with a non-zero exit code.
-class NonZeroExitCode implements Exception {
-  /// Create an exception that represents a non-zero exit code.
-  ///
-  /// The first argument must be non-zero.
-  const NonZeroExitCode(this.exitCode, this.stderr) : assert(exitCode != 0);
-
-  /// The code that the process will signal to the operating system.
-  ///
-  /// By definition, this is not zero.
-  final int exitCode;
-
-  /// The message to show on standard error.
-  final String stderr;
-
-  @override
-  String toString() => 'Exit code $exitCode: $stderr';
 }