Unverified Commit cd239599 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Attempt to mitigate flakiness around devtools (#94879)

parent 7ddd9984
...@@ -853,19 +853,7 @@ Future<void> _runFrameworkTests() async { ...@@ -853,19 +853,7 @@ Future<void> _runFrameworkTests() async {
if (Directory(pubCache).existsSync()) { if (Directory(pubCache).existsSync()) {
pubEnvironment['PUB_CACHE'] = pubCache; pubEnvironment['PUB_CACHE'] = pubCache;
} }
recompileFlutterToolWithAsserts(pubEnvironment);
// If an existing env variable exists append to it, but only if
// it doesn't appear to already include enable-asserts.
String toolsArgs = Platform.environment['FLUTTER_TOOL_ARGS'] ?? '';
if (!toolsArgs.contains('--enable-asserts')) {
toolsArgs += ' --enable-asserts';
}
pubEnvironment['FLUTTER_TOOL_ARGS'] = toolsArgs.trim();
// The flutter_tool will originally have been snapshotted without asserts.
// We need to force it to be regenerated with them enabled.
deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.snapshot'));
deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.stamp'));
await runCommand( await runCommand(
pub, pub,
args, args,
...@@ -1536,6 +1524,10 @@ Future<void> _runWebDebugTest(String target, { ...@@ -1536,6 +1524,10 @@ Future<void> _runWebDebugTest(String target, {
}) async { }) async {
final String testAppDirectory = path.join(flutterRoot, 'dev', 'integration_tests', 'web'); final String testAppDirectory = path.join(flutterRoot, 'dev', 'integration_tests', 'web');
bool success = false; bool success = false;
final Map<String, String> environment = <String, String>{
'FLUTTER_WEB': 'true',
};
recompileFlutterToolWithAsserts(environment);
final CommandResult result = await runCommand( final CommandResult result = await runCommand(
flutter, flutter,
<String>[ <String>[
...@@ -1565,9 +1557,7 @@ Future<void> _runWebDebugTest(String target, { ...@@ -1565,9 +1557,7 @@ Future<void> _runWebDebugTest(String target, {
} }
}, },
workingDirectory: testAppDirectory, workingDirectory: testAppDirectory,
environment: <String, String>{ environment: environment,
'FLUTTER_WEB': 'true',
},
); );
if (success) { if (success) {
...@@ -1663,16 +1653,7 @@ Future<void> _pubRunTest(String workingDirectory, { ...@@ -1663,16 +1653,7 @@ Future<void> _pubRunTest(String workingDirectory, {
pubEnvironment['PUB_CACHE'] = pubCache; pubEnvironment['PUB_CACHE'] = pubCache;
} }
if (enableFlutterToolAsserts) { if (enableFlutterToolAsserts) {
// If an existing env variable exists append to it, but only if recompileFlutterToolWithAsserts(pubEnvironment);
// it doesn't appear to already include enable-asserts.
String toolsArgs = Platform.environment['FLUTTER_TOOL_ARGS'] ?? '';
if (!toolsArgs.contains('--enable-asserts'))
toolsArgs += ' --enable-asserts';
pubEnvironment['FLUTTER_TOOL_ARGS'] = toolsArgs.trim();
// The flutter_tool will originally have been snapshotted without asserts.
// We need to force it to be regenerated with them enabled.
deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.snapshot'));
deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.stamp'));
} }
if (ensurePrecompiledTool) { if (ensurePrecompiledTool) {
// We rerun the `flutter` tool here just to make sure that it is compiled // We rerun the `flutter` tool here just to make sure that it is compiled
...@@ -1800,6 +1781,22 @@ Future<void> _runFlutterTest(String workingDirectory, { ...@@ -1800,6 +1781,22 @@ Future<void> _runFlutterTest(String workingDirectory, {
} }
} }
/// This will force the next run of the Flutter tool (if it uses the provided environment) to
/// have asserts enabled, by setting an environment variable and deleting the cache.
void recompileFlutterToolWithAsserts(Map<String, String> pubEnvironment) {
// If an existing env variable exists append to it, but only if
// it doesn't appear to already include enable-asserts.
String toolsArgs = Platform.environment['FLUTTER_TOOL_ARGS'] ?? '';
if (!toolsArgs.contains('--enable-asserts')) {
toolsArgs += ' --enable-asserts';
}
pubEnvironment['FLUTTER_TOOL_ARGS'] = toolsArgs.trim();
// The flutter_tool will originally have been snapshotted without asserts.
// We need to force it to be regenerated with them enabled.
deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.snapshot'));
deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.stamp'));
}
Map<String, String> _initGradleEnvironment() { Map<String, String> _initGradleEnvironment() {
final String? androidSdkRoot = (Platform.environment['ANDROID_HOME']?.isEmpty ?? true) final String? androidSdkRoot = (Platform.environment['ANDROID_HOME']?.isEmpty ?? true)
? Platform.environment['ANDROID_SDK_ROOT'] ? Platform.environment['ANDROID_SDK_ROOT']
......
...@@ -60,7 +60,10 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler { ...@@ -60,7 +60,10 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
bool launchedInBrowser = false; bool launchedInBrowser = false;
@override @override
DevToolsServerAddress get activeDevToolsServer => _devToolsLauncher?.activeDevToolsServer; DevToolsServerAddress get activeDevToolsServer {
assert(!_readyToAnnounce || _devToolsLauncher?.activeDevToolsServer != null);
return _devToolsLauncher?.activeDevToolsServer;
}
@override @override
bool get readyToAnnounce => _readyToAnnounce; bool get readyToAnnounce => _readyToAnnounce;
...@@ -72,6 +75,7 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler { ...@@ -72,6 +75,7 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
Uri devToolsServerAddress, Uri devToolsServerAddress,
@required List<FlutterDevice> flutterDevices, @required List<FlutterDevice> flutterDevices,
}) async { }) async {
assert(!_readyToAnnounce);
if (!_residentRunner.supportsServiceProtocol || _devToolsLauncher == null) { if (!_residentRunner.supportsServiceProtocol || _devToolsLauncher == null) {
return; return;
} }
...@@ -82,14 +86,20 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler { ...@@ -82,14 +86,20 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
_served = true; _served = true;
} }
await _devToolsLauncher.ready; await _devToolsLauncher.ready;
// Do not attempt to print debugger list if the connection has failed. // Do not attempt to print debugger list if the connection has failed or if we're shutting down.
if (_devToolsLauncher.activeDevToolsServer == null) { if (_devToolsLauncher.activeDevToolsServer == null || _shutdown) {
assert(!_readyToAnnounce);
return; return;
} }
final List<FlutterDevice> devicesWithExtension = await _devicesWithExtensions(flutterDevices); final List<FlutterDevice> devicesWithExtension = await _devicesWithExtensions(flutterDevices);
await _maybeCallDevToolsUriServiceExtension(devicesWithExtension); await _maybeCallDevToolsUriServiceExtension(devicesWithExtension);
await _callConnectedVmServiceUriExtension(devicesWithExtension); await _callConnectedVmServiceUriExtension(devicesWithExtension);
if (_shutdown) {
// If we're shutting down, no point reporting the debugger list.
return;
}
_readyToAnnounce = true; _readyToAnnounce = true;
assert(_devToolsLauncher.activeDevToolsServer != null);
if (_residentRunner.reportedDebuggers) { if (_residentRunner.reportedDebuggers) {
// Since the DevTools only just became available, we haven't had a chance to // Since the DevTools only just became available, we haven't had a chance to
// report their URLs yet. Do so now. // report their URLs yet. Do so now.
...@@ -248,6 +258,7 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler { ...@@ -248,6 +258,7 @@ class FlutterResidentDevtoolsHandler implements ResidentDevtoolsHandler {
return; return;
} }
_shutdown = true; _shutdown = true;
_readyToAnnounce = false;
await _devToolsLauncher.close(); await _devToolsLauncher.close();
} }
} }
......
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