From ad5133b3735a3b613dc3a1463d23ea624f83ebee Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Fri, 29 May 2026 13:48:45 -0700 Subject: [PATCH] [DDS] Fix server_connection_vm_service_test timeouts In `server_connection_common.dart`, the `server removes clients that disconnect from the API` test spawned its own Chrome instance using `package:devtools_shared`'s `Chrome` class without isolated profiles or essential headless flags. This caused the test to hang or fail flakily in container environments (like LUCI bots) and local environments: 1. Without `--use-mock-keychain`, headless Chrome on macOS blocks on system credential dialogs. 2. Without `--no-sandbox`, Chrome renderer processes can crash in restricted container environments. 3. Without `--user-data-dir`, Chrome uses the default system profile, which can cause it to attach to an existing open Chrome instance instead of starting a new one, meaning the process exits immediately and the test cannot terminate it. Fixed by directly using `package:browser_launcher`'s `Chrome` class in the test and passing: * `--user-data-dir` pointing to a unique temporary directory. * `--no-first-run` and `--no-default-browser-check` to bypass welcome prompts. * `--no-sandbox` and `--use-mock-keychain` where appropriate. Also wrapped the test in `try-finally` to guarantee cleanup of the temporary profile directory. Change-Id: I6fbe5a280524b57c635ab11ef54fa07dba2794cf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507600 Reviewed-by: Alexander Aprelev Auto-Submit: Ben Konyi Commit-Queue: Ben Konyi --- .../server_connection_common.dart | 66 +++++++++++++------ 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/pkg/dds/test/devtools_server/server_connection_common.dart b/pkg/dds/test/devtools_server/server_connection_common.dart index a7a7da27f47..a153334fee7 100644 --- a/pkg/dds/test/devtools_server/server_connection_common.dart +++ b/pkg/dds/test/devtools_server/server_connection_common.dart @@ -3,8 +3,10 @@ // found in the LICENSE file. import 'dart:convert'; +import 'dart:io'; -import 'package:devtools_shared/devtools_test_utils.dart'; +import 'package:browser_launcher/browser_launcher.dart'; +import 'package:devtools_shared/devtools_test_utils.dart' hide Chrome; import 'package:test/test.dart'; import 'utils/server_driver.dart'; @@ -84,36 +86,60 @@ void runTest({required bool useVmService}) { // Spawn our own Chrome process so we can terminate it. final devToolsUri = 'http://${event['params']['host']}:${event['params']['port']}'; - final chrome = await Chrome.locate()!.start(url: devToolsUri); + + // Create a temporary directory for an isolated user profile. + final tempDir = Directory.systemTemp.createTempSync('devtools_chrome_profile'); + + final chromeProcess = await Chrome.start([devToolsUri], args: [ + '--user-data-dir=${tempDir.path}', // Ensures process isolation + '--no-first-run', // Prevents welcome dialogs + '--no-default-browser-check', // Prevents default browser prompts + if (useChromeHeadless && headlessModeIsSupported) ...[ + '--headless', + '--disable-gpu', + '--no-sandbox', + ], + if (Platform.isMacOS) '--use-mock-keychain', + ]); + final stdoutSub = - chrome.process.stdout.transform(utf8.decoder).listen((e) { + chromeProcess.stdout.transform(utf8.decoder).listen((e) { print('[CHROME STDOUT]: $e'); }); final stderrSub = - chrome.process.stderr.transform(utf8.decoder).listen((e) { + chromeProcess.stderr.transform(utf8.decoder).listen((e) { print('[CHROME STDERR]: $e'); }); - // Wait for DevTools to inform server that it's connected. - print('Waiting for clients...'); - await testController.waitForClients(); + try { + // Wait for DevTools to inform server that it's connected. + print('Waiting for clients...'); + await testController.waitForClients(); - // Close the browser, which will disconnect DevTools SSE connection - // back to the server. - print('Killing Chrome...'); - chrome.kill(); - await chrome.onExit; - await Future.wait([stdoutSub.cancel(), stderrSub.cancel()]); + // Close the browser, which will disconnect DevTools SSE connection + // back to the server. + print('Killing Chrome...'); + chromeProcess.kill(); + await chromeProcess.exitCode; + await Future.wait([stdoutSub.cancel(), stderrSub.cancel()]); - // Await a long delay to wait for the SSE client to close. - print('Delaying to wait for SSE connection to cleanup...'); - await delay(duration: const Duration(seconds: 15)); + // Await a long delay to wait for the SSE client to close. + print('Delaying to wait for SSE connection to cleanup...'); + await delay(duration: const Duration(seconds: 15)); - // Ensure the client is completely removed from the list. - print('Expecting no clients...'); - await testController.waitForClients(expectNone: true); + // Ensure the client is completely removed from the list. + print('Expecting no clients...'); + await testController.waitForClients(expectNone: true); - print('Done!'); + print('Done!'); + } finally { + // Clean up the temporary profile directory. + try { + await tempDir.delete(recursive: true); + } catch (_) { + // Ignore cleanup errors since the OS temp dir is eventually cleaned up. + } + } }, timeout: const Timeout.factor(20)); }); }