From 12d9f409611a98dfdaa0e5ca7aa4fb05a9dabf00 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Thu, 26 Sep 2024 15:29:05 +0000 Subject: [PATCH] Reapply "[ CLI ] Don't disable DDS when --disable-dart-dev is provided" This reverts commit 9f5246e8c65bce499e04cec3dd38b01db5f84056. TEST=Existing tests Change-Id: I658290f248d8b26c05f24b7125c3ba71de318bf9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/385121 Reviewed-by: Siva Annamalai Commit-Queue: Ben Konyi --- .../integration/support/integration_tests.dart | 9 ++++++++- pkg/dartdev/test/commands/devtools_test.dart | 2 +- pkg/dds/test/common/test_helper.dart | 2 +- pkg/front_end/test/hot_reload_e2e_test.dart | 4 +--- pkg/vm/test/incremental_compiler_test.dart | 5 ----- pkg/vm_service/test/common/utils.dart | 2 +- .../test/get_supported_protocols_common.dart | 16 ++-------------- .../get_supported_protocols_with_dds_test.dart | 5 ----- pkg/vm_service/test/regress_55559_test.dart | 5 ++++- runtime/bin/main_impl.cc | 7 ++----- runtime/tests/vm/dart/isolates/reload_utils.dart | 2 +- runtime/tools/heapsnapshot/test/utils.dart | 1 + 12 files changed, 22 insertions(+), 38 deletions(-) diff --git a/pkg/analysis_server/test/integration/support/integration_tests.dart b/pkg/analysis_server/test/integration/support/integration_tests.dart index ae21dc5282c..8e640e267d1 100644 --- a/pkg/analysis_server/test/integration/support/integration_tests.dart +++ b/pkg/analysis_server/test/integration/support/integration_tests.dart @@ -549,12 +549,19 @@ class Server { // Guard against lines like: // {"event":"server.connected","params":{...}}The Dart VM service is listening on ... - var dartVMServiceMessage = 'The Dart VM service is listening on '; + const dartVMServiceMessage = 'The Dart VM service is listening on '; if (trimmedLine.contains(dartVMServiceMessage)) { trimmedLine = trimmedLine .substring(0, trimmedLine.indexOf(dartVMServiceMessage)) .trim(); } + const devtoolsMessage = + 'The Dart DevTools debugger and profiler is available at:'; + if (trimmedLine.contains(devtoolsMessage)) { + trimmedLine = trimmedLine + .substring(0, trimmedLine.indexOf(devtoolsMessage)) + .trim(); + } if (trimmedLine.isEmpty) { return; } diff --git a/pkg/dartdev/test/commands/devtools_test.dart b/pkg/dartdev/test/commands/devtools_test.dart index a8afc94d65f..f6025e2bc59 100644 --- a/pkg/dartdev/test/commands/devtools_test.dart +++ b/pkg/dartdev/test/commands/devtools_test.dart @@ -220,7 +220,7 @@ Future main() async { }) async { targetProjectInstance = await targetProject.start( [ - '--disable-dart-dev', + '--no-dds', '--observe=0', if (disableServiceAuthCodes) '--disable-service-auth-codes', targetProject.relativeFilePath, diff --git a/pkg/dds/test/common/test_helper.dart b/pkg/dds/test/common/test_helper.dart index c0b44d7a643..2217e12b64b 100644 --- a/pkg/dds/test/common/test_helper.dart +++ b/pkg/dds/test/common/test_helper.dart @@ -44,7 +44,7 @@ Future spawnDartProcess( final serviceInfoFile = await io.File.fromUri(serviceInfoUri).create(); final arguments = [ - '--disable-dart-dev', + '--no-dds', '--observe=0', if (!serveObservatory) '--no-serve-observatory', if (pauseOnStart) '--pause-isolates-on-start', diff --git a/pkg/front_end/test/hot_reload_e2e_test.dart b/pkg/front_end/test/hot_reload_e2e_test.dart index c62383cfebb..ddf0367d2e3 100644 --- a/pkg/front_end/test/hot_reload_e2e_test.dart +++ b/pkg/front_end/test/hot_reload_e2e_test.dart @@ -95,9 +95,7 @@ abstract class TestCase { '--enable-vm-service=0', // Note: use 0 to avoid port collisions. '--pause_isolates_on_start', '--disable-service-auth-codes', - // TODO(bkonyi): The service isolate starts before DartDev has a chance - // to spawn DDS. We should suppress the Observatory message until DDS - // starts (#42727). + '--no-dds', '--disable-dart-dev', outputUri.toFilePath() ]; diff --git a/pkg/vm/test/incremental_compiler_test.dart b/pkg/vm/test/incremental_compiler_test.dart index bf14a4bfbad..72b88c34bb1 100644 --- a/pkg/vm/test/incremental_compiler_test.dart +++ b/pkg/vm/test/incremental_compiler_test.dart @@ -503,7 +503,6 @@ main() { "--pause-isolates-on-exit", "--enable-vm-service:0", "--disable-service-auth-codes", - "--disable-dart-dev", list.path ]); @@ -607,7 +606,6 @@ main() { "--pause-isolates-on-exit", "--enable-vm-service:0", "--disable-service-auth-codes", - "--disable-dart-dev", list.path ]); @@ -873,7 +871,6 @@ main() { "--pause-isolates-on-exit", "--enable-vm-service:0", "--disable-service-auth-codes", - "--disable-dart-dev", list.path ]); @@ -955,7 +952,6 @@ main() { '--enable-vm-service=0', // Note: use 0 to avoid port collisions. '--pause_isolates_on_start', '--disable-service-auth-codes', - '--disable-dart-dev', outputFile.path ]; final vm = await Process.start(Platform.resolvedExecutable, vmArgs); @@ -1363,7 +1359,6 @@ main() { "--pause-isolates-on-start", "--enable-vm-service:0", "--disable-service-auth-codes", - "--disable-dart-dev", scriptOrDill.path ]); diff --git a/pkg/vm_service/test/common/utils.dart b/pkg/vm_service/test/common/utils.dart index 5ab35fabdd7..0d3b2c6468a 100644 --- a/pkg/vm_service/test/common/utils.dart +++ b/pkg/vm_service/test/common/utils.dart @@ -19,7 +19,7 @@ Future<(Process, Uri)> spawnDartProcess( final serviceInfoFile = await File.fromUri(serviceInfoUri).create(); final arguments = [ - '--disable-dart-dev', + '--no-dds', '--observe=0', if (!serveObservatory) '--no-serve-observatory', if (pauseOnStart) '--pause-isolates-on-start', diff --git a/pkg/vm_service/test/get_supported_protocols_common.dart b/pkg/vm_service/test/get_supported_protocols_common.dart index a7be12d5fd7..f3bb133a601 100644 --- a/pkg/vm_service/test/get_supported_protocols_common.dart +++ b/pkg/vm_service/test/get_supported_protocols_common.dart @@ -7,22 +7,10 @@ import 'package:vm_service/vm_service.dart'; import 'common/test_helper.dart'; -/// [expectMissingProtocol] allows for a single protocol to be missing. See -/// https://github.com/dart-lang/sdk/issues/54835 for context. This test will -/// fail without this flag on AOT configurations when DDS is expected since DDS -/// isn't currently setup to run with dart_precompiled_runtime. This flag is -/// meant to cause this test to fail if -/// https://github.com/dart-lang/sdk/issues/54841 is resolved so this test can -/// be updated. -VMTest expectedProtocolTest( - List expectedProtocols, { - bool expectMissingProtocol = false, -}) => +VMTest expectedProtocolTest(List expectedProtocols) => (VmService service) async { final protocols = (await service.getSupportedProtocols()).protocols!; - final expectedLength = - expectedProtocols.length - (expectMissingProtocol ? 1 : 0); - expect(protocols.length, expectedLength); + expect(protocols.length, expectedProtocols.length); for (final protocol in protocols) { expect(expectedProtocols.contains(protocol.protocolName), true); expect(protocol.minor, greaterThanOrEqualTo(0)); diff --git a/pkg/vm_service/test/get_supported_protocols_with_dds_test.dart b/pkg/vm_service/test/get_supported_protocols_with_dds_test.dart index d2dfb1ce99f..503bd7db7ff 100644 --- a/pkg/vm_service/test/get_supported_protocols_with_dds_test.dart +++ b/pkg/vm_service/test/get_supported_protocols_with_dds_test.dart @@ -2,8 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'dart:io'; - import 'common/test_helper.dart'; import 'get_supported_protocols_common.dart'; @@ -13,9 +11,6 @@ final tests = [ 'VM Service', 'DDS', ], - // See https://github.com/dart-lang/sdk/issues/54841 - expectMissingProtocol: - Platform.script.toString().endsWith('out.aotsnapshot'), ), ]; diff --git a/pkg/vm_service/test/regress_55559_test.dart b/pkg/vm_service/test/regress_55559_test.dart index 25248c03466..57c4f745192 100644 --- a/pkg/vm_service/test/regress_55559_test.dart +++ b/pkg/vm_service/test/regress_55559_test.dart @@ -54,7 +54,10 @@ void main() { ], ); final service = await vmServiceConnectUri(wsUri.toString()); - final vm = await service.getVM(); + VM vm; + do { + vm = await service.getVM(); + } while (vm.isolates!.isEmpty); final isolate = vm.isolates!.first; final errorCompleter = Completer(); final stackTraceCompleter = Completer(); diff --git a/runtime/bin/main_impl.cc b/runtime/bin/main_impl.cc index 6e2a86fff8b..9e3aa8c4f99 100644 --- a/runtime/bin/main_impl.cc +++ b/runtime/bin/main_impl.cc @@ -557,11 +557,8 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri, result = Dart_SetDeferredLoadHandler(Loader::DeferredLoadHandler); CHECK_RESULT(result); - // We do not spawn the external dds process in the following scenarios: - // - The DartDev CLI is disabled and VM service is enabled. - // - DDS is disabled. - bool wait_for_dds_to_advertise_service = - !(Options::disable_dart_dev() || Options::disable_dds()); + // We do not spawn the external dds process if DDS is explicitly disabled. + bool wait_for_dds_to_advertise_service = !Options::disable_dds(); bool serve_devtools = Options::enable_devtools() || !Options::disable_devtools(); // Load embedder specific bits and return. diff --git a/runtime/tests/vm/dart/isolates/reload_utils.dart b/runtime/tests/vm/dart/isolates/reload_utils.dart index a624a33055c..36ffc6ea2d4 100644 --- a/runtime/tests/vm/dart/isolates/reload_utils.dart +++ b/runtime/tests/vm/dart/isolates/reload_utils.dart @@ -127,7 +127,7 @@ Future launchOn(String file, {bool verbose = false}) async { if (verbose) '--trace-reload', if (verbose) '--trace-reload-verbose', '--enable-vm-service:0', - '--disable-dart-dev', + '--no-dds', '--disable-service-auth-codes', file ]; diff --git a/runtime/tools/heapsnapshot/test/utils.dart b/runtime/tools/heapsnapshot/test/utils.dart index 67476d34b59..5917bfcdcb8 100644 --- a/runtime/tools/heapsnapshot/test/utils.dart +++ b/runtime/tools/heapsnapshot/test/utils.dart @@ -30,6 +30,7 @@ class Testee { final processArgs = [ ...Platform.executableArguments, '--disable-dart-dev', + '--no-dds', '--disable-service-auth-codes', '--enable-vm-service:0', '--pause-isolates-on-exit',