Failures on "Remove spawning of DDS from DAP and let the spawned `dart run`/`dart test` do it"

[derekxu16]

There are new test failures on Remove spawning of DDS from DAP and let the spawned dart run/dart test do it.

The tests

pkg/dds/test/dap/integration/debug_breakpoints_test Timeout (expected Pass)
pkg/dds/test/dap/integration/debug_eval_test RuntimeError (expected Pass)
pkg/dds/test/dap/integration/debug_exceptions_test RuntimeError (expected Pass)
pkg/dds/test/dap/integration/debug_services_test RuntimeError (expected Pass)
pkg/dds/test/dap/integration/debug_stack_test RuntimeError (expected Pass)
pkg/dds/test/dap/integration/debug_variables_test RuntimeError (expected Pass)

are failing on configurations

unittest-asserts-release-win-arm64

cc @DanTup and @bkonyi

[dart-github-bot]

Summary: The commit "Remove spawning of DDS from DAP and let the spawned dart run/dart test do it" introduced test failures in the pkg/dds/test/dap/integration suite on the unittest-asserts-release-win-arm64 configuration. The failing tests cover various debug functionalities like breakpoints, evaluation, exceptions, services, stack, and variables.

[DanTup]

All the tests that expect to pause on breakpoints seem to be pausing on isolate exits:

  Expected: 'breakpoint'
    Actual: 'exit'

Taking a look...

[DanTup]

@derekxu16 @bkonyi I'm failing to reproduce this on my Windows PC (although it's not ARM). Is it possible to trigger Windows ARM trybots on https://dart-review.googlesource.com/c/sdk/+/391965 to get a complete trace of the protocol traffic?

[DanTup]

Looking at the logs, this looks like a drive letter casing mismatch:

file:///c:/opt/s/w/ir/x/t/dart-sdk-dap-test8c56f93c/appd3adfb9c/test_file.dart
file:///C:/opt/s/w/ir/x/t/dart-sdk-dap-test8c56f93c/appd3adfb9c/test_file.dart

When trying to add the breakpoint, we have an uppercase C, but in the scripts list, there's a lowercase C. I can't see anything in the code removed in 97ab888 that would affect the drive letter. @bkonyi is there anything in how the VM spawns DDS that might affect this? I think I've seen lowercase drive letters in the VM script when the SDK was spawned using a lowercase drive letter, but there's nothing different in how we're spawning it here (we do force the drive letter uppercase after reading Platform.resolvedExecutable when the adapter spawns dart run).

[bkonyi]

I don't think DDS should impact the drive letters at all, regardless of how it's spawned. The paths in the scripts list are all provided to the VM by the CFE and are reported without any modification by the VM service.

I wonder if these tests would pass if we disabled DDS? If they don't, then we can rule out DDS being related to the failures.

[DanTup]

@bkonyi good idea! I've pushed a change to disable it to https://dart-review.googlesource.com/c/sdk/+/391965 - could you trigger the Windows ARM trybot again?

[DanTup]

I was able to repro this locally by setting my Temp folder env var to a path with a lowercase drive letter. It repros with and without DDS, but it does not repro without the CL above.

I captured the protocol traffic and the first difference in the logs (that isn't just random tokens/port numbers/etc.) is the response to getVM where the working one has a reasonable path to the target script in the IsolateGroup name, and the other has a relative path that has likely been computed between paths with different drive letter casings:

So my guess is that somewhere there is a path.relative call that may be operating on paths with different casings with/without the change above. I'll see if I can narrow it down.

[DanTup]

@bkonyi after much head scratching, I tracked this down to occurring only when the current working directory had a lowercase drive letter. It's not clear to me how the CL noted above changed the behaviour, but if the working directory has a lowercase drive letter, before this change the VM would handle breakpoints with uppercase drive letters, but after the change above, it would not.

I'm fixing this by having DAP normalize Directory.current to have an uppercase drive letter always (even though there's not strictly any requirement that the working directory is related to the package being run), but FYI in case it rings any bells.