Allow Flutter golden file tests to be flaky (flutter#114450)

* allow marking a golden check as flaky * add matchesFlutterGolden to analyze.dart; no tags for flutter_goldens/lib * Pause * ++ * ++ * Analyzer therapy * Once more with feeling * Nits * Review feedback * Silly oops * Test progress * More tests * Finish * Nits * Analyzer * Review feedback Co-authored-by: Yegor Jbanov <[email protected]>
gspencergoog · Jan 19, 2023 · 06ac31c · 06ac31c
1 parent db4439b
commit 06ac31c
Show file tree

Hide file tree

Showing 35 changed files with 2,093 additions and 1,351 deletions.
diff --git a/dev/automated_tests/flutter_test/flutter_gold_test.dart b/dev/automated_tests/flutter_test/flutter_gold_test.dart
@@ -8,7 +8,7 @@ import 'dart:typed_data';
 
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
-import 'package:flutter_goldens/flutter_goldens.dart';
+import 'package:flutter_goldens/src/flutter_goldens_io.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:platform/platform.dart';
 

diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart
@@ -418,8 +418,8 @@ Future<void> verifyNoSyncAsyncStar(String workingDirectory, {int minimumMatches
   }
 }
 
-final RegExp _findGoldenTestPattern = RegExp(r'matchesGoldenFile\(');
-final RegExp _findGoldenDefinitionPattern = RegExp(r'matchesGoldenFile\(Object');
+final RegExp _findGoldenTestPattern = RegExp(r'(matchesGoldenFile|expectFlakyGolden)\(');
+final RegExp _findGoldenDefinitionPattern = RegExp(r'(matchesGoldenFile|expectFlakyGolden)\(Object');
 final RegExp _leadingComment = RegExp(r'//');
 final RegExp _goldenTagPattern1 = RegExp(r'@Tags\(');
 final RegExp _goldenTagPattern2 = RegExp(r"'reduced-test-set'");
@@ -431,8 +431,17 @@ const String _ignoreGoldenTag = '// flutter_ignore: golden_tag (see analyze.dart
 const String _ignoreGoldenTagForFile = '// flutter_ignore_for_file: golden_tag (see analyze.dart)';
 
 Future<void> verifyGoldenTags(String workingDirectory, { int minimumMatches = 2000 }) async {
+  // Skip flutter_goldens/lib because this library uses `matchesGoldenFile`
+  // but is not itself a test that needs tags.
+  final String flutterGoldensPackageLib = path.join(flutterPackages, 'flutter_goldens', 'lib');
+  bool isWithinFlutterGoldenLib(File file) {
+    return path.isWithin(flutterGoldensPackageLib, file.path);
+  }
+
   final List<String> errors = <String>[];
-  await for (final File file in _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)) {
+  final Stream<File> allTestFiles = _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)
+    .where((File file) => !isWithinFlutterGoldenLib(file));
+  await for (final File file in allTestFiles) {
     bool needsTag = false;
     bool hasTagNotation = false;
     bool hasReducedTag = false;

diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/flaky_golden_ignore.dart b/dev/bots/test/analyze-test-input/root/packages/foo/flaky_golden_ignore.dart
@@ -0,0 +1,16 @@
+// 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.
+
+// This would fail analysis, but it is ignored
+// flutter_ignore_for_file: golden_tag (see analyze.dart)
+
+@Tags(<String>['some-other-tag'])
+
+import 'package:test/test.dart';
+
+import 'golden_class.dart';
+
+void main() {
+  expectFlakyGolden('key', 'String');
+}
diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/flaky_golden_missing_tag.dart b/dev/bots/test/analyze-test-input/root/packages/foo/flaky_golden_missing_tag.dart
@@ -0,0 +1,14 @@
+// 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.
+
+// The reduced test set tag is missing. This should fail analysis.
+@Tags(<String>['some-other-tag'])
+
+import 'package:test/test.dart';
+
+import 'golden_class.dart';
+
+void main() {
+  expectFlakyGolden('finder', 'missing_tag.png');
+}
diff --git a/examples/api/test/goldens_io.dart → ...oot/packages/foo/flaky_golden_no_tag.dart b/examples/api/test/goldens_io.dart → ...oot/packages/foo/flaky_golden_no_tag.dart
@@ -2,4 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-export 'package:flutter_goldens/flutter_goldens.dart' show testExecutable;
+// The tag is missing. This should fail analysis.
+
+import 'golden_class.dart';
+
+void main() {
+  expectFlakyGolden('key', 'missing_tag.png');
+}
diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/golden_class.dart b/dev/bots/test/analyze-test-input/root/packages/foo/golden_class.dart
@@ -7,3 +7,7 @@
 void matchesGoldenFile(Object key) {
   return;
 }
+
+void expectFlakyGolden(Object key, String string){
+  return;
+}
diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/golden_doc.dart b/dev/bots/test/analyze-test-input/root/packages/foo/golden_doc.dart
@@ -35,8 +35,11 @@
 /// ```
 /// {@end-tool}
 ///
+/// expectFlakyGolden(a, b)
 
 // Other comments
 // matchesGoldenFile('comment.png');
+// expectFlakyGolden(a, b);
 
 String literal = 'matchesGoldenFile()'; // flutter_ignore: golden_tag (see analyze.dart)
+String flakyLiteral = 'expectFlakyGolden';
diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart
@@ -79,7 +79,9 @@ void main() {
                          'at the top of the file before import statements.';
     const String missingTag = "Files containing golden tests must be tagged with 'reduced-test-set'.";
     final List<String> lines = <String>[
+        '║ test/analyze-test-input/root/packages/foo/flaky_golden_no_tag.dart: $noTag',
         '║ test/analyze-test-input/root/packages/foo/golden_missing_tag.dart: $missingTag',
+        '║ test/analyze-test-input/root/packages/foo/flaky_golden_missing_tag.dart: $missingTag',
         '║ test/analyze-test-input/root/packages/foo/golden_no_tag.dart: $noTag',
       ]
       .map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/'))

diff --git a/dev/devicelab/bin/tasks/technical_debt__cost.dart b/dev/devicelab/bin/tasks/technical_debt__cost.dart
@@ -17,6 +17,7 @@ const double todoCost = 1009.0; // about two average SWE days, in dollars
 const double ignoreCost = 2003.0; // four average SWE days, in dollars
 const double pythonCost = 3001.0; // six average SWE days, in dollars
 const double skipCost = 2473.0; // 20 hours: 5 to fix the issue we're ignoring, 15 to fix the bugs we missed because the test was off
+const double flakyGoldenCost = 2467.0; // Similar to skip cost
 const double ignoreForFileCost = 2477.0; // similar thinking as skipCost
 const double asDynamicCost = 2011.0; // a few days to refactor the code.
 const double deprecationCost = 233.0; // a few hours to remove the old code.
@@ -69,6 +70,9 @@ Future<double> findCostsForFile(File file) async {
     if (isTest && line.contains('skip:') && !line.contains('[intended]')) {
       total += skipCost;
     }
+    if (isTest && line.contains('expectFlakyGolden(')) {
+      total += flakyGoldenCost;
+    }
     if (isDart && isOptingOutOfNullSafety(line)) {
       total += fileNullSafetyMigrationCost;
     }

diff --git a/examples/api/test/flutter_test_config.dart b/examples/api/test/flutter_test_config.dart
@@ -5,7 +5,7 @@
 import 'dart:async';
 
 
-import 'goldens_io.dart' if (dart.library.html) 'goldens_web.dart' as flutter_goldens;
+import 'package:flutter_goldens/flutter_goldens.dart' as flutter_goldens;
 
 Future<void> testExecutable(FutureOr<void> Function() testMain) {
   // Enable golden file testing using Skia Gold.

diff --git a/examples/api/test/goldens_web.dart b/examples/api/test/goldens_web.dart
diff --git a/packages/flutter/test/_goldens_io.dart b/packages/flutter/test/_goldens_io.dart
diff --git a/packages/flutter/test/_goldens_web.dart b/packages/flutter/test/_goldens_web.dart
diff --git a/packages/flutter/test/cupertino/date_picker_test.dart b/packages/flutter/test/cupertino/date_picker_test.dart
@@ -17,10 +17,11 @@ import 'dart:ui';
 import 'package:flutter/cupertino.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
+import 'package:flutter_goldens/flutter_goldens.dart' show expectFlakyGolden;
 import 'package:flutter_test/flutter_test.dart';
 
 // TODO(yjbanov): on the web text rendered with perspective produces flaky goldens: https://github.com/flutter/flutter/issues/110785
-const bool skipPerspectiveTextGoldens = isBrowser;
+const bool perspectiveTestIsFlaky = isBrowser;
 
 // A number of the hit tests below say "warnIfMissed: false". This is because
 // the way the CupertinoPicker works, the hits don't actually reach the labels,
@@ -1197,23 +1198,41 @@ void main() {
       }
 
       await tester.pumpWidget(buildApp(CupertinoDatePickerMode.time));
-      if (!skipPerspectiveTextGoldens) {
+
+      if (perspectiveTestIsFlaky) {
+        await expectFlakyGolden(
+          find.byType(CupertinoDatePicker),
+          'date_picker_test.time.initial.png',
+        );
+      } else {
         await expectLater(
           find.byType(CupertinoDatePicker),
           matchesGoldenFile('date_picker_test.time.initial.png'),
         );
       }
 
       await tester.pumpWidget(buildApp(CupertinoDatePickerMode.date));
-      if (!skipPerspectiveTextGoldens) {
+
+      if (perspectiveTestIsFlaky) {
+        await expectFlakyGolden(
+          find.byType(CupertinoDatePicker),
+          'date_picker_test.date.initial.png',
+        );
+      } else {
         await expectLater(
           find.byType(CupertinoDatePicker),
           matchesGoldenFile('date_picker_test.date.initial.png'),
         );
       }
 
       await tester.pumpWidget(buildApp(CupertinoDatePickerMode.dateAndTime));
-      if (!skipPerspectiveTextGoldens) {
+
+      if (perspectiveTestIsFlaky) {
+        await expectFlakyGolden(
+          find.byType(CupertinoDatePicker),
+          'date_picker_test.datetime.initial.png',
+        );
+      } else {
         await expectLater(
           find.byType(CupertinoDatePicker),
           matchesGoldenFile('date_picker_test.datetime.initial.png'),
@@ -1224,7 +1243,12 @@ void main() {
       await tester.drag(find.text('4'), Offset(0, _kRowOffset.dy / 2), warnIfMissed: false); // see top of file
       await tester.pump();
 
-      if (!skipPerspectiveTextGoldens) {
+      if (perspectiveTestIsFlaky) {
+        await expectFlakyGolden(
+          find.byType(CupertinoDatePicker),
+          'date_picker_test.datetime.drag.png',
+        );
+      } else {
         await expectLater(
           find.byType(CupertinoDatePicker),
           matchesGoldenFile('date_picker_test.datetime.drag.png'),
@@ -1314,7 +1338,12 @@ void main() {
       ),
     );
 
-    if (!skipPerspectiveTextGoldens) {
+    if (perspectiveTestIsFlaky) {
+      await expectFlakyGolden(
+        find.byType(CupertinoTimerPicker),
+        'timer_picker_test.datetime.initial.png',
+      );
+    } else {
       await expectLater(
         find.byType(CupertinoTimerPicker),
         matchesGoldenFile('timer_picker_test.datetime.initial.png'),
@@ -1325,7 +1354,12 @@ void main() {
     await tester.drag(find.text('59'), Offset(0, _kRowOffset.dy / 2), warnIfMissed: false); // see top of file
     await tester.pump();
 
-    if (!skipPerspectiveTextGoldens) {
+    if (perspectiveTestIsFlaky) {
+      await expectFlakyGolden(
+        find.byType(CupertinoTimerPicker),
+        'timer_picker_test.datetime.drag.png',
+      );
+    } else {
       await expectLater(
         find.byType(CupertinoTimerPicker),
         matchesGoldenFile('timer_picker_test.datetime.drag.png'),

diff --git a/packages/flutter/test/flutter_test_config.dart b/packages/flutter/test/flutter_test_config.dart
@@ -5,11 +5,9 @@
 import 'dart:async';
 
 import 'package:flutter/rendering.dart';
+import 'package:flutter_goldens/flutter_goldens.dart' as flutter_goldens;
 import 'package:flutter_test/flutter_test.dart';
 
-import '_goldens_io.dart'
-  if (dart.library.html) '_goldens_web.dart' as flutter_goldens;
-
 Future<void> testExecutable(FutureOr<void> Function() testMain) {
   // Enable checks because there are many implementations of [RenderBox] in this
   // package can benefit from the additional validations.
@@ -22,3 +20,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain) {
   // Enable golden file testing using Skia Gold.
   return flutter_goldens.testExecutable(testMain);
 }
+
+Future<void> processBrowserCommand(dynamic command) {
+  return flutter_goldens.processBrowserCommand(command);
+}
diff --git a/packages/flutter/test/flutter_web_test_config.dart b/packages/flutter/test/flutter_web_test_config.dart
@@ -0,0 +1,25 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+
+import 'flutter_test_config.dart';
+
+/// A custom host configuration for browser tests that supports flaky golden
+/// checks.
+///
+/// See also [processBrowserCommand].
+Future<void> startWebTestHostConfiguration(String testUri) async {
+  testExecutable(() async {
+    final Stream<dynamic> commands = stdin
+      .transform<String>(utf8.decoder)
+      .transform<String>(const LineSplitter())
+      .map<dynamic>(jsonDecode);
+    await for (final dynamic command in commands) {
+      await processBrowserCommand(command);
+    }
+  });
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,3 +7,7 @@ @@
     void matchesGoldenFile(Object key) {
       return;
     }
+    void expectFlakyGolden(Object key, String string){
+      return;
+    }