Skip to content

Commit c537955

Browse files
authored
Terminate the test device if the flutter tool is signal-killed. (#159115)
Closes flutter/flutter#20949. Signals (such as SIGTERM or SIGKILL) end up flowing through `exitWithHooks`, which in turn, after running hooks, call `exit().` That means, as a result, any `try { } finally { }` guarded execution may _not_ run, which happens to also be how `flutter_tester` instances are cleaned up if they have not terminated. This PR adds in-progress `flutter_tester` runs (or any platform `flutter_platform` supports) to the shutdown hooks, guaranteeing that the finalizers (which in turn, kill the process) are _always_ executed as long as either the test completes, _or_ `exitWithHooks` is called. The existing integration tests (`integration.shard/test_test.dart`) still pass as well.
1 parent 0e1c633 commit c537955

File tree

2 files changed

+81
-16
lines changed

2 files changed

+81
-16
lines changed

packages/flutter_tools/lib/src/test/flutter_platform.dart

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import '../base/async_guard.dart';
1515
import '../base/common.dart';
1616
import '../base/file_system.dart';
1717
import '../base/io.dart';
18+
import '../base/process.dart';
1819
import '../build_info.dart';
1920
import '../cache.dart';
2021
import '../compile.dart';
@@ -307,6 +308,7 @@ class FlutterPlatform extends PlatformPlugin {
307308
this.testTimeRecorder,
308309
this.nativeAssetsBuilder,
309310
this.buildInfo,
311+
this.shutdownHooks,
310312
});
311313

312314
final String shellPath;
@@ -325,6 +327,7 @@ class FlutterPlatform extends PlatformPlugin {
325327
final TestTimeRecorder? testTimeRecorder;
326328
final TestCompilerNativeAssetsBuilder? nativeAssetsBuilder;
327329
final BuildInfo? buildInfo;
330+
final ShutdownHooks? shutdownHooks;
328331

329332
/// The device to run the test on for Integration Tests.
330333
///
@@ -476,8 +479,35 @@ class FlutterPlatform extends PlatformPlugin {
476479

477480
_AsyncError? outOfBandError; // error that we couldn't send to the harness that we need to send via our future
478481

479-
final List<Finalizer> finalizers = <Finalizer>[]; // Will be run in reverse order.
482+
// Will be run in reverse order.
483+
final List<Finalizer> finalizers = <Finalizer>[];
484+
bool ranFinalizers = false;
480485
bool controllerSinkClosed = false;
486+
Future<void> finalize() async {
487+
if (ranFinalizers) {
488+
return;
489+
}
490+
ranFinalizers = true;
491+
globals.printTrace('test $ourTestCount: cleaning up...');
492+
for (final Finalizer finalizer in finalizers.reversed) {
493+
try {
494+
await finalizer();
495+
} on Exception catch (error, stack) {
496+
globals.printTrace('test $ourTestCount: error while cleaning up; ${controllerSinkClosed ? "reporting to console" : "sending to test framework"}');
497+
if (!controllerSinkClosed) {
498+
testHarnessChannel.sink.addError(error, stack);
499+
} else {
500+
globals.printError('unhandled error during finalization of test:\n$testPath\n$error\n$stack');
501+
outOfBandError ??= _AsyncError(error, stack);
502+
}
503+
}
504+
}
505+
}
506+
507+
// If the flutter CLI is forcibly terminated, cleanup processes.
508+
final ShutdownHooks shutdownHooks = this.shutdownHooks ?? globals.shutdownHooks;
509+
shutdownHooks.addShutdownHook(finalize);
510+
481511
try {
482512
// Callback can't throw since it's just setting a variable.
483513
unawaited(testHarnessChannel.sink.done.whenComplete(() {
@@ -608,21 +638,7 @@ class FlutterPlatform extends PlatformPlugin {
608638
outOfBandError ??= _AsyncError(reportedError, reportedStackTrace);
609639
}
610640
} finally {
611-
globals.printTrace('test $ourTestCount: cleaning up...');
612-
// Finalizers are treated like a stack; run them in reverse order.
613-
for (final Finalizer finalizer in finalizers.reversed) {
614-
try {
615-
await finalizer();
616-
} on Exception catch (error, stack) {
617-
globals.printTrace('test $ourTestCount: error while cleaning up; ${controllerSinkClosed ? "reporting to console" : "sending to test framework"}');
618-
if (!controllerSinkClosed) {
619-
testHarnessChannel.sink.addError(error, stack);
620-
} else {
621-
globals.printError('unhandled error during finalization of test:\n$testPath\n$error\n$stack');
622-
outOfBandError ??= _AsyncError(error, stack);
623-
}
624-
}
625-
}
641+
await finalize();
626642
if (!controllerSinkClosed) {
627643
// Waiting below with await.
628644
unawaited(testHarnessChannel.sink.close());

packages/flutter_tools/test/general.shard/flutter_platform_test.dart

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter_tools/src/application_package.dart';
77
import 'package:flutter_tools/src/base/file_system.dart';
88
import 'package:flutter_tools/src/base/io.dart';
99
import 'package:flutter_tools/src/base/logger.dart';
10+
import 'package:flutter_tools/src/base/process.dart';
1011
import 'package:flutter_tools/src/build_info.dart';
1112
import 'package:flutter_tools/src/device.dart';
1213
import 'package:flutter_tools/src/flutter_manifest.dart';
@@ -97,6 +98,35 @@ void main() {
9798
ApplicationPackageFactory: () => _FakeApplicationPackageFactory(),
9899
});
99100

101+
testUsingContext('a shutdown signal terminates the test device', () async {
102+
final _WorkingDevice testDevice = _WorkingDevice();
103+
104+
final ShutdownHooks shutdownHooks = ShutdownHooks();
105+
final FlutterPlatform flutterPlatform = FlutterPlatform(
106+
debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
107+
shellPath: '/',
108+
enableVmService: false,
109+
integrationTestDevice: testDevice,
110+
flutterProject: _FakeFlutterProject(),
111+
host: InternetAddress.anyIPv4,
112+
updateGoldens: false,
113+
shutdownHooks: shutdownHooks,
114+
);
115+
116+
await expectLater(
117+
() => flutterPlatform.loadChannel('test1.dart', fakeSuitePlatform).stream.drain<void>(),
118+
returnsNormally,
119+
);
120+
121+
final BufferLogger logger = globals.logger as BufferLogger;
122+
await shutdownHooks.runShutdownHooks(logger);
123+
expect(logger.traceText, contains('test 0: ensuring test device is terminated.'));
124+
}, overrides: <Type, Generator>{
125+
FileSystem: () => fileSystem,
126+
ProcessManager: () => FakeProcessManager.any(),
127+
ApplicationPackageFactory: () => _FakeApplicationPackageFactory(),
128+
});
129+
100130
testUsingContext('installHook creates a FlutterPlatform', () {
101131
expect(() => installHook(
102132
shellPath: 'abc',
@@ -214,6 +244,25 @@ class _UnstartableDevice extends Fake implements Device {
214244
}
215245
}
216246

247+
class _WorkingDevice extends Fake implements Device {
248+
@override
249+
Future<void> dispose() async {}
250+
251+
@override
252+
Future<TargetPlatform> get targetPlatform => Future<TargetPlatform>.value(TargetPlatform.android);
253+
254+
@override
255+
Future<bool> stopApp(ApplicationPackage? app, {String? userIdentifier}) async => true;
256+
257+
@override
258+
Future<bool> uninstallApp(ApplicationPackage app, {String? userIdentifier}) async => true;
259+
260+
@override
261+
Future<LaunchResult> startApp(covariant ApplicationPackage? package, {String? mainPath, String? route, required DebuggingOptions debuggingOptions, Map<String, Object?> platformArgs = const <String, Object>{}, bool prebuiltApplication = false, String? userIdentifier}) async {
262+
return LaunchResult.succeeded(vmServiceUri: Uri.parse('http://127.0.0.1:12345/vmService'));
263+
}
264+
}
265+
217266
class _FakeFlutterProject extends Fake implements FlutterProject {
218267
@override
219268
FlutterManifest get manifest => FlutterManifest.empty(logger: BufferLogger.test());

0 commit comments

Comments
 (0)