firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Terminate the test device if the flutter tool is signal-killed. (#159115)

Browse Source 
Closes https://github.com/flutter/flutter/issues/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.
This commit is contained in:
Matan Lurey 2024-11-19 08:10:32 -08:00 committed by GitHub
parent 0e1c6330ea
commit c5379557b3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 81 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
packages/flutter_tools/lib/src/test/flutter_platform.dart
View File

@ -15,6 +15,7 @@ import '../base/async_guard.dart';
import '../base/common.dart';
import '../base/file_system.dart';
import '../base/io.dart';
import '../base/process.dart';
import '../build_info.dart';
import '../cache.dart';
import '../compile.dart';
@ -307,6 +308,7 @@ class FlutterPlatform extends PlatformPlugin {
this.testTimeRecorder,
this.nativeAssetsBuilder,
this.buildInfo,
this.shutdownHooks,
});
final String shellPath;
@ -325,6 +327,7 @@ class FlutterPlatform extends PlatformPlugin {
final TestTimeRecorder? testTimeRecorder;
final TestCompilerNativeAssetsBuilder? nativeAssetsBuilder;
final BuildInfo? buildInfo;
final ShutdownHooks? shutdownHooks;
/// The device to run the test on for Integration Tests.
///
@ -476,8 +479,35 @@ class FlutterPlatform extends PlatformPlugin {
_AsyncError? outOfBandError; // error that we couldn't send to the harness that we need to send via our future
final List<Finalizer> finalizers = <Finalizer>[]; // Will be run in reverse order.
// Will be run in reverse order.
final List<Finalizer> finalizers = <Finalizer>[];
bool ranFinalizers = false;
bool controllerSinkClosed = false;
Future<void> finalize() async {
if (ranFinalizers) {
return;
}
ranFinalizers = true;
globals.printTrace('test $ourTestCount: cleaning up...');
for (final Finalizer finalizer in finalizers.reversed) {
try {
await finalizer();
} on Exception catch (error, stack) {
globals.printTrace('test $ourTestCount: error while cleaning up; ${controllerSinkClosed ? "reporting to console" : "sending to test framework"}');
if (!controllerSinkClosed) {
testHarnessChannel.sink.addError(error, stack);
} else {
globals.printError('unhandled error during finalization of test:\n$testPath\n$error\n$stack');
outOfBandError ??= _AsyncError(error, stack);
}
}
}
}
// If the flutter CLI is forcibly terminated, cleanup processes.
final ShutdownHooks shutdownHooks = this.shutdownHooks ?? globals.shutdownHooks;
shutdownHooks.addShutdownHook(finalize);
try {
// Callback can't throw since it's just setting a variable.
unawaited(testHarnessChannel.sink.done.whenComplete(() {
@ -608,21 +638,7 @@ class FlutterPlatform extends PlatformPlugin {
outOfBandError ??= _AsyncError(reportedError, reportedStackTrace);
}
} finally {
globals.printTrace('test $ourTestCount: cleaning up...');
// Finalizers are treated like a stack; run them in reverse order.
for (final Finalizer finalizer in finalizers.reversed) {
try {
await finalizer();
} on Exception catch (error, stack) {
globals.printTrace('test $ourTestCount: error while cleaning up; ${controllerSinkClosed ? "reporting to console" : "sending to test framework"}');
if (!controllerSinkClosed) {
testHarnessChannel.sink.addError(error, stack);
} else {
globals.printError('unhandled error during finalization of test:\n$testPath\n$error\n$stack');
outOfBandError ??= _AsyncError(error, stack);
}
}
}
await finalize();
if (!controllerSinkClosed) {
// Waiting below with await.
unawaited(testHarnessChannel.sink.close());

49
packages/flutter_tools/test/general.shard/flutter_platform_test.dart
View File

@ -7,6 +7,7 @@ import 'package:flutter_tools/src/application_package.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/flutter_manifest.dart';
@ -97,6 +98,35 @@ void main() {
ApplicationPackageFactory: () => _FakeApplicationPackageFactory(),
});
testUsingContext('a shutdown signal terminates the test device', () async {
final _WorkingDevice testDevice = _WorkingDevice();
final ShutdownHooks shutdownHooks = ShutdownHooks();
final FlutterPlatform flutterPlatform = FlutterPlatform(
debuggingOptions: DebuggingOptions.disabled(BuildInfo.debug),
shellPath: '/',
enableVmService: false,
integrationTestDevice: testDevice,
flutterProject: _FakeFlutterProject(),
host: InternetAddress.anyIPv4,
updateGoldens: false,
shutdownHooks: shutdownHooks,
);
await expectLater(
() => flutterPlatform.loadChannel('test1.dart', fakeSuitePlatform).stream.drain<void>(),
returnsNormally,
);
final BufferLogger logger = globals.logger as BufferLogger;
await shutdownHooks.runShutdownHooks(logger);
expect(logger.traceText, contains('test 0: ensuring test device is terminated.'));
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
ApplicationPackageFactory: () => _FakeApplicationPackageFactory(),
});
testUsingContext('installHook creates a FlutterPlatform', () {
expect(() => installHook(
shellPath: 'abc',
@ -214,6 +244,25 @@ class _UnstartableDevice extends Fake implements Device {
}
}
class _WorkingDevice extends Fake implements Device {
@override
Future<void> dispose() async {}
@override
Future<TargetPlatform> get targetPlatform => Future<TargetPlatform>.value(TargetPlatform.android);
@override
Future<bool> stopApp(ApplicationPackage? app, {String? userIdentifier}) async => true;
@override
Future<bool> uninstallApp(ApplicationPackage app, {String? userIdentifier}) async => true;
@override
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 {
return LaunchResult.succeeded(vmServiceUri: Uri.parse('http://127.0.0.1:12345/vmService'));
}
}
class _FakeFlutterProject extends Fake implements FlutterProject {
@override
FlutterManifest get manifest => FlutterManifest.empty(logger: BufferLogger.test());