Unverified Commit 0211df9c authored by Christopher Fujino's avatar Christopher Fujino Committed by GitHub

[flutter_tools] provide --timeout option to flutter drive (#114458)

parent 3b0f8335
...@@ -17,7 +17,14 @@ Future<void> main() async { ...@@ -17,7 +17,14 @@ Future<void> main() async {
final Directory galleryDir = Directory(path.join(galleryParentDir.path, 'gallery')); final Directory galleryDir = Directory(path.join(galleryParentDir.path, 'gallery'));
try { try {
await task(NewGalleryPerfTest(galleryDir).run); await task(
NewGalleryPerfTest(
galleryDir,
// time out after 20 minutes allowing the tool to take a screenshot to debug
// https://github.com/flutter/flutter/issues/114025.
timeoutSeconds: 20 * 60,
).run,
);
} finally { } finally {
rmTree(galleryParentDir); rmTree(galleryParentDir);
} }
......
...@@ -15,6 +15,7 @@ class NewGalleryPerfTest extends PerfTest { ...@@ -15,6 +15,7 @@ class NewGalleryPerfTest extends PerfTest {
String timelineFileName = 'transitions', String timelineFileName = 'transitions',
String dartDefine = '', String dartDefine = '',
bool enableImpeller = false, bool enableImpeller = false,
super.timeoutSeconds,
}) : super( }) : super(
galleryDir.path, galleryDir.path,
'test_driver/transitions_perf.dart', 'test_driver/transitions_perf.dart',
......
...@@ -914,6 +914,7 @@ class PerfTest { ...@@ -914,6 +914,7 @@ class PerfTest {
this.device, this.device,
this.flutterDriveCallback, this.flutterDriveCallback,
this.enableImpeller = false, this.enableImpeller = false,
this.timeoutSeconds,
}): _resultFilename = resultFilename; }): _resultFilename = resultFilename;
const PerfTest.e2e( const PerfTest.e2e(
...@@ -929,6 +930,7 @@ class PerfTest { ...@@ -929,6 +930,7 @@ class PerfTest {
this.device, this.device,
this.flutterDriveCallback, this.flutterDriveCallback,
this.enableImpeller = false, this.enableImpeller = false,
this.timeoutSeconds,
}) : saveTraceFile = false, timelineFileName = null, _resultFilename = resultFilename; }) : saveTraceFile = false, timelineFileName = null, _resultFilename = resultFilename;
/// The directory where the app under test is defined. /// The directory where the app under test is defined.
...@@ -963,6 +965,9 @@ class PerfTest { ...@@ -963,6 +965,9 @@ class PerfTest {
/// Whether the perf test should enable Impeller. /// Whether the perf test should enable Impeller.
final bool enableImpeller; final bool enableImpeller;
/// Number of seconds to time out the test after, allowing debug callbacks to run.
final int? timeoutSeconds;
/// The keys of the values that need to be reported. /// The keys of the values that need to be reported.
/// ///
/// If it's `null`, then report: /// If it's `null`, then report:
...@@ -1020,6 +1025,11 @@ class PerfTest { ...@@ -1020,6 +1025,11 @@ class PerfTest {
'-v', '-v',
'--verbose-system-logs', '--verbose-system-logs',
'--profile', '--profile',
if (timeoutSeconds != null)
...<String>[
'--timeout',
timeoutSeconds.toString(),
],
if (needsFullTimeline) if (needsFullTimeline)
'--trace-startup', // Enables "endless" timeline event buffering. '--trace-startup', // Enables "endless" timeline event buffering.
'-t', testTarget, '-t', testTarget,
...@@ -1039,7 +1049,7 @@ class PerfTest { ...@@ -1039,7 +1049,7 @@ class PerfTest {
if (flutterDriveCallback != null) { if (flutterDriveCallback != null) {
flutterDriveCallback!(options); flutterDriveCallback!(options);
} else { } else {
await flutter('drive', options:options); await flutter('drive', options: options);
} }
final Map<String, dynamic> data = json.decode( final Map<String, dynamic> data = json.decode(
file('${_testOutputDirectory(testDirectory)}/$resultFilename.json').readAsStringSync(), file('${_testOutputDirectory(testDirectory)}/$resultFilename.json').readAsStringSync(),
...@@ -1140,7 +1150,7 @@ class PerfTestWithSkSL extends PerfTest { ...@@ -1140,7 +1150,7 @@ class PerfTestWithSkSL extends PerfTest {
); );
@override @override
Future<TaskResult> run() async { Future<TaskResult> run({int? timeoutSeconds}) async {
return inDirectory<TaskResult>(testDirectory, () async { return inDirectory<TaskResult>(testDirectory, () async {
// Some initializations // Some initializations
_device = await devices.workingDevice; _device = await devices.workingDevice;
......
...@@ -150,7 +150,12 @@ class DriveCommand extends RunCommandBase { ...@@ -150,7 +150,12 @@ class DriveCommand extends RunCommandBase {
'Dart VM running The test script.') 'Dart VM running The test script.')
..addOption('profile-memory', help: 'Launch devtools and profile application memory, writing ' ..addOption('profile-memory', help: 'Launch devtools and profile application memory, writing '
'The output data to the file path provided to this argument as JSON.', 'The output data to the file path provided to this argument as JSON.',
valueHelp: 'profile_memory.json'); valueHelp: 'profile_memory.json')
..addOption('timeout',
help: 'Timeout the test after the given number of seconds. If the '
'"--screenshot" option is provided, a screenshot will be taken '
'before exiting. Defaults to no timeout.',
valueHelp: '360');
} }
final Signals signals; final Signals signals;
...@@ -173,6 +178,8 @@ class DriveCommand extends RunCommandBase { ...@@ -173,6 +178,8 @@ class DriveCommand extends RunCommandBase {
final FileSystem _fileSystem; final FileSystem _fileSystem;
final Logger _logger; final Logger _logger;
final FileSystemUtils _fsUtils; final FileSystemUtils _fsUtils;
Timer? timeoutTimer;
Map<ProcessSignal, Object>? screenshotTokens;
@override @override
final String name = 'drive'; final String name = 'drive';
...@@ -295,13 +302,17 @@ class DriveCommand extends RunCommandBase { ...@@ -295,13 +302,17 @@ class DriveCommand extends RunCommandBase {
androidEmulator: boolArgDeprecated('android-emulator'), androidEmulator: boolArgDeprecated('android-emulator'),
profileMemory: stringArgDeprecated('profile-memory'), profileMemory: stringArgDeprecated('profile-memory'),
); );
// If the test is sent a signal, take a screenshot before exiting
final Map<ProcessSignal, Object> screenshotTokens = _registerScreenshotCallbacks((ProcessSignal signal) async { // If the test is sent a signal or times out, take a screenshot
_logger.printError('Caught $signal'); _registerScreenshotCallbacks(device);
await _takeScreenshot(device);
});
final int testResult = await testResultFuture; final int testResult = await testResultFuture;
_unregisterScreenshotCallbacks(screenshotTokens);
if (timeoutTimer != null) {
timeoutTimer!.cancel();
}
_unregisterScreenshotCallbacks();
if (testResult != 0 && screenshot != null) { if (testResult != 0 && screenshot != null) {
// Take a screenshot while the app is still running. // Take a screenshot while the app is still running.
await _takeScreenshot(device); await _takeScreenshot(device);
...@@ -331,21 +342,59 @@ class DriveCommand extends RunCommandBase { ...@@ -331,21 +342,59 @@ class DriveCommand extends RunCommandBase {
return FlutterCommandResult.success(); return FlutterCommandResult.success();
} }
Map<ProcessSignal, Object> _registerScreenshotCallbacks(Function(ProcessSignal) callback) { int? get _timeoutSeconds {
final String? timeoutString = stringArg('timeout');
if (timeoutString == null) {
return null;
}
final int? timeoutSeconds = int.tryParse(timeoutString);
if (timeoutSeconds == null || timeoutSeconds <= 0) {
throwToolExit(
'Invalid value "$timeoutString" provided to the option --timeout: '
'expected a positive integer representing seconds.',
);
}
return timeoutSeconds;
}
void _registerScreenshotCallbacks(Device device) {
_logger.printTrace('Registering signal handlers...'); _logger.printTrace('Registering signal handlers...');
final Map<ProcessSignal, Object> tokens = <ProcessSignal, Object>{}; final Map<ProcessSignal, Object> tokens = <ProcessSignal, Object>{};
for (final ProcessSignal signal in signalsToHandle) { for (final ProcessSignal signal in signalsToHandle) {
tokens[signal] = signals.addHandler(signal, callback); tokens[signal] = signals.addHandler(
signal,
(ProcessSignal signal) {
_unregisterScreenshotCallbacks();
_logger.printError('Caught $signal');
return _takeScreenshot(device);
},
);
}
screenshotTokens = tokens;
final int? timeoutSeconds = _timeoutSeconds;
if (timeoutSeconds != null) {
timeoutTimer = Timer(
Duration(seconds: timeoutSeconds),
() {
_unregisterScreenshotCallbacks();
_takeScreenshot(device);
throwToolExit('Timed out after $timeoutSeconds seconds');
}
);
} }
return tokens;
} }
void _unregisterScreenshotCallbacks(Map<ProcessSignal, Object> tokens) { void _unregisterScreenshotCallbacks() {
if (screenshotTokens != null) {
_logger.printTrace('Unregistering signal handlers...'); _logger.printTrace('Unregistering signal handlers...');
for (final MapEntry<ProcessSignal, Object> entry in tokens.entries) { for (final MapEntry<ProcessSignal, Object> entry in screenshotTokens!.entries) {
signals.removeHandler(entry.key, entry.value); signals.removeHandler(entry.key, entry.value);
} }
} }
timeoutTimer?.cancel();
}
String? _getTestFile() { String? _getTestFile() {
if (argResults!['driver'] != null) { if (argResults!['driver'] != null) {
return stringArgDeprecated('driver'); return stringArgDeprecated('driver');
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
import 'dart:async'; import 'dart:async';
import 'dart:io' as io; import 'dart:io' as io;
import 'package:fake_async/fake_async.dart';
import 'package:file/memory.dart'; import 'package:file/memory.dart';
import 'package:flutter_tools/src/application_package.dart'; import 'package:flutter_tools/src/application_package.dart';
import 'package:flutter_tools/src/base/async_guard.dart';
import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
...@@ -203,6 +205,72 @@ void main() { ...@@ -203,6 +205,72 @@ void main() {
DeviceManager: () => fakeDeviceManager, DeviceManager: () => fakeDeviceManager,
}); });
testUsingContext('drive --timeout takes screenshot and tool exits after timeout', () async {
final DriveCommand command = DriveCommand(
fileSystem: fileSystem,
logger: logger,
platform: platform,
signals: Signals.test(),
flutterDriverFactory: NeverEndingFlutterDriverFactory(() {}),
);
fileSystem.file('lib/main.dart').createSync(recursive: true);
fileSystem.file('test_driver/main_test.dart').createSync(recursive: true);
fileSystem.file('pubspec.yaml').createSync();
fileSystem.directory('drive_screenshots').createSync();
final ScreenshotDevice screenshotDevice = ScreenshotDevice();
fakeDeviceManager.devices = <Device>[screenshotDevice];
expect(screenshotDevice.screenshots, isEmpty);
bool caughtToolExit = false;
FakeAsync().run<void>((FakeAsync time) {
// Because the tool exit will be thrown asynchronously by a [Timer],
// use [asyncGuard] to catch it
asyncGuard<void>(
() => createTestCommandRunner(command).run(
<String>[
'drive',
'--no-pub',
'-d',
screenshotDevice.id,
'--use-existing-app',
'http://localhost:8181',
'--screenshot',
'drive_screenshots',
'--timeout',
'300', // 5 minutes
],
),
onError: (Object error) {
expect(error, isA<ToolExit>());
expect(
(error as ToolExit).message,
contains('Timed out after 300 seconds'),
);
caughtToolExit = true;
}
);
time.elapse(const Duration(seconds: 299));
expect(screenshotDevice.screenshots, isEmpty);
time.elapse(const Duration(seconds: 2));
expect(
screenshotDevice.screenshots,
contains(isA<File>().having(
(File file) => file.path,
'path',
'drive_screenshots/drive_01.png',
)),
);
});
expect(caughtToolExit, isTrue);
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Pub: () => FakePub(),
DeviceManager: () => fakeDeviceManager,
});
testUsingContext('drive --screenshot takes screenshot if sent a registered signal', () async { testUsingContext('drive --screenshot takes screenshot if sent a registered signal', () async {
final FakeProcessSignal signal = FakeProcessSignal(); final FakeProcessSignal signal = FakeProcessSignal();
final ProcessSignal signalUnderTest = ProcessSignal(signal); final ProcessSignal signalUnderTest = ProcessSignal(signal);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment