Unverified Commit ccd4f6dd authored by Brian Eaton's avatar Brian Eaton Committed by GitHub

Reland "Make sure all isolates start during flutter driver tests" (#64432)

parent 1b46023e
......@@ -37,8 +37,21 @@ class VMServiceFlutterDriver extends FlutterDriver {
bool logCommunicationToFile = true,
}) : _printCommunication = printCommunication,
_logCommunicationToFile = logCommunicationToFile,
_isolateSubscription = _initIsolateHandler(_serviceClient),
_driverId = _nextDriverId++;
// The Dart VM may be running with --pause-isolates-on-start.
// Set a listener to unpause new isolates as they are ready to run,
// otherwise they'll hang indefinitely.
static StreamSubscription<VMIsolateRef> _initIsolateHandler(
VMServiceClient serviceClient) {
return serviceClient.onIsolateRunnable.listen(
(VMIsolateRef isolateRef) async {
await _resumeLeniently(isolateRef);
}
);
}
/// Connects to a Flutter application.
///
/// See [FlutterDriver.connect] for more documentation.
......@@ -100,9 +113,12 @@ class VMServiceFlutterDriver extends FlutterDriver {
null ? vm.isolates.first :
vm.isolates.firstWhere(
(VMIsolateRef isolate) => isolate.number == isolateNumber);
_log('Isolate found with number: ${isolateRef.number}');
_log('Isolate found with number: ${isolateRef.number}');
// There is a race condition here, the isolate may have been destroyed before
// we get around to loading it.
VMIsolate isolate = await isolateRef.loadRunnable();
_log('Loaded isolate: ${isolate.number}');
// TODO(yjbanov): vm_service_client does not support "None" pause event yet.
// It is currently reported as null, but we cannot rely on it because
......@@ -117,9 +133,22 @@ class VMServiceFlutterDriver extends FlutterDriver {
isolate.pauseEvent is! VMPauseExceptionEvent &&
isolate.pauseEvent is! VMPauseInterruptedEvent &&
isolate.pauseEvent is! VMResumeEvent) {
_log('Reloading first isolate.');
isolate = await isolateRef.loadRunnable();
_log('Reloaded first isolate.');
}
// Tells the Dart VM Service to notify us about "Isolate" events.
//
// This is a workaround for an issue in package:vm_service_client, which
// subscribes to the "Isolate" stream lazily upon subscription, which
// results in lost events.
//
// Details: https://github.com/dart-lang/vm_service_client/issues/17
await connection.peer.sendRequest('streamListen', <String, String>{
'streamId': 'Isolate',
});
final VMServiceFlutterDriver driver = VMServiceFlutterDriver.connectedTo(
client, connection.peer, isolate,
printCommunication: printCommunication,
......@@ -128,27 +157,6 @@ class VMServiceFlutterDriver extends FlutterDriver {
driver._dartVmReconnectUrl = dartVmServiceUrl;
// Attempts to resume the isolate, but does not crash if it fails because
// the isolate is already resumed. There could be a race with other tools,
// such as a debugger, any of which could have resumed the isolate.
Future<dynamic> resumeLeniently() {
_log('Attempting to resume isolate');
return isolate.resume().catchError((dynamic e) {
const int vmMustBePausedCode = 101;
if (e is rpc.RpcException && e.code == vmMustBePausedCode) {
// No biggie; something else must have resumed the isolate
_log(
'Attempted to resume an already resumed isolate. This may happen '
'when we lose a race with another tool (usually a debugger) that '
'is connected to the same isolate.'
);
} else {
// Failed to resume due to another reason. Fail hard.
throw e;
}
});
}
/// Waits for a signal from the VM service that the extension is registered.
///
/// Looks at the list of loaded extensions for the current [isolateRef], as
......@@ -183,42 +191,8 @@ class VMServiceFlutterDriver extends FlutterDriver {
]);
}
/// Tells the Dart VM Service to notify us about "Isolate" events.
///
/// This is a workaround for an issue in package:vm_service_client, which
/// subscribes to the "Isolate" stream lazily upon subscription, which
/// results in lost events.
///
/// Details: https://github.com/dart-lang/vm_service_client/issues/17
Future<void> enableIsolateStreams() async {
await connection.peer.sendRequest('streamListen', <String, String>{
'streamId': 'Isolate',
});
}
// Attempt to resume isolate if it was paused
if (isolate.pauseEvent is VMPauseStartEvent) {
_log('Isolate is paused at start.');
await resumeLeniently();
} else if (isolate.pauseEvent is VMPauseExitEvent ||
isolate.pauseEvent is VMPauseBreakpointEvent ||
isolate.pauseEvent is VMPauseExceptionEvent ||
isolate.pauseEvent is VMPauseInterruptedEvent) {
// If the isolate is paused for any other reason, assume the extension is
// already there.
_log('Isolate is paused mid-flight.');
await resumeLeniently();
} else if (isolate.pauseEvent is VMResumeEvent) {
_log('Isolate is not paused. Assuming application is ready.');
} else {
_log(
'Unknown pause event type ${isolate.pauseEvent.runtimeType}. '
'Assuming application is ready.'
);
}
await enableIsolateStreams();
await _resumeLeniently(isolate);
// We will never receive the extension event if the user does not register
// it. If that happens, show a message but continue waiting.
......@@ -241,6 +215,58 @@ class VMServiceFlutterDriver extends FlutterDriver {
return driver;
}
static bool _alreadyRunning(VMIsolate isolate) {
// Expected pause events.
if (isolate.pauseEvent is VMPauseStartEvent ||
isolate.pauseEvent is VMPauseExitEvent ||
isolate.pauseEvent is VMPauseBreakpointEvent ||
isolate.pauseEvent is VMPauseExceptionEvent ||
isolate.pauseEvent is VMPauseInterruptedEvent ||
isolate.pauseEvent is VMNoneEvent) {
return false;
}
// Already running.
if (isolate.pauseEvent is VMResumeEvent) {
_log('Isolate is not paused. Assuming application is ready.');
return true;
}
_log('Unknown pause event type ${isolate.pauseEvent.runtimeType}. '
'Assuming application is ready.');
return true;
}
/// Attempts to resume the isolate, but does not crash if it fails because
/// the isolate is already resumed. There could be a race with other tools,
/// such as a debugger, any of which could have resumed the isolate.
/// Returns the isolate if it is runnable, null otherwise.
static Future<void> _resumeLeniently(VMIsolateRef isolateRef) async {
try {
_log('New runnable isolate ${isolateRef.number}');
final VMIsolate isolate = await isolateRef.load();
if (_alreadyRunning(isolate)) {
return;
}
_log('Attempting to resume isolate ${isolate.number}');
await isolate.resume();
_log('Resumed isolate ${isolate.number}');
} on VMSentinelException {
// Probably OK, the isolate vanished before we got to it.
_log('Failed to load isolate, proceeding.');
} on rpc.RpcException catch (e) {
const int vmMustBePausedCode = 101;
if (e.code == vmMustBePausedCode) {
// No biggie; something else must have resumed the isolate
_log('Attempted to resume an already resumed isolate. This may happen '
'when we lose a race with another tool (usually a debugger) that '
'is connected to the same isolate.'
);
} else {
// Failed to resume due to another reason. Fail hard.
rethrow;
}
}
}
static int _nextDriverId = 0;
static const String _flutterExtensionMethodName = 'ext.flutter.driver';
......@@ -275,6 +301,9 @@ class VMServiceFlutterDriver extends FlutterDriver {
/// would like to instrument.
final VMServiceClient _serviceClient;
/// Subscription to isolate events happening in the app.
final StreamSubscription<VMIsolateRef> _isolateSubscription;
/// JSON-RPC client useful for sending raw JSON requests.
rpc.Peer _peer;
......@@ -552,6 +581,7 @@ class VMServiceFlutterDriver extends FlutterDriver {
@override
Future<void> close() async {
// Don't leak vm_service_client-specific objects, if any
await _isolateSubscription.cancel();
await _serviceClient.close();
await _peer.close();
}
......
......@@ -35,6 +35,7 @@ void main() {
MockVM mockVM;
MockIsolate mockIsolate;
MockPeer mockPeer;
MockIsolate otherIsolate;
void expectLogContains(String message) {
expect(log, anyElement(contains(message)));
......@@ -45,10 +46,15 @@ void main() {
mockClient = MockVMServiceClient();
mockVM = MockVM();
mockIsolate = MockIsolate();
otherIsolate = MockIsolate();
mockPeer = MockPeer();
when(mockClient.getVM()).thenAnswer((_) => Future<MockVM>.value(mockVM));
when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) {
return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[otherIsolate]);
});
when(mockVM.isolates).thenReturn(<VMRunnableIsolate>[mockIsolate]);
when(mockIsolate.loadRunnable()).thenAnswer((_) => Future<MockIsolate>.value(mockIsolate));
when(mockIsolate.load()).thenAnswer((_) => Future<MockIsolate>.value(mockIsolate));
when(mockIsolate.extensionRpcs).thenReturn(<String>[]);
when(mockIsolate.onExtensionAdded).thenAnswer((Invocation invocation) {
return Stream<String>.fromIterable(<String>['ext.flutter.driver']);
......@@ -60,6 +66,10 @@ void main() {
VMServiceClientConnection(mockClient, mockPeer)
);
};
when(otherIsolate.load()).thenAnswer((_) => Future<MockIsolate>.value(otherIsolate));
when(otherIsolate.resume()).thenAnswer((Invocation invocation) {
return Future<dynamic>.value(null);
});
});
tearDown(() async {
......@@ -77,15 +87,20 @@ void main() {
connectionLog.add('resume');
return Future<dynamic>.value(null);
});
when(otherIsolate.pauseEvent).thenReturn(MockVMPauseStartEvent());
when(mockIsolate.onExtensionAdded).thenAnswer((Invocation invocation) {
connectionLog.add('onExtensionAdded');
return Stream<String>.fromIterable(<String>['ext.flutter.driver']);
});
when(otherIsolate.resume()).thenAnswer((Invocation invocation) {
connectionLog.add('other-resume');
return Future<dynamic>.value(null);
});
final FlutterDriver driver = await FlutterDriver.connect(dartVmServiceUrl: '');
expect(driver, isNotNull);
expectLogContains('Isolate is paused at start');
expect(connectionLog, <String>['resume', 'streamListen', 'onExtensionAdded']);
expectLogContains('Attempting to resume isolate');
expect(connectionLog, <String>['streamListen', 'resume', 'other-resume', 'onExtensionAdded']);
});
test('connects to isolate paused mid-flight', () async {
......@@ -94,7 +109,7 @@ void main() {
final FlutterDriver driver = await FlutterDriver.connect(dartVmServiceUrl: '');
expect(driver, isNotNull);
expectLogContains('Isolate is paused mid-flight');
expectLogContains('Attempting to resume isolate');
});
// This test simulates a situation when we believe that the isolate is
......@@ -160,6 +175,9 @@ void main() {
mockClient = MockVMServiceClient();
mockPeer = MockPeer();
mockIsolate = MockIsolate();
when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) {
return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[]);
});
driver = VMServiceFlutterDriver.connectedTo(mockClient, mockPeer, mockIsolate);
});
......@@ -790,6 +808,9 @@ void main() {
mockClient = MockVMServiceClient();
mockPeer = MockPeer();
mockIsolate = MockIsolate();
when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) {
return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[]);
});
driver = VMServiceFlutterDriver.connectedTo(mockClient, mockPeer, mockIsolate);
});
......
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