Close tree if unapproved golden images get in (flutter#93516)

Author: Piinks

packages/flutter_goldens/test/flutter_goldens_test.dart

@@ -153,6 +153,45 @@ void main() {
       );
     });
 
+    test('throws for error state from imgtestAdd', () {
+      final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png')
+        ..createSync(recursive: true);
+      platform = FakePlatform(
+          environment: <String, String>{
+            'FLUTTER_ROOT': _kFlutterRoot,
+            'GOLDCTL' : 'goldctl',
+          },
+          operatingSystem: 'macos'
+      );
+
+      skiaClient = SkiaGoldClient(
+        workDirectory,
+        fs: fs,
+        process: process,
+        platform: platform,
+        httpClient: fakeHttpClient,
+      );
+
+      process.fallbackProcessResult = ProcessResult(123, 1, 'fail', 'fail');
+      const RunInvocation goldctlInvocation = RunInvocation(
+        <String>[
+          'goldctl',
+          'imgtest', 'add',
+          '--work-dir', '/workDirectory/temp',
+          '--test-name', 'golden_file_test',
+          '--png-file', '/workDirectory/temp/golden_file_test.png',
+          '--passfail',
+        ],
+        null,
+      );
+      process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'fail', 'fail');
+
+      expect(
+        skiaClient.imgtestAdd('golden_file_test', goldenFile),
+        throwsException,
+      );
+    });
+
     test('correctly inits tryjob for luci', () async {
       platform = FakePlatform(
         environment: <String, String>{

packages/flutter_goldens_client/lib/skia_client.dart

@@ -177,17 +177,28 @@ class SkiaGoldClient {
         .path,
       '--test-name', cleanTestName(testName),
       '--png-file', goldenFile.path,
+      '--passfail',
     ];
 
     final io.ProcessResult result = await process.run(imgtestCommand);
 
     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 help debugging in CI when an unexpected result occurs.
-      // See also: https://github.com/flutter/flutter/issues/91285
-      print('goldctl imgtest add stdout: ${result.stdout}'); // ignore: avoid_print
-      print('goldctl imgtest add stderr: ${result.stderr}'); // ignore: avoid_print
+      // If an unapproved image has made it to post-submit, throw to close the
+      // tree.
+      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}');
+      throw Exception(buf.toString());
     }
 
     return true;