flutter · Piinks · Dec 4, 2019 · Nov 8, 2019 · Nov 15, 2019 · Nov 15, 2019
diff --git a/packages/flutter/lib/src/animation/animation_controller.dart b/packages/flutter/lib/src/animation/animation_controller.dart
@@ -41,16 +41,17 @@ const Tolerance _kFlingTolerance = Tolerance(
   distance: 0.01,
 );
 
-/// Configures how an [AnimationController] behaves when animations are disabled.
+/// Configures how an [AnimationController] behaves when animations are
+/// disabled.
 ///
 /// When [AccessibilityFeatures.disableAnimations] is true, the device is asking
 /// Flutter to reduce or disable animations as much as possible. To honor this,
-/// we reduce the duration and the corresponding number of frames for animations.
-/// This enum is used to allow certain [AnimationController]s to opt out of this
-/// behavior.
+/// we reduce the duration and the corresponding number of frames for
+/// animations. This enum is used to allow certain [AnimationController]s to opt
+/// out of this behavior.
 ///
 /// For example, the [AnimationController] which controls the physics simulation
-/// for a scrollable list will have [AnimationBehavior.preserve] so that when
+/// for a scrollable list will have [AnimationBehavior.preserve], so that when
 /// a user attempts to scroll it does not jump to the end/beginning too quickly.
 enum AnimationBehavior {
   /// The [AnimationController] will reduce its duration when

diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart
@@ -129,6 +129,11 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
   Uri getTestUri(Uri key, int version) => key;
 
   /// Calculate the appropriate basedir for the current test context.
+  ///
+  /// The optional [suffix] argument is used by the
+  /// [FlutterSkiaGoldFileComparator] and the [FlutterPreSubmitFileComparator].
+  /// These [FlutterGoldenFileComparators] randomize their base directories to
+  /// maintain thread safety while using the `goldctl` tool.
   @protected
   @visibleForTesting
   static Directory getBaseDirectory(LocalFileComparator defaultComparator, Platform platform, {String suffix = ''}) {
@@ -217,10 +222,7 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
       platform,
       suffix: '${math.Random().nextInt(10000)}',
     );
-
-    if(!baseDirectory.existsSync()) {
-      baseDirectory.createSync(recursive: true);
-    }
+    baseDirectory.createSync(recursive: true);
 
     goldens ??= SkiaGoldClient(baseDirectory);
     await goldens.auth();
@@ -299,47 +301,23 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
     final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
       defaultComparator,
       platform,
+      suffix: '${math.Random().nextInt(10000)}',
     );
-
-    if(!baseDirectory.existsSync()) {
-      baseDirectory.createSync(recursive: true);
-    }
+    baseDirectory.createSync(recursive: true);
 
     goldens ??= SkiaGoldClient(baseDirectory);
-    await goldens.getExpectations();
-
+    await goldens.auth();
+    await goldens.tryjobInit();
     return FlutterPreSubmitFileComparator(baseDirectory.uri, goldens);
   }
 
   @override
   Future<bool> compare(Uint8List imageBytes, Uri golden) async {
     golden = _addPrefix(golden);
-    final String testName = skiaClient.cleanTestName(golden.path);
-    final List<String> testExpectations = skiaClient.expectations[testName];
-
-    if (testExpectations == null) {
-      // There is no baseline for this test
-      return true;
-    }
-
-    ComparisonResult result;
-    for (String expectation in testExpectations) {
-      final List<int> goldenBytes = await skiaClient.getImageBytes(expectation);
-
-      result = GoldenFileComparator.compareLists(
-        imageBytes,
-        goldenBytes,
-      );
-
-      if (result.passed) {
-        return true;
-      }
-    }
+    await update(golden, imageBytes);
+    final File goldenFile = getGoldenFile(golden);
 
-    return skiaClient.testIsIgnoredForPullRequest(
-      platform.environment['CIRRUS_PR'] ?? '',
-      golden.path,
-    );
+    return skiaClient.tryjobAdd(golden.path, goldenFile);
   }
 
   /// Decides based on the current environment whether goldens tests should be

diff --git a/packages/flutter_goldens/test/flutter_goldens_test.dart b/packages/flutter_goldens/test/flutter_goldens_test.dart
@@ -84,14 +84,12 @@ 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();
       });
@@ -212,120 +210,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/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;
@@ -507,39 +391,6 @@ void main() {
     });
 
     group('Pre-Submit', () {
-      FlutterPreSubmitFileComparator comparator;
-      final MockSkiaGoldClient mockSkiaClient = MockSkiaGoldClient();
-
-      setUp(() {
-        final Directory basedir = fs.directory('flutter/test/library/')
-          ..createSync(recursive: true);
-        comparator = FlutterPreSubmitFileComparator(
-          basedir.uri,
-          mockSkiaClient,
-          fs: fs,
-          platform: FakePlatform(
-            environment: <String, String>{
-              'FLUTTER_ROOT': _kFlutterRoot,
-              'CIRRUS_CI' : 'true',
-              'CIRRUS_PR' : '1234',
-              'GOLD_SERVICE_ACCOUNT' : 'service account...'
-            },
-            operatingSystem: 'macos'
-          ),
-        );
-
-        when(mockSkiaClient.getImageBytes('55109a4bed52acc780530f7a9aeff6c0'))
-          .thenAnswer((_) => Future<List<int>>.value(_kTestPngBytes));
-        when(mockSkiaClient.expectations)
-          .thenReturn(expectationsTemplate());
-        when(mockSkiaClient.cleanTestName('library.flutter.golden_test.1.png'))
-          .thenReturn('flutter.golden_test.1');
-        when(mockSkiaClient.isValidDigestForExpectation(
-          '55109a4bed52acc780530f7a9aeff6c0',
-          'library.flutter.golden_test.1.png',
-        ))
-          .thenAnswer((_) => Future<bool>.value(false));
-      });
       group('correctly determines testing environment', () {
         test('returns true', () {
           platform = FakePlatform(
@@ -601,50 +452,6 @@ void main() {
           );
         });
       });
-
-      test('comparison passes test that is ignored for this PR', () 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(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(
-          await comparator.compare(
-            Uint8List.fromList(_kFailPngBytes),
-            Uri.parse('flutter.golden_test.1.png'),
-          ),
-          isFalse,
-        );
-      });
-
-      test('passes non-existent baseline for new test', () async {
-        expect(
-          await comparator.compare(
-            Uint8List.fromList(_kFailPngBytes),
-            Uri.parse('flutter.new_golden_test.1.png'),
-          ),
-          isTrue,
-        );
-      });
     });
 
     group('Skipping', () {

diff --git a/packages/flutter_goldens/test/json_templates.dart b/packages/flutter_goldens/test/json_templates.dart
@@ -169,37 +169,6 @@ 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": "[email protected]",
-        "updatedBy": "[email protected]",
-        "expires": "$expires",
-        "query": "ext=png&name=$testName",
-        "note": "https://github.com/flutter/flutter/pull/$pullRequestNumber"
-      },
-      {
-        "id": "7579425228619212078",
-        "name": "[email protected]",
-        "updatedBy": "[email protected]",
-        "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() {