Skip to content

Commit

Permalink
Analyze against using Stopwatches in the framework (#138507)
Browse files Browse the repository at this point in the history
  • Loading branch information
Piinks authored Nov 29, 2023
1 parent 4d8ae57 commit 133711b
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 6 deletions.
45 changes: 45 additions & 0 deletions dev/bots/analyze.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ Future<void> run(List<String> arguments) async {
printProgress('Goldens...');
await verifyGoldenTags(flutterPackages);

printProgress('Prevent flakes from Stopwatches...');
await verifyNoStopwatches(flutterPackages);

printProgress('Skip test comments...');
await verifySkipTestComments(flutterRoot);

Expand Down Expand Up @@ -584,6 +587,48 @@ Future<void> verifyGoldenTags(String workingDirectory, { int minimumMatches = 20
}
}

/// Use of Stopwatches can introduce test flakes as the logical time of a
/// stopwatch can fall out of sync with the mocked time of FakeAsync in testing.
/// The Clock object provides a safe stopwatch instead, which is paired with
/// FakeAsync as part of the test binding.
final RegExp _findStopwatchPattern = RegExp(r'Stopwatch\(\)');
const String _ignoreStopwatch = '// flutter_ignore: stopwatch (see analyze.dart)';
const String _ignoreStopwatchForFile = '// flutter_ignore_for_file: stopwatch (see analyze.dart)';

Future<void> verifyNoStopwatches(String workingDirectory, { int minimumMatches = 2000 }) async {
final List<String> errors = <String>[];
await for (final File file in _allFiles(workingDirectory, 'dart', minimumMatches: minimumMatches)) {
if (file.path.contains('flutter_tool')) {
// Skip flutter_tool package.
continue;
}
int lineNumber = 1;
final List<String> lines = file.readAsLinesSync();
for (final String line in lines) {
// If the file is being ignored, skip parsing the rest of the lines.
if (line.contains(_ignoreStopwatchForFile)) {
break;
}

if (line.contains(_findStopwatchPattern)
&& !line.contains(_leadingComment)
&& !line.contains(_ignoreStopwatch)) {
// Stopwatch found
errors.add('\t${file.path}:$lineNumber');
}
lineNumber++;
}
}
if (errors.isNotEmpty) {
foundError(<String>[
'Stopwatch use was found in the following files:',
...errors,
'${bold}Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.$reset',
'A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.'
]);
}
}

final RegExp _findDeprecationPattern = RegExp(r'@[Dd]eprecated');
final RegExp _deprecationStartPattern = RegExp(r'^(?<indent> *)@Deprecated\($'); // flutter_ignore: deprecation_syntax (see analyze.dart)
final RegExp _deprecationMessagePattern = RegExp(r"^ *'(?<message>.+) '$");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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.

/// Sample Code
///
/// No analysis failures should be found.
///
/// {@tool snippet}
/// Sample invocations of [Stopwatch].
///
/// ```dart
/// Stopwatch();
/// ```
/// {@end-tool}
String? foo;
// Other comments
// Stopwatch();

String literal = 'Stopwatch()'; // flutter_ignore: stopwatch (see analyze.dart)
Original file line number Diff line number Diff line change
@@ -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.

// This should fail analysis.

void main() {
Stopwatch();

// Identify more than one in a file.
Stopwatch myStopwatch;
myStopwatch = Stopwatch();
myStopwatch.reset();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// 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: stopwatch (see analyze.dart)

void main() {
Stopwatch();
}
18 changes: 18 additions & 0 deletions dev/bots/test/analyze_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,24 @@ void main() {
expect(result[result.length - 1], ''); // trailing newline
});

test('analyze.dart - verifyNoStopwatches', () async {
final List<String> result = (await capture(() => verifyNoStopwatches(testRootPath, minimumMatches: 6), shouldHaveErrors: true)).split('\n');
final List<String> lines = <String>[
'║ \ttest/analyze-test-input/root/packages/foo/stopwatch_fail.dart:8',
'║ \ttest/analyze-test-input/root/packages/foo/stopwatch_fail.dart:12',
]
.map((String line) => line.replaceAll('/', Platform.isWindows ? r'\' : '/'))
.toList();
expect(result.length, 6 + lines.length, reason: 'output had unexpected number of lines:\n${result.join('\n')}');
expect(result[0], '╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════');
expect(result[1], '║ Stopwatch use was found in the following files:');
expect(result.getRange(2, result.length - 4).toSet(), lines.toSet());
expect(result[result.length - 4], '║ Stopwatches introduce flakes by falling out of sync with the FakeAsync used in testing.');
expect(result[result.length - 3], '║ A Stopwatch that stays in sync with FakeAsync is available through the Gesture or Test bindings, through samplingClock.');
expect(result[result.length - 2], '╚═══════════════════════════════════════════════════════════════════════════════');
expect(result[result.length - 1], ''); // trailing newline
});

test('analyze.dart - verifyNoMissingLicense', () async {
final String result = await capture(() => verifyNoMissingLicense(testRootPath, checkMinimums: false), shouldHaveErrors: true);
final String file = 'test/analyze-test-input/root/packages/foo/foo.dart'
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/lib/src/foundation/debug.dart
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ Future<T> debugInstrumentAction<T>(String description, Future<T> Function() acti
return true;
}());
if (instrument) {
final Stopwatch stopwatch = Stopwatch()..start();
final Stopwatch stopwatch = Stopwatch()..start(); // flutter_ignore: stopwatch (see analyze.dart)
// Ignore context: The framework does not use this function internally so it will not cause flakes.
try {
return await action();
} finally {
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/lib/src/foundation/print.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ int _debugPrintedCharacters = 0;
const int _kDebugPrintCapacity = 12 * 1024;
const Duration _kDebugPrintPauseTime = Duration(seconds: 1);
final Queue<String> _debugPrintBuffer = Queue<String>();
final Stopwatch _debugPrintStopwatch = Stopwatch();
final Stopwatch _debugPrintStopwatch = Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
// Ignore context: This is not used in tests, only for throttled logging.
Completer<void>? _debugPrintCompleter;
bool _debugPrintScheduled = false;
void _debugPrintTask() {
Expand Down
11 changes: 9 additions & 2 deletions packages/flutter/lib/src/gestures/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ class SamplingClock {
DateTime now() => DateTime.now();

/// Returns a new stopwatch that uses the current time as reported by `this`.
Stopwatch stopwatch() => Stopwatch();
///
/// See also:
///
/// * [GestureBinding.debugSamplingClock], which is used in tests and
/// debug builds to observe [FakeAsync].
Stopwatch stopwatch() => Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
// Ignore context: This is replaced by debugSampling clock in the test binding.
}

// Class that handles resampling of touch events for multiple pointer
Expand All @@ -59,7 +65,8 @@ class _Resampler {
Duration _frameTime = Duration.zero;

// Time since `_frameTime` was updated.
Stopwatch _frameTimeAge = Stopwatch();
Stopwatch _frameTimeAge = Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
// Ignore context: This is tested safely outside of FakeAsync.

// Last sample time and time stamp of last event.
//
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/test/foundation/timeline_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ void main() {
// a bit inconsistent with Stopwatch.
final int start = FlutterTimeline.now - 1;
FlutterTimeline.timeSync('TEST', () {
final Stopwatch watch = Stopwatch()..start();
final Stopwatch watch = Stopwatch()..start(); // flutter_ignore: stopwatch (see analyze.dart)
// Ignore context: Used safely for benchmarking.
while (watch.elapsedMilliseconds < 5) {}
watch.stop();
});
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter_test/lib/src/test_compat.dart
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ class _Reporter {
final bool _printPath;

/// A stopwatch that tracks the duration of the full run.
final Stopwatch _stopwatch = Stopwatch();
final Stopwatch _stopwatch = Stopwatch(); // flutter_ignore: stopwatch (see analyze.dart)
// Ignore context: Used for logging of actual test runs, outside of FakeAsync.

/// The size of `_engine.passed` last time a progress notification was
/// printed.
Expand Down

0 comments on commit 133711b

Please sign in to comment.