Skip to content

Commit

Permalink
Revert "Allow Flutter golden file tests to be flaky (#114450)" (#114902)
Browse files Browse the repository at this point in the history
This reverts commit 53e6876.
  • Loading branch information
zanderso authored Nov 8, 2022
1 parent a1432a9 commit a84e369
Show file tree
Hide file tree
Showing 35 changed files with 1,351 additions and 2,093 deletions.
2 changes: 1 addition & 1 deletion dev/automated_tests/flutter_test/flutter_gold_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'dart:typed_data';

import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:flutter_goldens/src/flutter_goldens_io.dart';
import 'package:flutter_goldens/flutter_goldens.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:platform/platform.dart';

Expand Down
15 changes: 3 additions & 12 deletions dev/bots/analyze.dart
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ Future<void> verifyNoSyncAsyncStar(String workingDirectory, {int minimumMatches
}
}

final RegExp _findGoldenTestPattern = RegExp(r'(matchesGoldenFile|expectFlakyGolden)\(');
final RegExp _findGoldenDefinitionPattern = RegExp(r'(matchesGoldenFile|expectFlakyGolden)\(Object');
final RegExp _findGoldenTestPattern = RegExp(r'matchesGoldenFile\(');
final RegExp _findGoldenDefinitionPattern = RegExp(r'matchesGoldenFile\(Object');
final RegExp _leadingComment = RegExp(r'//');
final RegExp _goldenTagPattern1 = RegExp(r'@Tags\(');
final RegExp _goldenTagPattern2 = RegExp(r"'reduced-test-set'");
Expand All @@ -431,17 +431,8 @@ 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>[];
final Stream<File> allTestFiles = _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)
.where((File file) => !isWithinFlutterGoldenLib(file));
await for (final File file in allTestFiles) {
await for (final File file in _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)) {
bool needsTag = false;
bool hasTagNotation = false;
bool hasReducedTag = false;
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,3 @@
void matchesGoldenFile(Object key) {
return;
}

void expectFlakyGolden(Object key, String string){
return;
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,8 @@
/// ```
/// {@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';
2 changes: 0 additions & 2 deletions dev/bots/test/analyze_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ 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'\' : '/'))
Expand Down
4 changes: 0 additions & 4 deletions dev/devicelab/bin/tasks/technical_debt__cost.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ 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.
Expand Down Expand Up @@ -70,9 +69,6 @@ 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;
}
Expand Down
2 changes: 1 addition & 1 deletion examples/api/test/flutter_test_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import 'dart:async';


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

Future<void> testExecutable(FutureOr<void> Function() testMain) {
// Enable golden file testing using Skia Gold.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,4 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// The tag is missing. This should fail analysis.

import 'golden_class.dart';

void main() {
expectFlakyGolden('key', 'missing_tag.png');
}
export 'package:flutter_goldens/flutter_goldens.dart' show testExecutable;
8 changes: 8 additions & 0 deletions examples/api/test/goldens_web.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// 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';

// package:flutter_goldens is not used as part of the test process for web.
Future<void> testExecutable(FutureOr<void> Function() testMain) async => testMain();
5 changes: 5 additions & 0 deletions packages/flutter/test/_goldens_io.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// 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.

export 'package:flutter_goldens/flutter_goldens.dart' show testExecutable;
8 changes: 8 additions & 0 deletions packages/flutter/test/_goldens_web.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// 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';

// package:flutter_goldens is not used as part of the test process for web.
Future<void> testExecutable(FutureOr<void> Function() testMain) async => testMain();
48 changes: 7 additions & 41 deletions packages/flutter/test/cupertino/date_picker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ 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 perspectiveTestIsFlaky = isBrowser;
const bool skipPerspectiveTextGoldens = 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,
Expand Down Expand Up @@ -1198,41 +1197,23 @@ void main() {
}

await tester.pumpWidget(buildApp(CupertinoDatePickerMode.time));

if (perspectiveTestIsFlaky) {
await expectFlakyGolden(
find.byType(CupertinoDatePicker),
'date_picker_test.time.initial.png',
);
} else {
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.time.initial.png'),
);
}

await tester.pumpWidget(buildApp(CupertinoDatePickerMode.date));

if (perspectiveTestIsFlaky) {
await expectFlakyGolden(
find.byType(CupertinoDatePicker),
'date_picker_test.date.initial.png',
);
} else {
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.date.initial.png'),
);
}

await tester.pumpWidget(buildApp(CupertinoDatePickerMode.dateAndTime));

if (perspectiveTestIsFlaky) {
await expectFlakyGolden(
find.byType(CupertinoDatePicker),
'date_picker_test.datetime.initial.png',
);
} else {
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.datetime.initial.png'),
Expand All @@ -1243,12 +1224,7 @@ void main() {
await tester.drag(find.text('4'), Offset(0, _kRowOffset.dy / 2), warnIfMissed: false); // see top of file
await tester.pump();

if (perspectiveTestIsFlaky) {
await expectFlakyGolden(
find.byType(CupertinoDatePicker),
'date_picker_test.datetime.drag.png',
);
} else {
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoDatePicker),
matchesGoldenFile('date_picker_test.datetime.drag.png'),
Expand Down Expand Up @@ -1338,12 +1314,7 @@ void main() {
),
);

if (perspectiveTestIsFlaky) {
await expectFlakyGolden(
find.byType(CupertinoTimerPicker),
'timer_picker_test.datetime.initial.png',
);
} else {
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoTimerPicker),
matchesGoldenFile('timer_picker_test.datetime.initial.png'),
Expand All @@ -1354,12 +1325,7 @@ void main() {
await tester.drag(find.text('59'), Offset(0, _kRowOffset.dy / 2), warnIfMissed: false); // see top of file
await tester.pump();

if (perspectiveTestIsFlaky) {
await expectFlakyGolden(
find.byType(CupertinoTimerPicker),
'timer_picker_test.datetime.drag.png',
);
} else {
if (!skipPerspectiveTextGoldens) {
await expectLater(
find.byType(CupertinoTimerPicker),
matchesGoldenFile('timer_picker_test.datetime.drag.png'),
Expand Down
8 changes: 3 additions & 5 deletions packages/flutter/test/flutter_test_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
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.
Expand All @@ -20,7 +22,3 @@ 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);
}
25 changes: 0 additions & 25 deletions packages/flutter/test/flutter_web_test_config.dart

This file was deleted.

Loading

0 comments on commit a84e369

Please sign in to comment.