The `Match.operator[]` does the same thing and is
generally recommended (and shorter).
(I want to deprecate `group` and `groups`)
Tested: Refactoring.
CoreLibraryReviewExempt: Calling equivalent function.
Change-Id: I4c758968ae622fe16b7322be1b29b05b91e7fcd9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489021
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
When running a large test file with `solo: true` on one test, the output looks like:
```
! test name
Skip: does not have "solo"
! test name
Skip: does not have "solo"
! test name
Skip: does not have "solo"
! test name
Skip: does not have "solo"
✓ solo test
! test name
Skip: does not have "solo"
```
While custom skip messages might be useful, when the skip messages are from using solo:true this just makes the output spammy and adds extra noise to scan through for the actual solo test you ran.
This change prevents creating output events for such skip messages (but still sends them for custom messages, since those are likely far less frequent).
The `print` events themselves are still forwarded to the DAP client, so it still has the data for this that it can present elsewhere (for example by showing the test node as skipped, if appropriate).
Fixes part of https://github.com/Dart-Code/Dart-Code/issues/5938
Change-Id: I1d26751207e7b89f77e13be0dfcc74710e7d75ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486780
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This test was flaky because thread-exit events are not guaranteed because the VM might terminate too quickly and we will instead emit a "terminated" event. Rather than trying to guarantee these events (for no benefit), this just makes the test accept the results with or without the text-exited event as long as we still got the terminated event.
Fixes https://github.com/dart-lang/sdk/issues/61826
Change-Id: I0dfbbae3f0d1f41f128ff19aa0f733aeba49f579
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464680
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>
In order for "Copy Value" to work in VS Code's debugger windows, we need to provide an expression for each variable (the "evaluateName"). To carry this down through the child fields we need to store it by the object ID on the server, but we were not doing so for global fields - this resulted in "Copy Value" giving truncated values and "Copy Expression" being missing.
The fix is to ensure we call `storeEvaluateName` for globals too.
Fixes https://github.com/Dart-Code/Dart-Code/issues/5778
Change-Id: I9fed4b5483099341799548aea0225c9cef03d211
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459560
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
A recent change somewhere caused some Dart-Code tests to fail. The expression "DateTime.now().ye" used to fail to compile and resulted in a failed RCP call to the VM service ("Expression compilation error") but now it returns a valid result that is a `vm.ErrorRef`.
This applies the same regex to the error message in this path to extract the most useful part of the error text when in the "watch" context so that the visible part of the message is not just "Unhandled exception".
Fixes https://github.com/Dart-Code/Dart-Code/issues/5759
Change-Id: I2933f9d811ec28446f4d2a857c1fd5bd18c94816
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/456241
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This reverts two commits (in separate patch sets in the CL):
- commit 482a7caed7: [dap] Simplify URI handling in IsolateManager.addBreakpoint
- commit d0a7ef4459: [dds/dap] Add/remove breakpoints as required instead of replacing the whole set
The first revert is to avoid conflicts while reverting the second, and will be reapplied later (likely after the release branch, since it is not critical to include). The second revert is because this change resulted in leaked Script objects in the VM.
There is an additional change (patch set 3) to fix up the changelog/versions so they don't go backwards.
Change-Id: Id55949e1622d3367ced87c9a307c43881b030cbb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440162
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
The DAP APIs for breakpoints send the whole set of breakpoints for a given file at once. Previously, we would just delete all breakpoints and then re-add them all, however since we added support for resolving breakpoints, this can result in all breakpoints in a file flickering to unresolved then back to resolved (as well as generally being slower).
This change splits the method that would replace all breakpoints into methods for add+remove, and then skips over any breakpoints (in setBreakpoints) that already match breakpoints we have.
Fixes https://github.com/Dart-Code/Dart-Code/issues/4678
Change-Id: I489c6c295bac3ebd3a0851895a7a81e330d571e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/433700
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
The change at https://dart-review.googlesource.com/c/sdk/+/410760 to fix a race incorrectly assumed that once we had handled startup for a thread, we would never need to send an automatic resume again. However this was not the case - after a hot reload, we need to resume the thread even though we had technically already handled startup.
This is essentially a partial revert of 84e6ed0784, with a test to verify we trigger readyToResume on PausePostRequest.
Change-Id: I1e171043f04f38dd6f52e1692d9257d34e5248be
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416580
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
The DAP tests have some debug logging that prints if a request/event takes longer than 10 seconds to arrive. In the case where tests run quickly and the DAP client is torn down, these delays cause the test run to stay alive for 10 sec unnecessarily (and print irrelevant warnings).
This changes that code to check periodically instead, and exits early if the client is torn down avoiding both the 10s wait and the spurious warning.
Change-Id: Ic07826942e8ae307867820020b30595a42e68d15
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/411680
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Part of the process to remove uses of
DartDevelopmentService.startDartDevelopmentService in favor of launching
with `dart development-service`.
Change-Id: I857aee932c0b2fc6ae2172eb106b78c76d536295
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380760
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Capturing debug logs that includes VM Service traffic is useful for debugging. Previously with the new DAPs, this could only be enabled at session start which meant users would have to capture very large log files.
This adds the ability to enable/disable this logging while a debug session is already running. A related change in Dart-Code will send this request when you use the "Capture Debugging Logs" command, so you can start a debug session, get to the point of reproducing an issue, and then just capture the relevant logs (VM Service traffic included).
Fixes https://github.com/Dart-Code/Dart-Code/issues/5197
Change-Id: I7e766e57bd80819eb5da164e544903fae23927ed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/377222
Commit-Queue: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
When a class contains both a field and getter with the same name, we'd include both in the `variables` response. In most cases these are the same value (although it's not guaranteed).
I've chosen to just hide the field in this case and always show the result from evaluating the getter (since I think that's what the user would expect, even in the case where they happen to have different values). Another option could be to show both (but change the name so that fields/getters are shown differently), however in that would change the display (for example adding `get ` in front of all getters) we should probably only do that if it's clear there is demand for it.
Fixes https://github.com/Dart-Code/Dart-Code/issues/5128
Change-Id: I9e23d22a844ee22c38988456b1f275422c5c9e04
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370640
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This test shows up as flaky quite often. It's waiting for breakpoint Resolved events by just calling `await pumpEventQueue(times: 5000);` but it appears this sometimes doesn't get all of the events.
This change waits up to 5s for the events to arrive (because we unfortunately have no signal to know when they should have come through). I'm hopeful 5s is more than enough and this won't flake, but we could increase this as not - the goal of this test is not to verify performance but just ensure the events do arrive.
Change-Id: Ia2236b65bbb5d11e2a5519d49869c6b87202a940
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367301
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Sometimes we see unhandled exceptions during this setup work, see:
- https://github.com/flutter/flutter/issues/148346
- https://github.com/flutter/flutter/issues/148348
Currently these are unhandled and bring the debug adapter down. In VS Code, the error (send to stderr) is not visible to the user so they just see a silent crash which makes it very difficult to report (and the Flutter crash reports don't have any context).
This change will send the exception to the client and then cleanly terminate, which should give the user more information to open a good bug report (assuming the issue wasn't just something like them terminating the app as it was starting).
Change-Id: I4aefbc278e6a0708924c6fa41c5179d581117689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366662
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
In a recent change I wrapped these exceptions to provide better failure messages in tests, but I missed updating these tests that had matchers against them.
For reasons I don't yet understand, the trybots that ran on that change didn't include these tests so didn't catch this.
Change-Id: I5227842d2317bbe33186670141586c315f98a5c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366665
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Helin Shiah <helinx@google.com>
getObject and invoke would `throw` if the response was a sentinel and that wasn't handled correctly, which meant sometimes an entire variables request would fail instead of only the individual values (eg. in a map/list) being shown as erroring.
This improves https://github.com/dart-lang/sdk/issues/54897 (you'll no longer see the entire request fail), but it still needs paging adding too.
Change-Id: Ic3ed3bee7c1dd647ebae843007bd30b0e1fda590
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364860
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Jake Macdonald