Repository navigation
⌘-click on link in test output doesn't go to location in file. #4089
Description
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:
- Run a failing test.
- 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.
- 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 [email protected]:gspencergoog/flutter.git is not a standard remote.
Set environment variable "FLUTTER_GIT_URL" to [email protected]: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
Activity
adilwahla commentedon Aug 4, 2022
@gspencergoog Hey share error in text form here
gspencergoog commentedon Aug 4, 2022
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:
DanTup commentedon Aug 4, 2022
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 commentedon Aug 4, 2022
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 commentedon Aug 4, 2022
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:anddart: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 (likefile:///foo.dart#12,34) or swap thefile://URIs to just standard file paths (where VS Code is a little more lenient in its parsing).gspencergoog commentedon Aug 4, 2022
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 commentedon Aug 4, 2022
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 commentedon Aug 8, 2022
I filed microsoft/vscode#157500 about having custom link providers in the test peek output (please add 👍 s!).
I would expect so, but @jwren might know for sure.
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 commentedon Nov 7, 2022
Any update on this? I wish for it every time I run a test that fails. :-)
DanTup commentedon Nov 8, 2022
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 commentedon Nov 14, 2022
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 handlepackage:URIs without microsoft/vscode#157500).However, if handling
package:URIs would be of significant benefit, we could try regexing them out and resolving them tofile://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 commentedon Nov 29, 2022
Turns out this is supported in VS Code, but with a
DocumentLinkProviderand scheme "vscode-test-data:". We can't directly reuse theTerminalLinkProvider, 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:colformat in the peek window.DanTup commentedon Nov 29, 2022
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 commentedon Nov 29, 2022
Yay! I'm so excited.
DanTup commentedon Nov 29, 2022
😄
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!
Rewrite URI line/cols for test output
Use terminal link providers also as document link providers for 'vsco…