From 97c7f95503312e4c9a98457170f4950bc51e97f7 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Wed, 20 Apr 2022 14:42:36 -0700 Subject: [PATCH 1/8] Fix finding locations on breakpoints, call frames, and expression evaluation --- dwds/CHANGELOG.md | 1 + dwds/lib/src/debugging/debugger.dart | 13 +------ dwds/lib/src/debugging/location.dart | 56 ++++++++++++++++++++++------ 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 8621e53dd..2a7d91f6d 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -8,6 +8,7 @@ used. - Fix crash when using flutter tools with web server device. - Remove clearing all scripts on page load for extension debugger. +- Find best locations for call frames, breakpoints, or expression evaluation. ## 13.1.0 - Update _fe_analyzer_shared to version ^38.0.0. diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index dca3156e5..f85188e8f 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -474,18 +474,7 @@ class Debugger extends Domain { return null; } - // TODO(sdk/issues/37240) - ideally we look for an exact location instead - // of the closest location on a given line. - Location bestLocation; - for (var location in await _locations.locationsForUrl(url)) { - if (location.jsLocation.line == line) { - bestLocation ??= location; - if ((location.jsLocation.column - column).abs() < - (bestLocation.jsLocation.column - column).abs()) { - bestLocation = location; - } - } - } + var bestLocation = await _locations.locationForJs(url, line, column); if (bestLocation == null) return null; var script = diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 1dad90baa..1477b2c3a 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -152,22 +152,54 @@ class Locations { /// Find the [Location] for the given Dart source position. /// /// The [line] number is 1-based. - Future locationForDart(DartUri uri, int line, int column) async => - (await locationsForDart(uri.serverPath)).firstWhere( - (location) => - location.dartLocation.line == line && - location.dartLocation.column >= column, - orElse: () => null); + Future locationForDart(DartUri uri, int line, int column) async { + return _bestDartLocation( + await locationsForDart(uri.serverPath), line, column); + } /// Find the [Location] for the given JS source position. /// /// The [line] number is 0-based. - Future locationForJs(String url, int line, int column) async => - (await locationsForUrl(url)).firstWhere( - (location) => - location.jsLocation.line == line && - location.jsLocation.column >= column, - orElse: () => null); + Future locationForJs(String url, int line, int column) async { + return _bestJsLocation(await locationsForUrl(url), line, column); + } + + /// Find closest existing JavaScript location for the line nd column. + /// + /// Chrome locations are either exact or placed at the first chracter + /// of the line, so find the closest location on the column or after. + Location _bestJsLocation(Iterable locations, int line, int column) { + Location bestLocation; + for (var location in locations) { + if (location.jsLocation.line == line && + location.jsLocation.column >= column) { + bestLocation ??= location; + if (location.jsLocation.column < bestLocation.jsLocation.column) { + bestLocation = location; + } + } + } + return bestLocation; + } + + /// Find closest existing Dart location for the line and column. + /// + /// Dart locations are either exact or placed at the start of the line, + /// so find the closest location on the column or after. + Location _bestDartLocation( + Iterable locations, int line, int column) { + Location bestLocation; + for (var location in locations) { + if (location.dartLocation.line == line && + location.dartLocation.column >= column) { + bestLocation ??= location; + if (location.dartLocation.column < bestLocation.dartLocation.column) { + bestLocation = location; + } + } + } + return bestLocation; + } /// Returns the tokenPosTable for the provided Dart script path as defined /// in: From c998387a32ec50302daf218d4aaa8764aaeafe74 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Wed, 20 Apr 2022 17:40:00 -0700 Subject: [PATCH 2/8] Update JS and Dart location search heuristic --- dwds/lib/src/debugging/location.dart | 70 +++++++++++++++++++--------- 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 1477b2c3a..036efb724 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -153,28 +153,31 @@ class Locations { /// /// The [line] number is 1-based. Future locationForDart(DartUri uri, int line, int column) async { - return _bestDartLocation( - await locationsForDart(uri.serverPath), line, column); + var locations = await locationsForDart(uri.serverPath); + return _bestDartLocation(locations, line, column); } /// Find the [Location] for the given JS source position. /// /// The [line] number is 0-based. Future locationForJs(String url, int line, int column) async { - return _bestJsLocation(await locationsForUrl(url), line, column); + var locations = await locationsForUrl(url); + return _bestJsLocation(locations, line, column); } - /// Find closest existing JavaScript location for the line nd column. + /// Find closest existing Dart location for the line and column. /// - /// Chrome locations are either exact or placed at the first chracter - /// of the line, so find the closest location on the column or after. - Location _bestJsLocation(Iterable locations, int line, int column) { + /// Dart columns for breakpoints are either exact or start at the + /// beginning of the line - return the first existing location + /// that comes after the given column. + Location _bestDartLocation( + Iterable locations, int line, int column) { Location bestLocation; for (var location in locations) { - if (location.jsLocation.line == line && - location.jsLocation.column >= column) { + if (location.dartLocation.line == line && + location.dartLocation.column >= column) { bestLocation ??= location; - if (location.jsLocation.column < bestLocation.jsLocation.column) { + if (location.dartLocation.column < bestLocation.dartLocation.column) { bestLocation = location; } } @@ -182,23 +185,46 @@ class Locations { return bestLocation; } - /// Find closest existing Dart location for the line and column. + /// Find closest existing JavaScript location for the line and column. /// - /// Dart locations are either exact or placed at the start of the line, - /// so find the closest location on the column or after. - Location _bestDartLocation( - Iterable locations, int line, int column) { - Location bestLocation; + /// Some JS locations are not stored in the source maps, so we find the + /// closest existing location, preferring the one coming after the given + /// column. + /// + /// For example: + /// + /// - `t33 = main.doSomething()` in top frame: + /// Current column is at `t33`. Return existing location starting + /// at `main`. + /// - `main.doSomething()` in top frame: + /// Current column is at `main`. Return existing location starting + /// at `main`. + /// - `main.doSomething()` in a frame down the stack: + /// Current column is at `doSomething`. Source map do not have a + /// location stored that starts at `doSomething(). Return existing + /// location starting at `main`. + Location _bestJsLocation(Iterable locations, int line, int column) { + Location bestBeforeLocation; + Location bestAfterLocation; for (var location in locations) { - if (location.dartLocation.line == line && - location.dartLocation.column >= column) { - bestLocation ??= location; - if (location.dartLocation.column < bestLocation.dartLocation.column) { - bestLocation = location; + if (location.jsLocation.line == line) { + if (location.jsLocation.column >= column) { + bestAfterLocation ??= location; + if (location.jsLocation.column < + bestAfterLocation.jsLocation.column) { + bestAfterLocation = location; + } + } + if (location.jsLocation.column <= column) { + bestBeforeLocation ??= location; + if (location.jsLocation.column > + bestBeforeLocation.jsLocation.column) { + bestBeforeLocation = location; + } } } } - return bestLocation; + return bestAfterLocation ?? bestBeforeLocation; } /// Returns the tokenPosTable for the provided Dart script path as defined From 78be6b57f0d26b6bbdefd2d3b9ed5f0f1d7b1a59 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Thu, 21 Apr 2022 10:10:21 -0700 Subject: [PATCH 3/8] Fix typos in comments --- dwds/lib/src/debugging/location.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 036efb724..5bc98a769 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -200,8 +200,8 @@ class Locations { /// Current column is at `main`. Return existing location starting /// at `main`. /// - `main.doSomething()` in a frame down the stack: - /// Current column is at `doSomething`. Source map do not have a - /// location stored that starts at `doSomething(). Return existing + /// Current column is at `doSomething`. Source map does not have a + /// location stored that starts at `doSomething()`. Return existing /// location starting at `main`. Location _bestJsLocation(Iterable locations, int line, int column) { Location bestBeforeLocation; From 2db0960799f1a67db6b094c9c0fde7bfb8af5d34 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 25 Apr 2022 10:24:51 -0700 Subject: [PATCH 4/8] Updated heuristic for finding dart locations to work for more cases, added tests --- dwds/lib/src/debugging/debugger.dart | 14 +- dwds/lib/src/debugging/frame_computer.dart | 20 +- dwds/lib/src/debugging/location.dart | 73 +++-- .../src/services/expression_evaluator.dart | 10 +- dwds/test/build_daemon_breakpoint_test.dart | 22 -- dwds/test/build_daemon_callstack_test.dart | 252 +++++++++++++++++ dwds/test/build_daemon_evaluate_test.dart | 11 +- dwds/test/fixtures/context.dart | 9 +- dwds/test/frontend_server_callstack_test.dart | 258 ++++++++++++++++++ fixtures/_testPackage/web/main.dart | 50 +++- fixtures/_testPackageSound/pubspec.yaml | 6 +- fixtures/_testPackageSound/web/main.dart | 56 +++- fixtures/_testSound/pubspec.yaml | 4 +- frontend_server_common/lib/src/devfs.dart | 24 +- .../lib/src/resident_runner.dart | 10 +- 15 files changed, 722 insertions(+), 97 deletions(-) create mode 100644 dwds/test/build_daemon_callstack_test.dart create mode 100644 dwds/test/frontend_server_callstack_test.dart diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index f85188e8f..f1332bf74 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -317,7 +317,7 @@ class Debugger extends Domain { } /// Returns Chrome script uri for Chrome script ID. - String _urlForScriptId(String scriptId) => + String urlForScriptId(String scriptId) => _remoteDebugger.scripts[scriptId]?.url; /// Returns source [Location] for the paused event. @@ -331,7 +331,7 @@ class Debugger extends Domain { var line = location['lineNumber'] as int; var column = location['columnNumber'] as int; - var url = _urlForScriptId(scriptId); + var url = urlForScriptId(scriptId); return _locations.locationForJs(url, line, column); } @@ -467,7 +467,7 @@ class Debugger extends Domain { var line = location.lineNumber; var column = location.columnNumber; - var url = _urlForScriptId(location.scriptId); + var url = urlForScriptId(location.scriptId); if (url == null) { logger.severe('Failed to create dart frame for ${frame.functionName}: ' 'cannot find location for script ${location.scriptId}'); @@ -495,7 +495,11 @@ class Debugger extends Domain { kind: CodeKind.kDart, id: createId(), ), - location: SourceLocation(tokenPos: bestLocation.tokenPos, script: script), + location: SourceLocation( + line: bestLocation.dartLocation.line, + column: bestLocation.dartLocation.column, + tokenPos: bestLocation.tokenPos, + script: script), kind: FrameKind.kRegular, ); @@ -568,7 +572,7 @@ class Debugger extends Domain { var frame = e.params['callFrames'][0]; var scriptId = '${frame["location"]["scriptId"]}'; - var url = _urlForScriptId(scriptId); + var url = urlForScriptId(scriptId); if (url == null) { logger.severe('Stepping failed: ' 'cannot find location for script $scriptId'); diff --git a/dwds/lib/src/debugging/frame_computer.dart b/dwds/lib/src/debugging/frame_computer.dart index f03dc8d39..3ac62ed1c 100644 --- a/dwds/lib/src/debugging/frame_computer.dart +++ b/dwds/lib/src/debugging/frame_computer.dart @@ -4,22 +4,24 @@ // @dart = 2.9 +import 'package:logging/logging.dart'; import 'package:pool/pool.dart'; import 'package:vm_service/vm_service.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; -import 'dart_scope.dart'; import 'debugger.dart'; class FrameComputer { final Debugger debugger; + final _logger = Logger('FrameComputer'); + // To ensure that the frames are computed only once, we use a pool to guard // the work. Frames are computed sequentially. final _pool = Pool(1); final List _callFrames; - final _computedFrames = []; + final List _computedFrames = []; var _frameIndex = 0; @@ -36,12 +38,6 @@ class FrameComputer { return frameIndex < _callFrames.length ? _callFrames[frameIndex] : null; } - /// Return the WipScopes for the given JavaScript frame index that are - /// pertinent for Dart debugging. - List getWipScopesForFrameIndex(int frameIndex) { - return filterScopes(jsFrameForIndex(frameIndex)); - } - /// Translates Chrome callFrames contained in [DebuggerPausedEvent] into Dart /// [Frame]s. Future> calculateFrames({int limit}) async { @@ -101,6 +97,14 @@ class FrameComputer { var location = WipLocation.fromValues( callFrame.scriptId, callFrame.lineNumber, columnNumber: callFrame.columnNumber); + + var url = callFrame.url ?? debugger.urlForScriptId(location.scriptId); + if (url == null) { + _logger.severe( + 'Failed to create dart frame for ${callFrame.functionName}: ' + 'cannot find location for script ${callFrame.scriptId}'); + } + var tempWipFrame = WipCallFrame({ 'url': callFrame.url, 'functionName': callFrame.functionName, diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart index 5bc98a769..acdd19b38 100644 --- a/dwds/lib/src/debugging/location.dart +++ b/dwds/lib/src/debugging/location.dart @@ -189,7 +189,16 @@ class Locations { /// /// Some JS locations are not stored in the source maps, so we find the /// closest existing location, preferring the one coming after the given - /// column. + /// column, if the current break is at an expression statement, or the + /// one coming before if the current break is at a function call. + /// + /// This is a known problem that other code bases solve using by finding + /// the closest location to the current one: + /// + /// https://github.com/microsoft/vscode-js-debug/blob/536f96bae61a3d87546b61bc7916097904c81429/src/common/sourceUtils.ts#L286 + /// + /// Unfortunately, this approach fails for Flutter code too often, as it + /// frequently contains multi-line statements with nested objects. /// /// For example: /// @@ -204,27 +213,51 @@ class Locations { /// location stored that starts at `doSomething()`. Return existing /// location starting at `main`. Location _bestJsLocation(Iterable locations, int line, int column) { - Location bestBeforeLocation; - Location bestAfterLocation; - for (var location in locations) { - if (location.jsLocation.line == line) { - if (location.jsLocation.column >= column) { - bestAfterLocation ??= location; - if (location.jsLocation.column < - bestAfterLocation.jsLocation.column) { - bestAfterLocation = location; - } - } - if (location.jsLocation.column <= column) { - bestBeforeLocation ??= location; - if (location.jsLocation.column > - bestBeforeLocation.jsLocation.column) { - bestBeforeLocation = location; - } - } + Location bestLocationBefore; + Location bestLocationAfter; + + var locationsAfter = locations.where((location) => + location.jsLocation.line == line && + location.jsLocation.column >= column); + var locationsBefore = locations.where((location) => + location.jsLocation.line == line && + location.jsLocation.column < column); + + for (var location in locationsAfter) { + bestLocationAfter ??= location; + if (location.jsLocation.column < bestLocationAfter.jsLocation.column) { + bestLocationAfter = location; + } + } + for (var location in locationsBefore) { + bestLocationBefore ??= location; + if (location.jsLocation.column > bestLocationBefore.jsLocation.column) { + bestLocationBefore = location; } } - return bestAfterLocation ?? bestBeforeLocation; + if (bestLocationAfter == null) return bestLocationBefore; + if (bestLocationBefore == null) return bestLocationAfter; + + if (bestLocationAfter.jsLocation.line == line && + bestLocationAfter.jsLocation.column == column) { + // Prefer exact match. + return bestLocationAfter; + } + + // Return the closest location after the current if the current location + // is at the beginning of the line (i.e. on expression statement). + // Return the closest location before the current if the current location + // is in the middle of the line (i.e. on function call). + if (locationsBefore.length == 1 && + locationsBefore.first.jsLocation.column == 0) { + // Best guess on whether the the current location is at the beginning of + // the line (i.e. expression statement): + // The only location on the left has column 0, so only spaces are on the + // left. + return bestLocationAfter; + } + // Current column is in the middle of the line (i.e .function call). + return bestLocationBefore; } /// Returns the tokenPosTable for the provided Dart script path as defined diff --git a/dwds/lib/src/services/expression_evaluator.dart b/dwds/lib/src/services/expression_evaluator.dart index f86fd5ffe..9fabccc9a 100644 --- a/dwds/lib/src/services/expression_evaluator.dart +++ b/dwds/lib/src/services/expression_evaluator.dart @@ -155,7 +155,9 @@ class ExpressionEvaluator { var jsFrame = _inspector.debugger.jsFrameForIndex(frameIndex); if (jsFrame == null) { return _createError( - ErrorKind.internal, 'No frame with index $frameIndex'); + ErrorKind.internal, + 'Expression evaluation in async frames ' + 'is not supported. No frame with index $frameIndex.'); } var functionName = jsFrame.functionName; @@ -165,12 +167,6 @@ class ExpressionEvaluator { var jsScope = await _collectLocalJsScope(jsFrame); // Find corresponding dart location and scope. - // - // TODO(annagrin): handle unknown dart locations - // Debugger does not map every js location to a dart location, - // so this will result in expressions not evaluated in some - // cases. Invent location matching strategy for those cases. - // [issue 890](https://github.com/dart-lang/webdev/issues/890) var url = _urlForScriptId(jsScriptId); var locationMap = await _locations.locationForJs(url, jsLine, jsColumn); if (locationMap == null) { diff --git a/dwds/test/build_daemon_breakpoint_test.dart b/dwds/test/build_daemon_breakpoint_test.dart index 876383a5e..847adedf9 100644 --- a/dwds/test/build_daemon_breakpoint_test.dart +++ b/dwds/test/build_daemon_breakpoint_test.dart @@ -74,28 +74,6 @@ void main() { await service.removeBreakpoint(isolate.id, bp.id); }); - test('set breakpoint inside a JavaScript line succeeds on', () async { - var line = await context.findBreakpointLine( - 'printNestedObjectMultiLine', isolate.id, mainScript); - var column = 0; - var bp = await service.addBreakpointWithScriptUri( - isolate.id, mainScript.uri, line, - column: column); - await stream.firstWhere( - (Event event) => event.kind == EventKind.kPauseBreakpoint); - - expect(bp, isNotNull); - expect( - bp.location, - isA() - .having((loc) => loc.script, 'script', equals(mainScript)) - .having((loc) => loc.line, 'line', equals(line)) - .having((loc) => loc.column, 'column', greaterThan(column))); - - // Remove breakpoint so it doesn't impact other tests. - await service.removeBreakpoint(isolate.id, bp.id); - }); - test('set breakpoint again', () async { var line = await context.findBreakpointLine( 'printLocal', isolate.id, mainScript); diff --git a/dwds/test/build_daemon_callstack_test.dart b/dwds/test/build_daemon_callstack_test.dart new file mode 100644 index 000000000..f57245225 --- /dev/null +++ b/dwds/test/build_daemon_callstack_test.dart @@ -0,0 +1,252 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// 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. + +// @dart = 2.9 + +@TestOn('vm') +import 'dart:async'; + +import 'package:dwds/src/connections/debug_connection.dart'; +import 'package:dwds/src/services/chrome_proxy_service.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:vm_service/vm_service.dart'; +import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; + +import 'fixtures/context.dart'; +import 'fixtures/logging.dart'; + +class TestSetup { + static final contextUnsound = TestContext( + directory: p.join('..', 'fixtures', '_testPackage'), + entry: p.join('..', 'fixtures', '_testPackage', 'web', 'main.dart'), + path: 'index.html', + pathToServe: 'web'); + + static final contextSound = TestContext( + directory: p.join('..', 'fixtures', '_testPackageSound'), + entry: p.join('..', 'fixtures', '_testPackageSound', 'web', 'main.dart'), + path: 'index.html', + pathToServe: 'web'); + + TestContext context; + + TestSetup.sound() : context = contextSound; + + TestSetup.unsound() : context = contextUnsound; + + ChromeProxyService get service => + fetchChromeProxyService(context.debugConnection); + WipConnection get tabConnection => context.tabConnection; +} + +void main() { + group('shared context |', () { + // Enable verbose logging for debugging. + var debug = false; + + for (var soundNullSafety in [false, true]) { + var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); + var context = setup.context; + + group('${soundNullSafety ? "" : "no "}sound null safety |', () { + setUpAll(() async { + setCurrentLogWriter(debug: debug); + await context.setUp(enableExpressionEvaluation: true); + }); + + tearDownAll(() async { + await context.tearDown(); + }); + + group('callStack |', () { + ChromeProxyService service; + VM vm; + Isolate isolate; + ScriptList scripts; + ScriptRef mainScript; + Stream stream; + + setUp(() async { + setCurrentLogWriter(debug: debug); + service = setup.service; + vm = await service.getVM(); + isolate = await service.getIsolate(vm.isolates.first.id); + scripts = await service.getScripts(isolate.id); + + await service.streamListen('Debug'); + stream = service.onEvent('Debug'); + + mainScript = scripts.scripts + .firstWhere((each) => each.uri.contains('main.dart')); + }); + + tearDown(() async { + await service.resume(isolate.id); + }); + + Future onBreakPoint(ScriptRef script, String breakPointId, + Future Function() body) async { + Breakpoint bp; + try { + var line = await context.findBreakpointLine( + breakPointId, isolate.id, script); + bp = await setup.service + .addBreakpointWithScriptUri(isolate.id, script.uri, line); + + expect(bp, isNotNull); + expect(bp.location, _matchBpLocation(mainScript, line, 0)); + + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseBreakpoint); + + await body(); + } finally { + // Remove breakpoint so it doesn't impact other tests or retries. + if (bp != null) { + await setup.service.removeBreakpoint(isolate.id, bp.id); + } + } + } + + Future testCallStack( + List bpIds, List functions) async { + // Find lines the breakpoints are located on. + var lines = await Future.wait(bpIds.map((bpId) => + context.findBreakpointLine(bpId, isolate.id, mainScript))); + + // Get current stack. + var stack = await service.getStack(isolate.id); + + // Verify the stack is correct. + expect(stack.frames.length, greaterThanOrEqualTo(lines.length)); + var expected = [ + for (var i = 0; i < lines.length; i++) + _matchFrame(mainScript, functions[i], lines[i]) + ]; + expect(stack.frames, containsAll(expected)); + + // Verify that expression evaluation is not failing. + var instance = await service.evaluateInFrame(isolate.id, 1, 'main'); + expect(instance, isA()); + } + + test('breakpoint succeeds with correct callstack', () async { + // Breakpoint IDs. + var bpIds = [ + 'printEnclosingObject', + 'printEnclosingFunctionMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'printEnclosingObject', + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint( + mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + }); + + test('breakpoint inside a line gives correct callstack', () async { + // Breakpoint IDs. + var bpIds = [ + 'newEnclosedClass', + 'printNestedObjectMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'new', + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint( + mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + }); + + test('breakpoint gives correct callstack after step out', () async { + // Breakpoint IDs. + var bpIds = [ + 'printEnclosingObjectMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint(mainScript, 'newEnclosedClass', () async { + await service.resume(isolate.id, step: 'Out'); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseInterrupted); + return testCallStack(bpIds, functions); + }); + }); + + test('breakpoint gives correct callstack after step in', () async { + // Breakpoint IDs. + var bpIds = [ + 'newEnclosedClass', + 'printNestedObjectMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'new', + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint(mainScript, 'printNestedObjectMultiLine', + () async { + await service.resume(isolate.id, step: 'Into'); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseInterrupted); + return testCallStack(bpIds, functions); + }); + }); + + test('breakpoint gives correct callstack after step into chain calls', + () async { + // Breakpoint IDs. + var bpIds = [ + 'createObjectWithMethod', + // This is currently incorrect, should be printObjectMultiLine. + // See issue: https://github.com/dart-lang/sdk/issues/48874 + 'printMultiLine', + 'callPrintObjectMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'createObject', + 'printObjectMultiLine', + '', + ]; + await onBreakPoint(mainScript, 'printMultiLine', () async { + await service.resume(isolate.id, step: 'Into'); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseInterrupted); + return testCallStack(bpIds, functions); + }); + }); + }); + }); + } + }); +} + +Matcher _matchFrame(ScriptRef script, String function, int line) => isA() + .having((frame) => frame.code.name, 'function', function) + .having((frame) => frame.location, 'location', + _matchFrameLocation(script, line)); + +Matcher _matchBpLocation(ScriptRef script, int line, int column) => + isA() + .having((loc) => loc.script, 'script', equals(script)) + .having((loc) => loc.line, 'line', equals(line)) + .having((loc) => loc.column, 'column', greaterThanOrEqualTo(column)); + +Matcher _matchFrameLocation(ScriptRef script, int line) => isA() + .having((loc) => loc.script, 'script', equals(script)) + .having((loc) => loc.line, 'line', equals(line)); diff --git a/dwds/test/build_daemon_evaluate_test.dart b/dwds/test/build_daemon_evaluate_test.dart index 4718ecebf..05714db94 100644 --- a/dwds/test/build_daemon_evaluate_test.dart +++ b/dwds/test/build_daemon_evaluate_test.dart @@ -52,7 +52,7 @@ void main() async { // regardless of the logger settings. var verboseCompiler = false; - for (var soundNullSafety in [false, true]) { + for (var soundNullSafety in [/*false, */ true]) { var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); var context = setup.context; group('${soundNullSafety ? "" : "no "}sound null safety', () { @@ -107,14 +107,17 @@ void main() async { await setup.service.streamListen('Debug'); stream = setup.service.onEvent('Debug'); + var testPackage = + soundNullSafety ? '_test_package_sound' : '_test_package'; + var test = soundNullSafety ? '_test_sound' : '_package'; mainScript = scripts.scripts .firstWhere((each) => each.uri.contains('main.dart')); testLibraryScript = scripts.scripts.firstWhere((each) => - each.uri.contains('package:_test_package/test_library.dart')); + each.uri.contains('package:$testPackage/test_library.dart')); testLibraryPartScript = scripts.scripts.firstWhere((each) => - each.uri.contains('package:_test_package/src/test_part.dart')); + each.uri.contains('package:$testPackage/src/test_part.dart')); libraryScript = scripts.scripts.firstWhere( - (each) => each.uri.contains('package:_test/library.dart')); + (each) => each.uri.contains('package:$test/library.dart')); }); tearDown(() async { diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index 61c38b2dd..79702ad59 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -72,6 +72,10 @@ class TestContext { /// Note: build_runner-based setups ignore this setting and read /// this value from the ddc debug metadata and pass it to the /// expression compiler worker initialiation API. + /// + /// TODO(annagrin): Currently setting sound null safety for frontend + /// server tests fails due to missing sound SDK JavaScript and maps. + /// Issue: https://github.com/dart-lang/webdev/issues/1591 bool soundNullSafety; final _logger = logging.Logger('Context'); @@ -125,6 +129,7 @@ class TestContext { UrlEncoder urlEncoder, bool restoreBreakpoints, CompilationMode compilationMode, + bool soundNullSafety, bool enableExpressionEvaluation, bool verboseCompiler, SdkConfigurationProvider sdkConfigurationProvider, @@ -140,6 +145,7 @@ class TestContext { spawnDds ??= true; verboseCompiler ??= false; sdkConfigurationProvider ??= DefaultSdkConfigurationProvider(); + soundNullSafety ??= false; try { configureLogWriter(); @@ -246,7 +252,6 @@ class TestContext { break; case CompilationMode.frontendServer: { - soundNullSafety ??= true; var projectDirectory = p.dirname(p.dirname(_packagesFilePath)); var entryPath = _entryFile.path.substring(projectDirectory.length + 1); @@ -257,6 +262,7 @@ class TestContext { [projectDirectory], 'org-dartlang-app', _outputDir.path, + soundNullSafety, verboseCompiler); var assetServerPort = await findUnusedPort(); @@ -287,7 +293,6 @@ class TestContext { // since headless Chrome does not support extensions. var headless = Platform.environment['DWDS_DEBUG_CHROME'] != 'true' && !enableDebugExtension; - var capabilities = Capabilities.chrome ..addAll({ Capabilities.chromeOptions: { diff --git a/dwds/test/frontend_server_callstack_test.dart b/dwds/test/frontend_server_callstack_test.dart new file mode 100644 index 000000000..79dce043e --- /dev/null +++ b/dwds/test/frontend_server_callstack_test.dart @@ -0,0 +1,258 @@ +// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file +// 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. + +// @dart = 2.9 + +@TestOn('vm') +import 'dart:async'; + +import 'package:dwds/src/connections/debug_connection.dart'; +import 'package:dwds/src/services/chrome_proxy_service.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:vm_service/vm_service.dart'; +import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; + +import 'fixtures/context.dart'; +import 'fixtures/logging.dart'; + +class TestSetup { + static final contextUnsound = TestContext( + directory: p.join('..', 'fixtures', '_testPackage'), + entry: p.join('..', 'fixtures', '_testPackage', 'web', 'main.dart'), + path: 'index.html', + pathToServe: 'web'); + + static final contextSound = TestContext( + directory: p.join('..', 'fixtures', '_testPackageSound'), + entry: p.join('..', 'fixtures', '_testPackageSound', 'web', 'main.dart'), + path: 'index.html', + pathToServe: 'web'); + + TestContext context; + + TestSetup.sound() : context = contextSound; + + TestSetup.unsound() : context = contextUnsound; + + ChromeProxyService get service => + fetchChromeProxyService(context.debugConnection); + WipConnection get tabConnection => context.tabConnection; +} + +void main() { + group('shared context |', () { + // Enable verbose logging for debugging. + var debug = false; + + for (var soundNullSafety in [false, true]) { + var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); + var context = setup.context; + + group('${soundNullSafety ? "" : "no "}sound null safety |', () { + setUpAll(() async { + setCurrentLogWriter(debug: debug); + await context.setUp( + enableExpressionEvaluation: true, + compilationMode: CompilationMode.frontendServer, + soundNullSafety: soundNullSafety, + ); + }); + + tearDownAll(() async { + await context.tearDown(); + }); + + group('callStack |', () { + ChromeProxyService service; + VM vm; + Isolate isolate; + ScriptList scripts; + ScriptRef mainScript; + Stream stream; + + setUp(() async { + setCurrentLogWriter(debug: debug); + service = setup.service; + vm = await service.getVM(); + isolate = await service.getIsolate(vm.isolates.first.id); + scripts = await service.getScripts(isolate.id); + + await service.streamListen('Debug'); + stream = service.onEvent('Debug'); + + mainScript = scripts.scripts + .firstWhere((each) => each.uri.contains('main.dart')); + }); + + tearDown(() async { + await service.resume(isolate.id); + }); + + Future onBreakPoint(ScriptRef script, String breakPointId, + Future Function() body) async { + Breakpoint bp; + try { + var line = await context.findBreakpointLine( + breakPointId, isolate.id, script); + bp = await setup.service + .addBreakpointWithScriptUri(isolate.id, script.uri, line); + + expect(bp, isNotNull); + expect(bp.location, _matchBpLocation(mainScript, line, 0)); + + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseBreakpoint); + + await body(); + } finally { + // Remove breakpoint so it doesn't impact other tests or retries. + if (bp != null) { + await setup.service.removeBreakpoint(isolate.id, bp.id); + } + } + } + + Future testCallStack( + List bpIds, List functions) async { + // Find lines the breakpoints are located on. + var lines = await Future.wait(bpIds.map((bpId) => + context.findBreakpointLine(bpId, isolate.id, mainScript))); + + // Get current stack. + var stack = await service.getStack(isolate.id); + + // Verify the stack is correct. + expect(stack.frames.length, greaterThanOrEqualTo(lines.length)); + var expected = [ + for (var i = 0; i < lines.length; i++) + _matchFrame(mainScript, functions[i], lines[i]) + ]; + expect(stack.frames, containsAll(expected)); + + // Verify that expression evaluation is not failing. + var instance = await service.evaluateInFrame(isolate.id, 1, 'main'); + expect(instance, isA()); + } + + test('breakpoint succeeds with correct callstack', () async { + // Breakpoint IDs. + var bpIds = [ + 'printEnclosingObject', + 'printEnclosingFunctionMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'printEnclosingObject', + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint( + mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + }); + + test('breakpoint inside a line gives correct callstack', () async { + // Breakpoint IDs. + var bpIds = [ + 'newEnclosedClass', + 'printNestedObjectMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'new', + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint( + mainScript, bpIds[0], () => testCallStack(bpIds, functions)); + }); + + test('breakpoint gives correct callstack after step out', () async { + // Breakpoint IDs. + var bpIds = [ + 'printEnclosingObjectMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint(mainScript, 'newEnclosedClass', () async { + await service.resume(isolate.id, step: 'Out'); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseInterrupted); + return testCallStack(bpIds, functions); + }); + }); + + test('breakpoint gives correct callstack after step in', () async { + // Breakpoint IDs. + var bpIds = [ + 'newEnclosedClass', + 'printNestedObjectMultiLine', + 'callPrintEnclosingFunctionMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'new', + 'printNestedObjectsMultiLine', + '', + ]; + await onBreakPoint(mainScript, 'printNestedObjectMultiLine', + () async { + await service.resume(isolate.id, step: 'Into'); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseInterrupted); + return testCallStack(bpIds, functions); + }); + }); + + test('breakpoint gives correct callstack after step into chain calls', + () async { + // Breakpoint IDs. + var bpIds = [ + 'createObjectWithMethod', + // This is currently incorrect, should be printObjectMultiLine. + // See issue: https://github.com/dart-lang/sdk/issues/48874 + 'printMultiLine', + 'callPrintObjectMultiLine', + ]; + // Functions breakpoints are located in. + var functions = [ + 'createObject', + 'printObjectMultiLine', + '', + ]; + await onBreakPoint(mainScript, 'printMultiLine', () async { + await service.resume(isolate.id, step: 'Into'); + await stream.firstWhere( + (Event event) => event.kind == EventKind.kPauseInterrupted); + return testCallStack(bpIds, functions); + }); + }); + }); + }, + skip: + soundNullSafety); // https://github.com/dart-lang/webdev/issues/1591 + } + }); +} + +Matcher _matchFrame(ScriptRef script, String function, int line) => isA() + .having((frame) => frame.code.name, 'function', function) + .having((frame) => frame.location, 'location', + _matchFrameLocation(script, line)); + +Matcher _matchBpLocation(ScriptRef script, int line, int column) => + isA() + .having((loc) => loc.script, 'script', equals(script)) + .having((loc) => loc.line, 'line', equals(line)) + .having((loc) => loc.column, 'column', greaterThanOrEqualTo(column)); + +Matcher _matchFrameLocation(ScriptRef script, int line) => isA() + .having((loc) => loc.script, 'script', equals(script)) + .having((loc) => loc.line, 'line', equals(line)); diff --git a/fixtures/_testPackage/web/main.dart b/fixtures/_testPackage/web/main.dart index 0d82f4b12..56fe7511c 100644 --- a/fixtures/_testPackage/web/main.dart +++ b/fixtures/_testPackage/web/main.dart @@ -40,7 +40,8 @@ void main() { printFromTestPackage(); printCallExtension(); printLoopVariable(); - printNestedObjectsMultiLine(); + printObjectMultiLine(); // Breakpoint: callPrintObjectMultiLine + printNestedObjectsMultiLine(); // Breakpoint: callPrintEnclosingFunctionMultiLine }); document.body.appendText(concatenate('Program', ' is running!')); @@ -101,23 +102,56 @@ Future printDeferred() async { } void printNestedObjectsMultiLine() { - print(// Breakpoint: printEnclosingFunctionMultiLine - EnclosingMainClass(// Breakpoint: printEnclosingObjectMultiLine - MainClass(0) // Breakpoint: printNestedObjectMultiLine + printEnclosingObject(// Breakpoint: printEnclosingFunctionMultiLine + EnclosingClass(// Breakpoint: printEnclosingObjectMultiLine + EnclosedClass(0) // Breakpoint: printNestedObjectMultiLine )); } +void printObjectMultiLine() { + print(// Breakpoint: printMultiLine + createObject() // Breakpoint: printObjectMultiLine + ..initialize(), + ); +} + +void printEnclosingObject(EnclosingClass o) { + print(o); // Breakpoint: printEnclosingObject +} + +ClassWithMethod createObject() { + return ClassWithMethod(0); // Breakpoint: createObjectWithMethod +} + class MainClass { final int _field; - MainClass(this._field); + MainClass(this._field); // Breakpoint: newMainClass + + @override + String toString() => '$_field'; +} + +class EnclosedClass { + final int _field; + EnclosedClass(this._field); // Breakpoint: newEnclosedClass + + @override + String toString() => '$_field'; +} + +class ClassWithMethod { + final int _field; + ClassWithMethod(this._field); + + void initialize() {} @override String toString() => '$_field'; } -class EnclosingMainClass { - final MainClass _field; - EnclosingMainClass(this._field); +class EnclosingClass { + final EnclosedClass _field; + EnclosingClass(this._field); // Breakpoint: newEnclosingClass @override String toString() => '$_field'; diff --git a/fixtures/_testPackageSound/pubspec.yaml b/fixtures/_testPackageSound/pubspec.yaml index 5306420e3..1d16ae578 100644 --- a/fixtures/_testPackageSound/pubspec.yaml +++ b/fixtures/_testPackageSound/pubspec.yaml @@ -1,14 +1,14 @@ -name: _test_package +name: _test_package_sound version: 1.0.0 description: >- A fake package used for testing imports. Imports _test. publish_to: none environment: - sdk: '>=2.12.0-259.0.dev <3.0.0' + sdk: '>=2.13.0 <3.0.0' dependencies: - _test: + _test_sound: path: ../_testSound dev_dependencies: diff --git a/fixtures/_testPackageSound/web/main.dart b/fixtures/_testPackageSound/web/main.dart index 5305c9639..1f19889e0 100644 --- a/fixtures/_testPackageSound/web/main.dart +++ b/fixtures/_testPackageSound/web/main.dart @@ -6,9 +6,9 @@ import 'dart:async'; import 'dart:core'; import 'dart:html'; -import 'package:_test/deferred_library.dart' deferred as d; -import 'package:_test/library.dart'; -import 'package:_test_package/test_library.dart'; +import 'package:_test_package_sound/test_library.dart'; +import 'package:_test_sound/deferred_library.dart' deferred as d; +import 'package:_test_sound/library.dart'; extension NumberParsing on String { int parseInt() { @@ -40,7 +40,8 @@ void main() async { printCallExtension(); printLoopVariable(); printGeneric(0); - printNestedObjectsMultiLine(); + printObjectMultiLine(); // Breakpoint: callPrintObjectMultiLine + printNestedObjectsMultiLine(); // Breakpoint: callPrintEnclosingFunctionMultiLine }); document.body?.appendText(concatenate('Program', ' is running!')); @@ -110,23 +111,56 @@ Future printDeferred() async { } void printNestedObjectsMultiLine() { - print(// Breakpoint: printEnclosingFunctionMultiLine - EnclosingMainClass(// Breakpoint: printEnclosingObjectMultiLine - MainClass(0) // Breakpoint: printNestedObjectMultiLine + printEnclosingObject(// Breakpoint: printEnclosingFunctionMultiLine + EnclosingClass(// Breakpoint: printEnclosingObjectMultiLine + EnclosedClass(0) // Breakpoint: printNestedObjectMultiLine )); } +void printObjectMultiLine() { + print(// Breakpoint: printMultiLine + createObject() // Breakpoint: printObjectMultiLine + ..initialize(), + ); +} + +void printEnclosingObject(EnclosingClass o) { + print(o); // Breakpoint: printEnclosingObject +} + +ClassWithMethod createObject() { + return ClassWithMethod(0); // Breakpoint: createObjectWithMethod +} + class MainClass { final int _field; - MainClass(this._field); + MainClass(this._field); // Breakpoint: newMainClass + + @override + String toString() => '$_field'; +} + +class EnclosedClass { + final int _field; + EnclosedClass(this._field); // Breakpoint: newEnclosedClass + + @override + String toString() => '$_field'; +} + +class ClassWithMethod { + final int _field; + ClassWithMethod(this._field); + + void initialize() {} @override String toString() => '$_field'; } -class EnclosingMainClass { - final MainClass _field; - EnclosingMainClass(this._field); +class EnclosingClass { + final EnclosedClass _field; + EnclosingClass(this._field); // Breakpoint: newEnclosingClass @override String toString() => '$_field'; diff --git a/fixtures/_testSound/pubspec.yaml b/fixtures/_testSound/pubspec.yaml index 98288b7ec..83f4cb689 100644 --- a/fixtures/_testSound/pubspec.yaml +++ b/fixtures/_testSound/pubspec.yaml @@ -1,11 +1,11 @@ -name: _test +name: _test_sound version: 1.0.0 description: >- A fake package used for testing publish_to: none environment: - sdk: '>=2.12.0-259.0.dev <3.0.0' + sdk: '>=2.13.0 <3.0.0' dependencies: intl: ^0.17.0-nullsafety.2 diff --git a/frontend_server_common/lib/src/devfs.dart b/frontend_server_common/lib/src/devfs.dart index df097a498..2fa2027a8 100644 --- a/frontend_server_common/lib/src/devfs.dart +++ b/frontend_server_common/lib/src/devfs.dart @@ -34,6 +34,7 @@ class WebDevFS { this.packageConfigPath, this.root, this.urlTunneller, + this.soundNullSafety, }); final FileSystem fileSystem; @@ -43,6 +44,7 @@ class WebDevFS { final String packageConfigPath; final String root; final UrlEncoder urlTunneller; + final bool soundNullSafety; Directory _savedCurrentDirectory; List sources; PackageConfig _packageConfig; @@ -94,9 +96,11 @@ class WebDevFS { ); assetServer.writeFile('/main_module.digests', '{}'); - assetServer.writeFile('/dart_sdk.js', dartSdk.readAsStringSync()); - assetServer.writeFile( - '/dart_sdk.js.map', dartSdkSourcemap.readAsStringSync()); + var sdk = soundNullSafety ? dartSdkSound : dartSdk; + var sdkSourceMap = + soundNullSafety ? dartSdkSourcemapSound : dartSdkSourcemap; + assetServer.writeFile('/dart_sdk.js', sdk.readAsStringSync()); + assetServer.writeFile('/dart_sdk.js.map', sdkSourceMap.readAsStringSync()); // TODO(jonahwilliams): refactor the asset code in this and the regular devfs to // be shared. if (bundle != null) { @@ -159,6 +163,13 @@ class WebDevFS { 'dart_sdk.js', )); + File get dartSdkSound => fileSystem.file(fileSystem.path.join( + dartWebSdkPath, + 'kernel', + 'amd', + 'dart_sdk_sound.js', + )); + File get dartSdkSourcemap => fileSystem.file(fileSystem.path.join( dartWebSdkPath, 'kernel', @@ -166,6 +177,13 @@ class WebDevFS { 'dart_sdk.js.map', )); + File get dartSdkSourcemapSound => fileSystem.file(fileSystem.path.join( + dartWebSdkPath, + 'kernel', + 'amd', + 'dart_sdk_sound.js.map', + )); + File get stackTraceMapper => fileSystem.file(fileSystem.path.join( dartWebSdkPath, 'web', diff --git a/frontend_server_common/lib/src/resident_runner.dart b/frontend_server_common/lib/src/resident_runner.dart index ed692f8ea..f5329ebe9 100644 --- a/frontend_server_common/lib/src/resident_runner.dart +++ b/frontend_server_common/lib/src/resident_runner.dart @@ -19,8 +19,10 @@ import 'devfs_content.dart'; import 'frontend_server_client.dart'; import 'utilities.dart'; -final Uri platformDill = +final Uri platformDillUnsound = Uri.file(p.join(dartSdkPath, 'lib', '_internal', 'ddc_sdk.dill')); +final Uri platformDillSound = + Uri.file(p.join(dartSdkPath, 'lib', '_internal', 'ddc_outline_sound.dill')); Logger _logger = Logger('ResidentWebRunner'); @@ -32,10 +34,12 @@ class ResidentWebRunner { this.fileSystemRoots, this.fileSystemScheme, this.outputPath, + this.soundNullSafety, bool verbose) { generator = ResidentCompiler(dartSdkPath, packageConfigPath: packageConfigPath, - platformDill: '$platformDill', + platformDill: + soundNullSafety ? '$platformDillSound' : '$platformDillUnsound', fileSystemRoots: fileSystemRoots, fileSystemScheme: fileSystemScheme, verbose: verbose); @@ -48,6 +52,7 @@ class ResidentWebRunner { final String outputPath; final List fileSystemRoots; final String fileSystemScheme; + final bool soundNullSafety; ResidentCompiler generator; ExpressionCompiler expressionCompiler; @@ -68,6 +73,7 @@ class ResidentWebRunner { packageConfigPath: packageConfigPath, root: root, urlTunneller: urlTunneller, + soundNullSafety: soundNullSafety, ); uri = await devFS.create(); From 041f0095d7c1a159c4b20dc689f8ca8b7e50763a Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 25 Apr 2022 11:40:25 -0700 Subject: [PATCH 5/8] Addressed CR comments --- dwds/lib/src/debugging/metadata/provider.dart | 2 +- dwds/test/build_daemon_callstack_test.dart | 2 +- dwds/test/build_daemon_evaluate_test.dart | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dwds/lib/src/debugging/metadata/provider.dart b/dwds/lib/src/debugging/metadata/provider.dart index 28ccefc95..ad73a74f2 100644 --- a/dwds/lib/src/debugging/metadata/provider.dart +++ b/dwds/lib/src/debugging/metadata/provider.dart @@ -216,7 +216,7 @@ class MetadataProvider { _soundNullSafety = hasSoundNullSafety; } _logger.info('Loaded debug metadata ' - '(${_soundNullSafety ? "" : "no "}sound null safety)'); + '(${_soundNullSafety ? "sound" : "weak"} null safety)'); } }); } diff --git a/dwds/test/build_daemon_callstack_test.dart b/dwds/test/build_daemon_callstack_test.dart index f57245225..7241b54c8 100644 --- a/dwds/test/build_daemon_callstack_test.dart +++ b/dwds/test/build_daemon_callstack_test.dart @@ -50,7 +50,7 @@ void main() { var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); var context = setup.context; - group('${soundNullSafety ? "" : "no "}sound null safety |', () { + group('${soundNullSafety ? "sound" : "weak"} null safety |', () { setUpAll(() async { setCurrentLogWriter(debug: debug); await context.setUp(enableExpressionEvaluation: true); diff --git a/dwds/test/build_daemon_evaluate_test.dart b/dwds/test/build_daemon_evaluate_test.dart index 05714db94..f0b0835fe 100644 --- a/dwds/test/build_daemon_evaluate_test.dart +++ b/dwds/test/build_daemon_evaluate_test.dart @@ -55,7 +55,7 @@ void main() async { for (var soundNullSafety in [/*false, */ true]) { var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); var context = setup.context; - group('${soundNullSafety ? "" : "no "}sound null safety', () { + group('${soundNullSafety ? "sound" : "weak"} null safety', () { Future onBreakPoint(String isolate, ScriptRef script, String breakPointId, Future Function() body) async { Breakpoint bp; From fbcb39326280f7766d631a0e697ddc74a8e33c5e Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 25 Apr 2022 11:42:16 -0700 Subject: [PATCH 6/8] Enable accidentally disabled tests --- dwds/test/build_daemon_evaluate_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/test/build_daemon_evaluate_test.dart b/dwds/test/build_daemon_evaluate_test.dart index f0b0835fe..7f22cf779 100644 --- a/dwds/test/build_daemon_evaluate_test.dart +++ b/dwds/test/build_daemon_evaluate_test.dart @@ -52,7 +52,7 @@ void main() async { // regardless of the logger settings. var verboseCompiler = false; - for (var soundNullSafety in [/*false, */ true]) { + for (var soundNullSafety in [false, true]) { var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); var context = setup.context; group('${soundNullSafety ? "sound" : "weak"} null safety', () { From e7e029cd11b040fb670394c84908784936d04d8b Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 25 Apr 2022 12:08:43 -0700 Subject: [PATCH 7/8] Format --- dwds/test/build_daemon_evaluate_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/test/build_daemon_evaluate_test.dart b/dwds/test/build_daemon_evaluate_test.dart index 7f22cf779..8c4b94b05 100644 --- a/dwds/test/build_daemon_evaluate_test.dart +++ b/dwds/test/build_daemon_evaluate_test.dart @@ -52,7 +52,7 @@ void main() async { // regardless of the logger settings. var verboseCompiler = false; - for (var soundNullSafety in [false, true]) { + for (var soundNullSafety in [false, true]) { var setup = soundNullSafety ? TestSetup.sound() : TestSetup.unsound(); var context = setup.context; group('${soundNullSafety ? "sound" : "weak"} null safety', () { From f06a5c1ac528f2388c029824090cd1dda878b59e Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 25 Apr 2022 13:32:16 -0700 Subject: [PATCH 8/8] Fix failing tests --- dwds/test/build_daemon_evaluate_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/test/build_daemon_evaluate_test.dart b/dwds/test/build_daemon_evaluate_test.dart index 8c4b94b05..0f94c6dbc 100644 --- a/dwds/test/build_daemon_evaluate_test.dart +++ b/dwds/test/build_daemon_evaluate_test.dart @@ -109,7 +109,7 @@ void main() async { var testPackage = soundNullSafety ? '_test_package_sound' : '_test_package'; - var test = soundNullSafety ? '_test_sound' : '_package'; + var test = soundNullSafety ? '_test_sound' : '_test'; mainScript = scripts.scripts .firstWhere((each) => each.uri.contains('main.dart')); testLibraryScript = scripts.scripts.firstWhere((each) =>