Unverified Commit 459c7fb8 authored by Danny Tuppeny's avatar Danny Tuppeny Committed by GitHub

Resume isolate before terminating tests to prevent flutter_tester leaks in...

Resume isolate before terminating tests to prevent flutter_tester leaks in integration tests (#45248)

* Resume isolate before terminating tests to prevent flutter_tester leak

* Fix lint

* Catch exceptions from resume() as well as timeout()

* Formatting fixes

* Don't try to resume if there's no vm service

* Fix handling of timeouts to not leak futures
parent dc68d570
...@@ -164,6 +164,12 @@ abstract class FlutterTestDriver { ...@@ -164,6 +164,12 @@ abstract class FlutterTestDriver {
if (_processPid == null) { if (_processPid == null) {
return -1; return -1;
} }
// If we try to kill the process while it's paused, we'll end up terminating
// it forcefully and it won't terminate child processes, so we need to ensure
// it's running before terminating.
await resume().timeout(defaultTimeout)
.catchError((Object e) => _debugPrint('Ignoring failure to resume during shutdown'));
_debugPrint('Sending SIGTERM to $_processPid..'); _debugPrint('Sending SIGTERM to $_processPid..');
ProcessSignal.SIGTERM.send(_processPid); ProcessSignal.SIGTERM.send(_processPid);
return _process.exitCode.timeout(quitTimeout, onTimeout: _killForcefully); return _process.exitCode.timeout(quitTimeout, onTimeout: _killForcefully);
...@@ -345,7 +351,7 @@ abstract class FlutterTestDriver { ...@@ -345,7 +351,7 @@ abstract class FlutterTestDriver {
return; return;
} }
if ((event != null && json['event'] == event) || if ((event != null && json['event'] == event) ||
(id != null && json['id'] == id)) { (id != null && json['id'] == id)) {
await subscription.cancel(); await subscription.cancel();
_debugPrint('OK ($interestingOccurrence)'); _debugPrint('OK ($interestingOccurrence)');
response.complete(json); response.complete(json);
...@@ -380,10 +386,8 @@ abstract class FlutterTestDriver { ...@@ -380,10 +386,8 @@ abstract class FlutterTestDriver {
if (_printDebugOutputToStdOut) { if (_printDebugOutputToStdOut) {
_debugPrint('$task...'); _debugPrint('$task...');
return callback()..timeout(timeout, onTimeout: () { final Timer longWarning = Timer(timeout, () => _debugPrint('$task is taking longer than usual...'));
_debugPrint('$task is taking longer than usual...'); return callback().whenComplete(longWarning.cancel);
return null;
});
} }
// We're not showing all output to the screen, so let's capture the output // We're not showing all output to the screen, so let's capture the output
...@@ -399,14 +403,12 @@ abstract class FlutterTestDriver { ...@@ -399,14 +403,12 @@ abstract class FlutterTestDriver {
} }
final StreamSubscription<String> subscription = _allMessages.stream.listen(logMessage); final StreamSubscription<String> subscription = _allMessages.stream.listen(logMessage);
final Future<T> future = callback(); final Timer longWarning = Timer(timeout, () {
future.timeout(timeout ?? defaultTimeout, onTimeout: () {
_debugPrint(messages.toString()); _debugPrint(messages.toString());
timeoutExpired = true; timeoutExpired = true;
_debugPrint('$task is taking longer than usual...'); _debugPrint('$task is taking longer than usual...');
return null;
}); });
final Future<T> future = callback().whenComplete(longWarning.cancel);
return future.catchError((dynamic error) { return future.catchError((dynamic error) {
if (!timeoutExpired) { if (!timeoutExpired) {
...@@ -525,7 +527,7 @@ class FlutterRunTestDriver extends FlutterTestDriver { ...@@ -525,7 +527,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
// have already completed. // have already completed.
_currentRunningAppId = (await started)['params']['appId'] as String; _currentRunningAppId = (await started)['params']['appId'] as String;
prematureExitGuard.complete(); prematureExitGuard.complete();
} catch(error, stackTrace) { } catch (error, stackTrace) {
prematureExitGuard.completeError(error, stackTrace); prematureExitGuard.completeError(error, stackTrace);
} }
}()); }());
...@@ -546,7 +548,7 @@ class FlutterRunTestDriver extends FlutterTestDriver { ...@@ -546,7 +548,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
'app.restart', 'app.restart',
<String, dynamic>{'appId': _currentRunningAppId, 'fullRestart': fullRestart, 'pause': pause}, <String, dynamic>{'appId': _currentRunningAppId, 'fullRestart': fullRestart, 'pause': pause},
); );
_debugPrint('${ fullRestart ? "Hot restart" : "Hot reload" } complete.'); _debugPrint('${fullRestart ? "Hot restart" : "Hot reload"} complete.');
if (hotReloadResponse == null || hotReloadResponse['code'] != 0) { if (hotReloadResponse == null || hotReloadResponse['code'] != 0) {
_throwErrorResponse('Hot ${fullRestart ? 'restart' : 'reload'} request failed'); _throwErrorResponse('Hot ${fullRestart ? 'restart' : 'reload'} request failed');
......
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