⌘-click on link in test output doesn't go to location in file.

[gspencergoog]

Description

When I run a test and it fails I get output that includes the absolute path to the file and the line number and character of the failure, and while VSCode detects the file:/// URI as a link, and I can ⌘-click on it, it either fails and gives me an error dialog that says something like "Unable to open 'menu_bar_test.dart:205:7'" because it doesn't know about a file with a filename that ends in ":205:7" (unsurprisingly), or (if I click on the other link in the output that is only the filename followed by "line 205", it doesn't go anywhere (because it's already got that file open).

This issue also appears to happen on Linux, so I don't think it's macOS specific.

To Reproduce

Steps to reproduce the behavior:

1. Run a failing test.
2. In the "inline" failed test result at the test location in the file, ⌘-click on either link for the file location where the test failure occurred.
3. See the failure dialog appear, or that the cursor doesn't move.

Expected behavior

I expected the cursor to jump to the referenced test file location where the error occurred.

Screenshots

Here's what the error output from the test looks like:

flutter doctor -v

[!] Flutter (Channel menu_bar_iv, 3.1.0-0.0.pre.2094, on macOS 12.5 21G72 darwin-arm, locale en)
    • Flutter version 3.1.0-0.0.pre.2094 on channel menu_bar_iv at /Users/gspencer/code/flutter
    ! Upstream repository git@github.com:gspencergoog/flutter.git is not a standard remote.
      Set environment variable "FLUTTER_GIT_URL" to git@github.com:gspencergoog/flutter.git to dismiss this error.
    • Framework revision 1f76314471 (21 minutes ago), 2022-08-04 11:06:24 -0700
    • Engine revision 51296a62d9
    • Dart version 2.19.0 (build 2.19.0-58.0.dev)
    • DevTools version 2.16.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to
      perform update checks and upgrades.
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)

• Android SDK at /Users/gspencer/Library/Android/sdk

• Platform android-33, build-tools 33.0.0

• ANDROID_HOME = /Users/gspencer/Library/Android/sdk

• Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java

• Java version OpenJDK Runtime Environment (build 11.0.12+0-b1504.28-7817840)

• All Android licenses accepted.
[✓] Xcode - develop for iOS and macOS (Xcode 13.4.1)

• Xcode at /Applications/Xcode.app/Contents/Developer

• Build 13F100

• CocoaPods version 1.11.3
[✓] Chrome - develop for the web

• Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome
[✓] VS Code (version 1.69.2)

• VS Code at /Applications/Visual Studio Code.app/Contents

• Flutter extension version 3.46.0

• Target device (if the issue relates to Flutter debugging): macOS desktop

[adilwahla]

@gspencergoog Hey share error in text form here

[gspencergoog]

The error text is kind of irrelevant: the error is expected, and not produced by Dart-Code (it's produced by running a unit test). It's the result of ⌘-clicking on the error location in the text that I'm concerned with.

Nevertheless, here's the error text:

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: Rect:<Rect.fromLTRB(486.0, 73.0, 640.0, 817.0)>
  Actual: Rect:<Rect.fromLTRB(486.0, 73.0, 640.0, 87.0)>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure>.<anonymous closure> (file:///Users/gspencer/code/flutter/packages/flutter/test/material/menu_bar_test.dart:205:7)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///Users/gspencer/code/flutter/packages/flutter/test/material/menu_bar_test.dart line 205
The test description was:
  geometry with RTL direction
════════════════════════════════════════════════════════════════════════════════════════════════════

[DanTup]

This is an unfortunate combination of a VS Code limitation and Dart/Flutter's stack trace format:

microsoft/vscode#150702

When VS Code parses a file:/// URI, it only handles line/column numbers in some specific formats, and otherwise treats them as part of the filename. Please 👍 that issue :)

I'm planning to file an issue about allowing us to have custom link detectors in places like this (as we do in the terminal), because then we could handle package: links too (and may allow us to handler this ourselves if VS Code does not). I'll post a link back to that once I have.

[gspencergoog]

Can we change the output format for Flutter's stack traces so that VSCode (and hopefully still other IDEs) can parse the links properly?

[DanTup]

We discussed this a little yesterday. Changing the format may be an option, but is potentially breaking (for ex. for IntelliJ that may already parse this format) and doing so only for Flutter may leave the problem for other Dart.

Being able to run custom link detectors is probably the ideal solution because we can also handle things like package: and dart: but if that's not possible (or there's at least no movement in the near-term) we should probably consider it. One option is to adopt one of the formats VS Code does support for URIs (like file:///foo.dart#12,34) or swap the file:// URIs to just standard file paths (where VS Code is a little more lenient in its parsing).

[gspencergoog]

Yeah, custom link detection seems ideal. Would IntelliJ already allow custom link detection? If it does, then we can switch the output format to the hash version and custom parse it in IntelliJ, and it'll work in both places then.

[gspencergoog]

When our test framework is writing the output to a port, does it know from the port negotiation what destination plugin/version it is talking to? If so, we could customize the output format for the plugin it is talking to.

[DanTup]

I filed microsoft/vscode#157500 about having custom link providers in the test peek output (please add 👍 s!).

Would IntelliJ already allow custom link detection? If it does, then we can switch the output format to the hash version and custom parse it in IntelliJ, and it'll work in both places then.

I would expect so, but @jwren might know for sure.

When our test framework is writing the output to a port, does it know from the port negotiation what destination plugin/version it is talking to? If so, we could customize the output format for the plugin it is talking to.

Possibly not reliably today, but there's no reason we couldn't pass a flag (or set an env var) to make that explicit.

[gspencergoog]

Any update on this? I wish for it every time I run a test that fails. :-)

[DanTup]

Unfortunately not. My request for custom link providers (microsoft/vscode#157500) was originally closed as a duplicate and only recently re-opened.

I suggest we give it a little longer (I have pinged enquiring if it might be something they're likely to do soon) if not, we may want to consider going ahead with changing the output in some way to be a format VS Code already understands (which I don't think is a great solution, but perhaps we can do by just regexing in the VS Code extension at the point of sending the test output to VS Code which would minimise the impact elsewhere and avoid changing Dart/Flutter).

[DanTup]

I'm now rewriting these when we pass them to VS Code (for the test output - the Debug Console will remain the same). Note that this only works for file:// URIs (we can't handle package: URIs without microsoft/vscode#157500).

However, if handling package: URIs would be of significant benefit, we could try regexing them out and resolving them to file:// URIs in the same step. It would make the stack traces much more verbose (because in most cases they would be huge PubCache paths) so I'm not sure it's a good route, but I'm open to opinions from others that are dealing with this dialog more than I.

[DanTup]

Turns out this is supported in VS Code, but with a DocumentLinkProvider and scheme "vscode-test-data:". We can't directly reuse the TerminalLinkProvider, but the implementation is easy to use.

I'll redo this before the release. Unlike the rewriting noted above, this will support package: URIs too, and won't show the user the odd #line:col format in the peek window.

[DanTup]

With these latest changes, file:// URIs work as expected (I've also added support for "file://foo line x" which seems to come up in this output):

And also package: URIs (at least, those that we were able to resolve from the editor by looking in .dart_tool/package_config.json):

(note: all that "SetMark" markup is a VS Code bug and not something we can fix directly here 😞)

[gspencergoog]

Yay! I'm so excited.

[DanTup]

😄

FWIW the release went out a few hours ago (to both stable and pre-release versions) so as long as VS Code has updated (and if required, you've "reloaded") you should have the fixes. Let me know if you spot any issues!