Skip to content

Commit 53e6876

Browse files
Piinksyjbanov
andauthored
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]>
1 parent 139b8f4 commit 53e6876

35 files changed

+2093
-1351
lines changed

dev/automated_tests/flutter_test/flutter_gold_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import 'dart:typed_data';
88

99
import 'package:file/file.dart';
1010
import 'package:file/memory.dart';
11-
import 'package:flutter_goldens/flutter_goldens.dart';
11+
import 'package:flutter_goldens/src/flutter_goldens_io.dart';
1212
import 'package:flutter_test/flutter_test.dart';
1313
import 'package:platform/platform.dart';
1414

dev/bots/analyze.dart

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,8 @@ Future<void> verifyNoSyncAsyncStar(String workingDirectory, {int minimumMatches
418418
}
419419
}
420420

421-
final RegExp _findGoldenTestPattern = RegExp(r'matchesGoldenFile\(');
422-
final RegExp _findGoldenDefinitionPattern = RegExp(r'matchesGoldenFile\(Object');
421+
final RegExp _findGoldenTestPattern = RegExp(r'(matchesGoldenFile|expectFlakyGolden)\(');
422+
final RegExp _findGoldenDefinitionPattern = RegExp(r'(matchesGoldenFile|expectFlakyGolden)\(Object');
423423
final RegExp _leadingComment = RegExp(r'//');
424424
final RegExp _goldenTagPattern1 = RegExp(r'@Tags\(');
425425
final RegExp _goldenTagPattern2 = RegExp(r"'reduced-test-set'");
@@ -431,8 +431,17 @@ const String _ignoreGoldenTag = '// flutter_ignore: golden_tag (see analyze.dart
431431
const String _ignoreGoldenTagForFile = '// flutter_ignore_for_file: golden_tag (see analyze.dart)';
432432

433433
Future<void> verifyGoldenTags(String workingDirectory, { int minimumMatches = 2000 }) async {
434+
// Skip flutter_goldens/lib because this library uses `matchesGoldenFile`
435+
// but is not itself a test that needs tags.
436+
final String flutterGoldensPackageLib = path.join(flutterPackages, 'flutter_goldens', 'lib');
437+
bool isWithinFlutterGoldenLib(File file) {
438+
return path.isWithin(flutterGoldensPackageLib, file.path);
439+
}
440+
434441
final List<String> errors = <String>[];
435-
await for (final File file in _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)) {
442+
final Stream<File> allTestFiles = _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)
443+
.where((File file) => !isWithinFlutterGoldenLib(file));
444+
await for (final File file in allTestFiles) {
436445
bool needsTag = false;
437446
bool hasTagNotation = false;
438447
bool hasReducedTag = false;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// This would fail analysis, but it is ignored
6+
// flutter_ignore_for_file: golden_tag (see analyze.dart)
7+
8+
@Tags(<String>['some-other-tag'])
9+
10+
import 'package:test/test.dart';
11+
12+
import 'golden_class.dart';
13+
14+
void main() {
15+
expectFlakyGolden('key', 'String');
16+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// The reduced test set tag is missing. This should fail analysis.
6+
@Tags(<String>['some-other-tag'])
7+
8+
import 'package:test/test.dart';
9+
10+
import 'golden_class.dart';
11+
12+
void main() {
13+
expectFlakyGolden('finder', 'missing_tag.png');
14+
}

examples/api/test/goldens_io.dart renamed to dev/bots/test/analyze-test-input/root/packages/foo/flaky_golden_no_tag.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,10 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
export 'package:flutter_goldens/flutter_goldens.dart' show testExecutable;
5+
// The tag is missing. This should fail analysis.
6+
7+
import 'golden_class.dart';
8+
9+
void main() {
10+
expectFlakyGolden('key', 'missing_tag.png');
11+
}

dev/bots/test/analyze-test-input/root/packages/foo/golden_class.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,7 @@
77
void matchesGoldenFile(Object key) {
88
return;
99
}
10+
11+
void expectFlakyGolden(Object key, String string){
12+
return;
13+
}

dev/bots/test/analyze-test-input/root/packages/foo/golden_doc.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,11 @@
3535
/// ```
3636
/// {@end-tool}
3737
///
38+
/// expectFlakyGolden(a, b)
3839
3940
// Other comments
4041
// matchesGoldenFile('comment.png');
42+
// expectFlakyGolden(a, b);
4143

4244
String literal = 'matchesGoldenFile()'; // flutter_ignore: golden_tag (see analyze.dart)
45+
String flakyLiteral = 'expectFlakyGolden';

dev/bots/test/analyze_test.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ void main() {
7979
'at the top of the file before import statements.';
8080
const String missingTag = "Files containing golden tests must be tagged with 'reduced-test-set'.";
8181
final List<String> lines = <String>[
82+
'║ test/analyze-test-input/root/packages/foo/flaky_golden_no_tag.dart: $noTag',
8283
'║ test/analyze-test-input/root/packages/foo/golden_missing_tag.dart: $missingTag',
84+
'║ test/analyze-test-input/root/packages/foo/flaky_golden_missing_tag.dart: $missingTag',
8385
'║ test/analyze-test-input/root/packages/foo/golden_no_tag.dart: $noTag',
8486
]
8587
.map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/'))

dev/devicelab/bin/tasks/technical_debt__cost.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const double todoCost = 1009.0; // about two average SWE days, in dollars
1717
const double ignoreCost = 2003.0; // four average SWE days, in dollars
1818
const double pythonCost = 3001.0; // six average SWE days, in dollars
1919
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
20+
const double flakyGoldenCost = 2467.0; // Similar to skip cost
2021
const double ignoreForFileCost = 2477.0; // similar thinking as skipCost
2122
const double asDynamicCost = 2011.0; // a few days to refactor the code.
2223
const double deprecationCost = 233.0; // a few hours to remove the old code.
@@ -69,6 +70,9 @@ Future<double> findCostsForFile(File file) async {
6970
if (isTest && line.contains('skip:') && !line.contains('[intended]')) {
7071
total += skipCost;
7172
}
73+
if (isTest && line.contains('expectFlakyGolden(')) {
74+
total += flakyGoldenCost;
75+
}
7276
if (isDart && isOptingOutOfNullSafety(line)) {
7377
total += fileNullSafetyMigrationCost;
7478
}

examples/api/test/flutter_test_config.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import 'dart:async';
66

77

8-
import 'goldens_io.dart' if (dart.library.html) 'goldens_web.dart' as flutter_goldens;
8+
import 'package:flutter_goldens/flutter_goldens.dart' as flutter_goldens;
99

1010
Future<void> testExecutable(FutureOr<void> Function() testMain) {
1111
// Enable golden file testing using Skia Gold.

examples/api/test/goldens_web.dart

Lines changed: 0 additions & 8 deletions
This file was deleted.

packages/flutter/test/_goldens_io.dart

Lines changed: 0 additions & 5 deletions
This file was deleted.

packages/flutter/test/_goldens_web.dart

Lines changed: 0 additions & 8 deletions
This file was deleted.

packages/flutter/test/cupertino/date_picker_test.dart

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ import 'dart:ui';
1717
import 'package:flutter/cupertino.dart';
1818
import 'package:flutter/material.dart';
1919
import 'package:flutter/rendering.dart';
20+
import 'package:flutter_goldens/flutter_goldens.dart' show expectFlakyGolden;
2021
import 'package:flutter_test/flutter_test.dart';
2122

2223
// TODO(yjbanov): on the web text rendered with perspective produces flaky goldens: https://github.com/flutter/flutter/issues/110785
23-
const bool skipPerspectiveTextGoldens = isBrowser;
24+
const bool perspectiveTestIsFlaky = isBrowser;
2425

2526
// A number of the hit tests below say "warnIfMissed: false". This is because
2627
// the way the CupertinoPicker works, the hits don't actually reach the labels,
@@ -1197,23 +1198,41 @@ void main() {
11971198
}
11981199

11991200
await tester.pumpWidget(buildApp(CupertinoDatePickerMode.time));
1200-
if (!skipPerspectiveTextGoldens) {
1201+
1202+
if (perspectiveTestIsFlaky) {
1203+
await expectFlakyGolden(
1204+
find.byType(CupertinoDatePicker),
1205+
'date_picker_test.time.initial.png',
1206+
);
1207+
} else {
12011208
await expectLater(
12021209
find.byType(CupertinoDatePicker),
12031210
matchesGoldenFile('date_picker_test.time.initial.png'),
12041211
);
12051212
}
12061213

12071214
await tester.pumpWidget(buildApp(CupertinoDatePickerMode.date));
1208-
if (!skipPerspectiveTextGoldens) {
1215+
1216+
if (perspectiveTestIsFlaky) {
1217+
await expectFlakyGolden(
1218+
find.byType(CupertinoDatePicker),
1219+
'date_picker_test.date.initial.png',
1220+
);
1221+
} else {
12091222
await expectLater(
12101223
find.byType(CupertinoDatePicker),
12111224
matchesGoldenFile('date_picker_test.date.initial.png'),
12121225
);
12131226
}
12141227

12151228
await tester.pumpWidget(buildApp(CupertinoDatePickerMode.dateAndTime));
1216-
if (!skipPerspectiveTextGoldens) {
1229+
1230+
if (perspectiveTestIsFlaky) {
1231+
await expectFlakyGolden(
1232+
find.byType(CupertinoDatePicker),
1233+
'date_picker_test.datetime.initial.png',
1234+
);
1235+
} else {
12171236
await expectLater(
12181237
find.byType(CupertinoDatePicker),
12191238
matchesGoldenFile('date_picker_test.datetime.initial.png'),
@@ -1224,7 +1243,12 @@ void main() {
12241243
await tester.drag(find.text('4'), Offset(0, _kRowOffset.dy / 2), warnIfMissed: false); // see top of file
12251244
await tester.pump();
12261245

1227-
if (!skipPerspectiveTextGoldens) {
1246+
if (perspectiveTestIsFlaky) {
1247+
await expectFlakyGolden(
1248+
find.byType(CupertinoDatePicker),
1249+
'date_picker_test.datetime.drag.png',
1250+
);
1251+
} else {
12281252
await expectLater(
12291253
find.byType(CupertinoDatePicker),
12301254
matchesGoldenFile('date_picker_test.datetime.drag.png'),
@@ -1314,7 +1338,12 @@ void main() {
13141338
),
13151339
);
13161340

1317-
if (!skipPerspectiveTextGoldens) {
1341+
if (perspectiveTestIsFlaky) {
1342+
await expectFlakyGolden(
1343+
find.byType(CupertinoTimerPicker),
1344+
'timer_picker_test.datetime.initial.png',
1345+
);
1346+
} else {
13181347
await expectLater(
13191348
find.byType(CupertinoTimerPicker),
13201349
matchesGoldenFile('timer_picker_test.datetime.initial.png'),
@@ -1325,7 +1354,12 @@ void main() {
13251354
await tester.drag(find.text('59'), Offset(0, _kRowOffset.dy / 2), warnIfMissed: false); // see top of file
13261355
await tester.pump();
13271356

1328-
if (!skipPerspectiveTextGoldens) {
1357+
if (perspectiveTestIsFlaky) {
1358+
await expectFlakyGolden(
1359+
find.byType(CupertinoTimerPicker),
1360+
'timer_picker_test.datetime.drag.png',
1361+
);
1362+
} else {
13291363
await expectLater(
13301364
find.byType(CupertinoTimerPicker),
13311365
matchesGoldenFile('timer_picker_test.datetime.drag.png'),

packages/flutter/test/flutter_test_config.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@
55
import 'dart:async';
66

77
import 'package:flutter/rendering.dart';
8+
import 'package:flutter_goldens/flutter_goldens.dart' as flutter_goldens;
89
import 'package:flutter_test/flutter_test.dart';
910

10-
import '_goldens_io.dart'
11-
if (dart.library.html) '_goldens_web.dart' as flutter_goldens;
12-
1311
Future<void> testExecutable(FutureOr<void> Function() testMain) {
1412
// Enable checks because there are many implementations of [RenderBox] in this
1513
// package can benefit from the additional validations.
@@ -22,3 +20,7 @@ Future<void> testExecutable(FutureOr<void> Function() testMain) {
2220
// Enable golden file testing using Skia Gold.
2321
return flutter_goldens.testExecutable(testMain);
2422
}
23+
24+
Future<void> processBrowserCommand(dynamic command) {
25+
return flutter_goldens.processBrowserCommand(command);
26+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:convert';
7+
import 'dart:io';
8+
9+
import 'flutter_test_config.dart';
10+
11+
/// A custom host configuration for browser tests that supports flaky golden
12+
/// checks.
13+
///
14+
/// See also [processBrowserCommand].
15+
Future<void> startWebTestHostConfiguration(String testUri) async {
16+
testExecutable(() async {
17+
final Stream<dynamic> commands = stdin
18+
.transform<String>(utf8.decoder)
19+
.transform<String>(const LineSplitter())
20+
.map<dynamic>(jsonDecode);
21+
await for (final dynamic command in commands) {
22+
await processBrowserCommand(command);
23+
}
24+
});
25+
}

0 commit comments

Comments
 (0)