Remove Cirrus support for Gold (#67468)

.cirrus.yml

@@ -89,7 +89,6 @@ task:
         # We use 3 CPUs because that's the minimum required to get framework_tests-widgets-linux
         # running fast enough that it is not the long pole, as of October 2019.
         CPU: 3
-        GOLD_SERVICE_ACCOUNT: ENCRYPTED[3afeea5ac7201151c3d0dc9648862f0462b5e4f55dc600ca8b692319622f7c3eda3d577b1b16cc2ef0311b7314c1c095]
       script:
         - dart --enable-asserts ./dev/bots/test.dart
 
@@ -100,7 +99,6 @@ task:
         # framework_tests-libraries-linux shard running fast enough that it is not the long pole, as
         # of October 2019.
         CPU: 3
-        GOLD_SERVICE_ACCOUNT: ENCRYPTED[3afeea5ac7201151c3d0dc9648862f0462b5e4f55dc600ca8b692319622f7c3eda3d577b1b16cc2ef0311b7314c1c095]
       script:
         - dart --enable-asserts ./dev/bots/test.dart
 
@@ -150,7 +148,6 @@ task:
         CPU: 2
         MEMORY: 8G
         CHROME_NO_SANDBOX: true
-        GOLD_SERVICE_ACCOUNT: ENCRYPTED[3afeea5ac7201151c3d0dc9648862f0462b5e4f55dc600ca8b692319622f7c3eda3d577b1b16cc2ef0311b7314c1c095]
       script:
         - dart --enable-asserts ./dev/bots/test.dart
 

dev/bots/download_goldctl.ps1

-# 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.
-
-$url= "https://storage.googleapis.com/chrome-infra/depot_tools.zip"
-$zipPath = "C:\Windows\Temp\depot_tools.zip"
-$path = "C:\Windows\Temp\depot_tools"
-$gclient = "C:\Windows\Temp\depot_tools\gclient.bat"
-$cipd = "C:\Windows\Temp\depot_tools\cipd.bat"
-$ensureFile = "C:\Windows\Temp\depot_tools\ensure.txt"
-$text = "# Ensure File`n`$ServiceURL https://chrome-infra-packages.appspot.com`n`n# Skia Gold Client goldctl`nskia/tools/goldctl/`${platform} git_revision:b57f561ad4ad624bd399b8b7b500aa1955276d41"
-
-(New-Object System.Net.WebClient).DownloadFile($url, $zipPath)
-Expand-Archive -LiteralPath $zipPath -DestinationPath $path
-cd $path
-cmd.exe /C "$gclient"
-$text | Out-File -filePath $ensureFile -encoding ascii
-cmd.exe /C "$cipd ensure -ensure-file $ensureFile -root $path"

dev/bots/download_goldctl.sh

-#!/usr/bin/env bash
-# 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.
-
-git clone --depth 1 https://chromium.googlesource.com/chromium/tools/depot_tools.git ./depot_tools
-cd depot_tools
-echo -e '# Ensure File\n$ServiceURL https://chrome-infra-packages.appspot.com\n\n# Skia Gold Client goldctl\nskia/tools/goldctl/${platform} git_revision:b57f561ad4ad624bd399b8b7b500aa1955276d41' > ensure.txt
-./cipd ensure -ensure-file ./ensure.txt -root .

dev/ci/docker_linux/Dockerfile

@@ -123,8 +123,3 @@ COPY ci/docker_linux/Gemfile.lock /Gemfile.lock
 
 RUN bundle config set system 'true' && \
   bundle install --system
-
-# Install goldctl, for Golden testing
-COPY bots/download_goldctl.sh /download_goldctl.sh
-ENV GOLDCTL '/depot_tools/goldctl'
-RUN /download_goldctl.sh

packages/flutter/test/widgets/basic_test.dart

@@ -443,7 +443,7 @@ void main() {
     // golden file can be approved at any time.
     await tester.pumpWidget(RepaintBoundary(
       child: Container(
-        color: const Color(0xFF42A5F5),
+        color: const Color(0xABCDABCD),
       ),
     ));
 

packages/flutter_goldens/lib/flutter_goldens.dart

@@ -35,7 +35,7 @@ Future<void> testExecutable(FutureOr<void> testMain()) async {
     goldenFileComparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(platform);
   } else if (FlutterSkippingFileComparator.isAvailableForEnvironment(platform)) {
     goldenFileComparator = FlutterSkippingFileComparator.fromDefaultComparator(
-      'Golden file testing is not executed on some Cirrus & Luci environments.'
+      'Golden file testing is not executed on Cirrus, or LUCI environments outside of flutter/flutter.'
     );
   } else {
     goldenFileComparator = await FlutterLocalFileComparator.fromDefaultComparator(platform);
@@ -84,10 +84,10 @@ Future<void> testExecutable(FutureOr<void> testMain()) async {
 ///     output the new image for verification.
 ///
 ///  The [FlutterSkippingFileComparator] is utilized to skip tests outside
-///  of the appropriate environments described above. Currently, some Cirrus
-///  test shards and Luci environments do not execute golden file testing, and
-///  as such do not require a comparator. This comparator is also used when an
-///  internet connection is unavailable.
+///  of the appropriate environments described above. Currently, some Luci
+///  environments do not execute golden file testing, and as such do not require
+///  a comparator. This comparator is also used when an internet connection is
+///  unavailable.
 abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
   /// Creates a [FlutterGoldenFileComparator] that will resolve golden file
   /// URIs relative to the specified [basedir], and retrieve golden baselines
@@ -241,12 +241,7 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
     );
     baseDirectory.createSync(recursive: true);
 
-    goldens ??= SkiaGoldClient(
-      baseDirectory,
-      ci: platform.environment.containsKey('CIRRUS_CI')
-        ? ContinuousIntegrationEnvironment.cirrus
-        : ContinuousIntegrationEnvironment.luci,
-    );
+    goldens ??= SkiaGoldClient(baseDirectory);
     await goldens.auth();
     await goldens.imgtestInit();
     return FlutterPostSubmitFileComparator(baseDirectory.uri, goldens);
@@ -264,30 +259,18 @@ class FlutterPostSubmitFileComparator extends FlutterGoldenFileComparator {
   /// Decides based on the current environment if goldens tests should be
   /// executed through Skia Gold.
   static bool isAvailableForEnvironment(Platform platform) {
-    final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
-    final String cirrusBranch = platform.environment['CIRRUS_BRANCH'] ?? '';
-    final bool cirrusPostSubmit = platform.environment.containsKey('CIRRUS_CI')
-      && cirrusPR.isEmpty
-      && cirrusBranch == 'master'
-      && platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
-
     final bool luciPostSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
       && platform.environment.containsKey('GOLDCTL')
       // Luci tryjob environments contain this value to inform the [FlutterPreSubmitComparator].
       && !platform.environment.containsKey('GOLD_TRYJOB');
 
-    return cirrusPostSubmit || luciPostSubmit;
+    return luciPostSubmit;
   }
 }
 
 /// A [FlutterGoldenFileComparator] for testing golden images before changes are
-/// merged into the master branch.
-///
-/// When authorized (on luci and most cirrus testing conditions), the comparator
-/// executes tryjobs using the [SkiaGoldClient].
-///
-/// When unauthorized, this comparator utilizes the [SkiaGoldClient] to request
-/// baseline images for the given device under test for manual comparison.
+/// merged into the master branch. The comparator executes tryjobs using the
+/// [SkiaGoldClient].
 ///
 /// See also:
 ///
@@ -339,80 +322,17 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
     if (!baseDirectory.existsSync())
       baseDirectory.createSync(recursive: true);
 
-    goldens ??= SkiaGoldClient(
-      baseDirectory,
-      ci: platform.environment.containsKey('CIRRUS_CI')
-        ? ContinuousIntegrationEnvironment.cirrus
-        : ContinuousIntegrationEnvironment.luci,
-    );
+    goldens ??= SkiaGoldClient(baseDirectory);
 
-    bool onCirrusWithPermission = false;
-    if (platform.environment.containsKey('GOLD_SERVICE_ACCOUNT')) {
-      // Some contributors may not have permission on Cirrus to decrypt the
-      // service account.
-      onCirrusWithPermission =
-        !platform.environment['GOLD_SERVICE_ACCOUNT']!.startsWith('ENCRYPTED');
-    }
-    final bool onLuci = platform.environment.containsKey('SWARMING_TASK_ID');
-    if (onCirrusWithPermission || onLuci) {
     await goldens.auth();
     await goldens.tryjobInit();
-      return _AuthorizedFlutterPreSubmitComparator(
+      return FlutterPreSubmitFileComparator(
         baseDirectory.uri,
         goldens,
         platform: platform,
       );
     }
 
-    goldens.emptyAuth();
-    return _UnauthorizedFlutterPreSubmitComparator(
-      baseDirectory.uri,
-      goldens,
-      platform: platform,
-    );
-  }
-
-  @override
-  Future<bool> compare(Uint8List imageBytes, Uri golden) async {
-    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 if goldens tests should be
-  /// executed as pre-submit tests with Skia Gold.
-  static bool isAvailableForEnvironment(Platform platform) {
-    final String cirrusPR = platform.environment['CIRRUS_PR'] ?? '';
-    final bool cirrusPreSubmit = platform.environment.containsKey('CIRRUS_CI')
-      && cirrusPR.isNotEmpty
-      && platform.environment.containsKey('GOLD_SERVICE_ACCOUNT');
-
-    final bool luciPreSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
-      && platform.environment.containsKey('GOLDCTL')
-      && platform.environment.containsKey('GOLD_TRYJOB');
-    return cirrusPreSubmit || luciPreSubmit;
-  }
-}
-
-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);
@@ -425,74 +345,22 @@ class _AuthorizedFlutterPreSubmitComparator extends FlutterPreSubmitFileComparat
     // in pre-submit checks by the flutter-gold status check.
     return true;
   }
-}
-
-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 hash, so we need to check manually.
-    final String testName = skiaClient.cleanTestName(golden.path);
-    final String? testExpectation = await skiaClient.getExpectationForTest(testName);
-    if (testExpectation == 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.
-    String? pullRequest;
-    switch(skiaClient.ci) {
-      case ContinuousIntegrationEnvironment.cirrus:
-        pullRequest = platform.environment['CIRRUS_PR']!;
-        break;
-      case ContinuousIntegrationEnvironment.luci:
-        final List<String> refs = platform.environment['GOLD_TRYJOB']!.split('/');
-        pullRequest = refs[refs.length - 2];
-        break;
-      case ContinuousIntegrationEnvironment.none:
-        pullRequest = '';
-        break;
-    }
-
-    final bool ignoreResult = await skiaClient.testIsIgnoredForPullRequest(
-      pullRequest,
-      golden.path,
-    );
-    // If true, this is an intended change and is being handled on the Flutter
-    // Gold dashboard: https://flutter-gold.skia.org/ignores
-    return ignoreResult;
+  /// Decides based on the current environment if goldens tests should be
+  /// executed as pre-submit tests with Skia Gold.
+  static bool isAvailableForEnvironment(Platform platform) {
+    final bool luciPreSubmit = platform.environment.containsKey('SWARMING_TASK_ID')
+      && platform.environment.containsKey('GOLDCTL')
+      && platform.environment.containsKey('GOLD_TRYJOB');
+    return luciPreSubmit;
   }
 }
 
 /// A [FlutterGoldenFileComparator] for testing conditions that do not execute
 /// golden file tests.
 ///
-/// Currently, this comparator is used in some Cirrus test shards and Luci
-/// environments.
+/// Currently, this comparator is used on Cirrus, or in Luci environments when executing tests
+/// outside of the flutter/flutter repository.
 ///
 /// See also:
 ///
@@ -527,7 +395,7 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
     defaultComparator ??= goldenFileComparator as LocalFileComparator;
     const FileSystem fs = LocalFileSystem();
     final Uri basedir = defaultComparator.basedir;
-    final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir), ci: ContinuousIntegrationEnvironment.none);
+    final SkiaGoldClient skiaClient = SkiaGoldClient(fs.directory(basedir));
     return FlutterSkippingFileComparator(basedir, skiaClient, reason);
   }
 
@@ -545,10 +413,11 @@ class FlutterSkippingFileComparator extends FlutterGoldenFileComparator {
   /// Decides, based on the current environment, if this comparator should be
   /// used.
   ///
-  /// If we are in a CI environment, luci or Cirrus, but are not using the other
+  /// If we are in a CI environment, LUCI or Cirrus, but are not using the other
   /// comparators, we skip.
   static bool isAvailableForEnvironment(Platform platform) {
     return platform.environment.containsKey('SWARMING_TASK_ID')
+      // Some builds are still being run on Cirrus, we should skip these.
       || platform.environment.containsKey('CIRRUS_CI');
   }
 }
@@ -619,7 +488,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
       baseDirectory.createSync(recursive: true);
     }
 
-    goldens ??= SkiaGoldClient(baseDirectory, ci: ContinuousIntegrationEnvironment.none);
+    goldens ??= SkiaGoldClient(baseDirectory);
     try {
       // Check if we can reach Gold.
       await goldens.getExpectationForTest('');

packages/flutter_goldens/test/flutter_goldens_test.dart

@@ -3,8 +3,8 @@
 // found in the LICENSE file.
 
 // @dart = 2.8
+
 import 'dart:async';
-import 'dart:convert';
 import 'dart:core';
 import 'dart:io';
 import 'dart:typed_data';
@@ -76,7 +76,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.luci,
       );
     });
 
@@ -121,7 +120,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.cirrus,
       );
 
       when(process.run(any))
@@ -150,7 +148,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.luci
       );
 
       when(process.run(
@@ -198,7 +195,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.luci,
       );
 
       final List<String> ciArguments = skiaClient.getCIArguments();
@@ -215,73 +211,8 @@ void main() {
       );
     });
 
-    test('correctly inits tryjob for cirrus', () async {
-      platform = FakePlatform(
-        environment: <String, String>{
-          'FLUTTER_ROOT': _kFlutterRoot,
-          'GOLDCTL' : 'goldctl',
-          'CIRRUS_CI' : 'true',
-          'CIRRUS_TASK_ID' : '8885996262141582672',
-          'CIRRUS_PR' : '49815',
-        },
-        operatingSystem: 'macos'
-      );
-
-      skiaClient = SkiaGoldClient(
-        workDirectory,
-        fs: fs,
-        process: process,
-        platform: platform,
-        httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.cirrus,
-      );
-
-      final List<String> ciArguments = skiaClient.getCIArguments();
-
-      expect(
-        ciArguments,
-        equals(
-          <String>[
-            '--changelist', '49815',
-            '--cis', 'cirrus',
-            '--jobid', '8885996262141582672',
-          ],
-        ),
-      );
-    });
-
     test('Creates traceID correctly', () {
       String traceID;
-
-      // On Cirrus
-      platform = FakePlatform(
-        environment: <String, String>{
-          'FLUTTER_ROOT': _kFlutterRoot,
-          'GOLDCTL' : 'goldctl',
-          'CIRRUS_CI' : 'true',
-          'CIRRUS_TASK_ID' : '8885996262141582672',
-          'CIRRUS_PR' : '49815',
-        },
-        operatingSystem: 'macos'
-      );
-
-      skiaClient = SkiaGoldClient(
-        workDirectory,
-        fs: fs,
-        process: process,
-        platform: platform,
-        httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.cirrus,
-      );
-
-      traceID = skiaClient.getTraceID('flutter.golden.1');
-
-      expect(
-        traceID,
-        equals(',CI=cirrus,Platform=macos,name=flutter.golden.1,source_type=flutter,'),
-      );
-
-      // On Luci
       platform = FakePlatform(
         environment: <String, String>{
           'FLUTTER_ROOT': _kFlutterRoot,
@@ -299,7 +230,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.luci,
       );
 
       traceID = skiaClient.getTraceID('flutter.golden.1');
@@ -328,7 +258,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.luci,
       );
 
       traceID = skiaClient.getTraceID('flutter.golden.1');
@@ -352,7 +281,6 @@ void main() {
         process: process,
         platform: platform,
         httpClient: mockHttpClient,
-        ci: ContinuousIntegrationEnvironment.luci,
       );
 
       traceID = skiaClient.getTraceID('flutter.golden.1');
@@ -364,13 +292,9 @@ void main() {
     });
 
     group('Request Handling', () {
-      String testName;
-      String pullRequestNumber;
       String expectation;
 
       setUp(() {
-        testName = 'flutter.golden_test.1.png';
-        pullRequestNumber = '1234';
         expectation = '55109a4bed52acc780530f7a9aeff6c0';
       });
 
@@ -391,120 +315,6 @@ void main() {
 
         expect(masterBytes, equals(_kTestPngBytes));
       });
-
-      group('ignores', () {
-        Uri url;
-        MockHttpClientRequest mockHttpRequest;
-        MockHttpClientResponse mockHttpResponse;
-
-        setUp(() {
-          url = Uri.parse('https://flutter-gold.skia.org/json/v1/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,
-          );
-        });
-      });
     });
   });
 
@@ -559,7 +369,7 @@ void main() {
       });
 
       group('correctly determines testing environment', () {
-        test('returns true for Luci', () {
+        test('returns true for configured Luci', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
@@ -574,47 +384,11 @@ void main() {
           );
         });
 
-        test('returns true for Cirrus', () {
+        test('returns false - GOLDCTL not present', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '',
-              'CIRRUS_BRANCH': 'master',
-              'GOLD_SERVICE_ACCOUNT': 'service account...',
-            },
-            operatingSystem: 'macos'
-          );
-          expect(
-            FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
-            isTrue,
-          );
-        });
-
-        test('returns false - PR active', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
-              'CIRRUS_BRANCH': 'master',
-              'GOLD_SERVICE_ACCOUNT': 'service account...',
-            },
-            operatingSystem: 'macos'
-          );
-          expect(
-            FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
-            isFalse,
-          );
-        });
-
-        test('returns false - no service account', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '',
-              'CIRRUS_BRANCH': 'master',
+              'SWARMING_TASK_ID' : '12345678990',
             },
             operatingSystem: 'macos'
           );
@@ -624,12 +398,13 @@ void main() {
           );
         });
 
-        test('returns false - not on cirrus', () {
+        test('returns false - GOLD_TRYJOB active', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-              'SWARMING_ID' : '1234567890',
-              'GOLD_SERVICE_ACCOUNT': 'service account...'
+              'SWARMING_TASK_ID' : '12345678990',
+              'GOLDCTL' : 'goldctl',
+              'GOLD_TRYJOB' : 'git/ref/12345/head'
             },
             operatingSystem: 'macos'
           );
@@ -639,13 +414,13 @@ void main() {
           );
         });
 
-        test('returns false - not on master', () {
+        test('returns false - on Cirrus', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
               'CIRRUS_CI': 'true',
               'CIRRUS_PR': '',
-              'CIRRUS_BRANCH': 'hotfix',
+              'CIRRUS_BRANCH': 'master',
               'GOLD_SERVICE_ACCOUNT': 'service account...'
             },
             operatingSystem: 'macos'
@@ -659,26 +434,7 @@ void main() {
     });
 
     group('Pre-Submit', () {
-      FlutterGoldenFileComparator comparator;
-      final MockSkiaGoldClient mockSkiaClient = MockSkiaGoldClient();
-
       group('correctly determines testing environment', () {
-        test('returns true for Cirrus', () {
-          platform = FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...',
-            },
-            operatingSystem: 'macos'
-          );
-          expect(
-            FlutterPreSubmitFileComparator.isAvailableForEnvironment(platform),
-            isTrue,
-          );
-        });
-
         test('returns true for Luci', () {
           platform = FakePlatform(
             environment: <String, String>{
@@ -695,13 +451,10 @@ void main() {
           );
         });
 
-        test('returns false - no PR', () {
+        test('returns false - not on Luci', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...',
             },
             operatingSystem: 'macos'
           );
@@ -711,12 +464,12 @@ void main() {
           );
         });
 
-        test('returns false - no service account', () {
+        test('returns false - GOLDCTL missing', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI': 'true',
-              'CIRRUS_PR': '1234',
+              'SWARMING_TASK_ID' : '12345678990',
+              'GOLD_TRYJOB' : 'git/ref/12345/head'
             },
             operatingSystem: 'macos'
           );
@@ -726,10 +479,12 @@ void main() {
           );
         });
 
-        test('returns false - not on Cirrus or Luci', () {
+        test('returns false - GOLD_TRYJOB missing', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
+              'SWARMING_TASK_ID' : '12345678990',
+              'GOLDCTL' : 'goldctl',
             },
             operatingSystem: 'macos'
           );
@@ -738,127 +493,47 @@ void main() {
             isFalse,
           );
         });
-      });
 
-      group('_Authorized', () {
-        setUp(() async {
-          final Directory basedir = fs.directory('flutter/test/library/')
-            ..createSync(recursive: true);
-          comparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(
-            FakePlatform(
+        test('returns false - on Cirrus', () {
+          platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-                'CIRRUS_CI' : 'true',
-                'CIRRUS_PR' : '1234',
-                'GOLD_SERVICE_ACCOUNT' : 'service account...',
-                'CIRRUS_USER_PERMISSION' : 'admin',
+              'CIRRUS_CI': 'true',
+              'CIRRUS_PR': '',
+              'CIRRUS_BRANCH': 'master',
+              'GOLD_SERVICE_ACCOUNT': 'service account...'
             },
             operatingSystem: 'macos'
-            ),
-            goldens: mockSkiaClient,
-            testBasedir: basedir,
           );
-        });
-
-        test('fromDefaultComparator chooses correct comparator', () async {
           expect(
-            comparator.runtimeType.toString(),
-            '_AuthorizedFlutterPreSubmitComparator',
+            FlutterPostSubmitFileComparator.isAvailableForEnvironment(platform),
+            isFalse,
           );
         });
       });
+    });
 
-      group('_UnAuthorized', () {
-        setUp(() async {
-          final Directory basedir = fs.directory('flutter/test/library/')
-            ..createSync(recursive: true);
-          comparator = await FlutterPreSubmitFileComparator.fromDefaultComparator(
-            FakePlatform(
+    group('Skipping', () {
+      group('correctly determines testing environment', () {
+        test('returns true on Cirrus builds', () {
+          platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-                'CIRRUS_CI' : 'true',
-                'CIRRUS_PR' : '1234',
-                'GOLD_SERVICE_ACCOUNT' : 'ENCRYPTED[...]',
-                'CIRRUS_USER_PERMISSION' : 'none',
+              'CIRRUS_CI' : 'yep',
             },
             operatingSystem: 'macos'
-            ),
-            goldens: mockSkiaClient,
-            testBasedir: basedir,
-          );
-          when(mockSkiaClient.cleanTestName('library.flutter.golden_test.1.png'))
-            .thenReturn('flutter.golden_test.1');
-        });
-
-        test('fromDefaultComparator chooses correct comparator', () async {
-          expect(
-            comparator.runtimeType.toString(),
-            '_UnauthorizedFlutterPreSubmitComparator',
           );
-        });
-
-        test('comparison passes test that is ignored for this PR', () async {
-          when(mockSkiaClient.imgtestCheck(any, any))
-            .thenAnswer((_) => Future<bool>.value(false));
-          when(mockSkiaClient.getExpectationForTest('flutter.golden_test.1'))
-            .thenAnswer((_) => Future<String>.value('123456789abc'));
-          when(mockSkiaClient.ci).thenReturn(ContinuousIntegrationEnvironment.cirrus);
-          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.imgtestCheck(any, any))
-            .thenAnswer((_) => Future<bool>.value(false));
-          when(mockSkiaClient.getExpectationForTest('flutter.golden_test.1'))
-            .thenAnswer((_) => Future<String>.value('123456789abc'));
-          when(mockSkiaClient.ci).thenReturn(ContinuousIntegrationEnvironment.cirrus);
-          when(mockSkiaClient.testIsIgnoredForPullRequest(
-            '1234',
-            'library.flutter.golden_test.1.png',
-          ))
-            .thenAnswer((_) => Future<bool>.value(false));
-          expect(
-            await comparator.compare(
-              Uint8List.fromList(_kFailPngBytes),
-              Uri.parse('flutter.golden_test.1.png'),
-            ),
-            isFalse,
-          );
-        });
-
-        testWithOutput('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(
-            await comparator.compare(
-              Uint8List.fromList(_kFailPngBytes),
-              Uri.parse('flutter.new_golden_test.1.png'),
-            ),
+            FlutterSkippingFileComparator.isAvailableForEnvironment(platform),
             isTrue,
           );
-        }, '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');
-      });
         });
 
-    group('Skipping', () {
-      group('correctly determines testing environment', () {
-        test('returns true on Cirrus shards that don\'t run golden tests', () {
+        test('returns true on irrelevant LUCI builds', () {
           platform = FakePlatform(
             environment: <String, String>{
               'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI' : 'yep',
+              'SWARMING_TASK_ID' : '1234567890',
             },
             operatingSystem: 'macos'
           );

packages/flutter_goldens/test/json_templates.dart

@@ -16,37 +16,6 @@ String authTemplate({
   ''';
 }
 
-/// Json response template for Skia Gold ignore request:
-/// https://flutter-gold.skia.org/json/v1/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

@@ -17,16 +17,8 @@ import 'package:process/process.dart';
 
 const String _kFlutterRootKey = 'FLUTTER_ROOT';
 const String _kGoldctlKey = 'GOLDCTL';
-const String _kServiceAccountKey = 'GOLD_SERVICE_ACCOUNT';
 const String _kTestBrowserKey = 'FLUTTER_TEST_BROWSER';
 
-/// Enum representing the supported CI environments used by flutter/flutter.
-enum ContinuousIntegrationEnvironment {
-  luci,
-  cirrus,
-  none,
-}
-
 /// A client for uploading image tests and making baseline requests to the
 /// Flutter Gold Dashboard.
 class SkiaGoldClient {
@@ -35,7 +27,6 @@ class SkiaGoldClient {
     this.fs = const LocalFileSystem(),
     this.process = const LocalProcessManager(),
     this.platform = const LocalPlatform(),
-    required this.ci,
     io.HttpClient? httpClient,
   }) : httpClient = httpClient ?? io.HttpClient();
 
@@ -58,9 +49,6 @@ class SkiaGoldClient {
   /// sub-processes.
   final ProcessManager process;
 
-  /// What testing environment we may be in, like Cirrus or Luci.
-  final ContinuousIntegrationEnvironment ci;
-
   /// A client for making Http requests to the Flutter Gold dashboard.
   final io.HttpClient httpClient;
 
@@ -82,12 +70,6 @@ class SkiaGoldClient {
   /// Uses the [platform] environment in this implementation.
   String get _goldctl => platform.environment[_kGoldctlKey]!;
 
-  /// The path to the local [Directory] where the service account key is
-  /// hosted.
-  ///
-  /// Uses the [platform] environment in this implementation.
-  String get _serviceAccount => platform.environment[_kServiceAccountKey]!;
-
   /// Prepares the local work space for golden file testing and calls the
   /// goldctl `auth` command.
   ///
@@ -101,49 +83,13 @@ class SkiaGoldClient {
   Future<void> auth() async {
     if (await clientIsAuthorized())
       return;
-
-    List<String> authArguments;
-    String failureContext;
-
-    switch (ci) {
-      case ContinuousIntegrationEnvironment.luci:
-        authArguments = <String>[
+    final List<String> authArguments = <String>[
       'auth',
       '--work-dir', workDirectory
         .childDirectory('temp')
         .path,
       '--luci',
     ];
-        failureContext =
-        'Luci environments authenticate using the file provided '
-          'by LUCI_CONTEXT. There may be an error with this file or Gold '
-          'authentication.';
-        break;
-      case ContinuousIntegrationEnvironment.cirrus:
-        if (_serviceAccount.isEmpty) {
-          final StringBuffer buf = StringBuffer()
-            ..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');
-        await authorization.writeAsString(_serviceAccount);
-        authArguments = <String>[
-          'auth',
-          '--service-account', authorization.path,
-          '--work-dir', workDirectory
-            .childDirectory('temp')
-            .path,
-        ];
-        failureContext = 'This could be caused by incorrect user permissions on '
-          'Cirrus, if the debug information below contains ENCRYPTED, the wrong '
-          'comparator was chosen for the test case.';
-        break;
-      case ContinuousIntegrationEnvironment.none:
-        return;
-    }
 
     final io.ProcessResult result = await io.Process.run(
       _goldctl,
@@ -153,40 +99,9 @@ class SkiaGoldClient {
     if (result.exitCode != 0) {
       final StringBuffer buf = StringBuffer()
         ..writeln('Skia Gold authorization failed.')
-        ..writeln(failureContext)
-        ..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 {
-    // We only use emptyAuth when the service account cannot be decrypted on
-    // Cirrus.
-    assert(ci == ContinuousIntegrationEnvironment.cirrus);
-
-    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('Luci environments authenticate using the file provided '
+          'by LUCI_CONTEXT. There may be an error with this file or Gold '
+          'authentication.')
         ..writeln('Debug information for Gold:')
         ..writeln('stdout: ${result.stdout}')
         ..writeln('stderr: ${result.stderr}');
@@ -306,10 +221,9 @@ class SkiaGoldClient {
       '--passfail',
       '--crs', 'github',
       '--patchset_id', commitHash,
+      ...getCIArguments(),
     ];
 
-    imgtestInitArguments.addAll(getCIArguments());
-
     if (imgtestInitArguments.contains(null)) {
       final StringBuffer buf = StringBuffer()
         ..writeln('A null argument was provided for Skia Gold tryjob init.')
@@ -558,10 +472,12 @@ class SkiaGoldClient {
   String _getKeysJSON() {
     final Map<String, dynamic> keys = <String, dynamic>{
       'Platform' : platform.operatingSystem,
-      'CI' : ci.toString().split('.').last,
+      'CI' : 'luci',
     };
-    if (platform.environment[_kTestBrowserKey] != null)
+    if (platform.environment[_kTestBrowserKey] != null) {
       keys['Browser'] = platform.environment[_kTestBrowserKey];
+      keys['Platform'] = keys['Platform'] + '-browser';
+    }
     return json.encode(keys);
   }
 
@@ -590,29 +506,13 @@ class SkiaGoldClient {
   /// Returns a list of arguments for initializing a tryjob based on the testing
   /// environment.
   List<String> getCIArguments() {
-    String pullRequest;
-    String jobId;
-    String cis;
-
-    switch (ci) {
-      case ContinuousIntegrationEnvironment.luci:
-        jobId = platform.environment['LOGDOG_STREAM_PREFIX']!.split('/').last;
+    final String jobId = platform.environment['LOGDOG_STREAM_PREFIX']!.split('/').last;
     final List<String> refs = platform.environment['GOLD_TRYJOB']!.split('/');
-        pullRequest = refs[refs.length - 2];
-        cis = 'buildbucket';
-        break;
-      case ContinuousIntegrationEnvironment.cirrus:
-        pullRequest = platform.environment['CIRRUS_PR']!;
-        jobId = platform.environment['CIRRUS_TASK_ID']!;
-        cis = 'cirrus';
-        break;
-      case ContinuousIntegrationEnvironment.none:
-        return <String>[];
-    }
+    final String pullRequest = refs[refs.length - 2];
 
     return <String>[
       '--changelist', pullRequest,
-      '--cis', cis,
+      '--cis', 'buildbucket',
       '--jobid', jobId,
     ];
   }
@@ -627,14 +527,12 @@ class SkiaGoldClient {
   /// Example TraceID for Flutter Gold:
   ///   ',CI=cirrus,Platform=linux,name=cupertino.activityIndicator.inprogress.1.0,source_type=flutter,'
   String getTraceID(String testName) {
-    // If we are not in a CI environment, fallback on luci.
     return '${platform.environment[_kTestBrowserKey] == null ? ',' : ',Browser=${platform.environment[_kTestBrowserKey]},'}'
-      'CI=${ci == ContinuousIntegrationEnvironment.none ? 'luci' : ci.toString().split('.').last},'
+      'CI=luci,'
       'Platform=${platform.operatingSystem},'
       'name=$testName,'
       'source_type=flutter,';
   }
-
 }
 
 /// Used to make HttpRequests during testing.