dart-lang
Provide file/line/col with errors during test runs
I'd like to make some improvements to the "Peek Test Error" popup that VS Code shows when a test fails:
https://github.com/Dart-Code/Dart-Code/issues/4351
One common complaint is that it's only tied to the location of the test, and not the exception. VS Code allows us to provide the location of an error during a test run (which I presume will solve this), but we don't currently have that information from package:test.
It would be good if for exceptions and assertion/expectation failures, we got the file/line/col where the failure occurred, which should allow the user to navigate there more easily from the failure (without needing to run with the debugger and pause during the run).
I think a reasonable place to put this would be on ErrorEvent (perhaps an optional Location that contains these three). I don't know if there might be a case where there are two locations (similar to when we discover tests, to support "helper" methods), for example if a user makes a custom checkFoo() helper, they might expect the failure to lead to the call of that function, and not an expect inside it (if so, perhaps that could be handled similar to the other, where we get both the actual location, and the first stack frame that's in the test suite where the test was executed).
At least for flutter specifically I am skeptical if we can do anything in package:test directly, that looks like very custom output, where they are handling the failure and then creating their own description... we might not even have the info for where it failed?
I am curious what the stackTrace parameter looks like in the JSON for that error, that should tell us some more.
In general, the only info we have is the stack trace, which we are already exposing, so that should give you all the info you need to try and reproduce the original failure line/col, or at least the same amount of information we have. I am not sure that package:test is actually the best place for this logic to live? It is more specific to the IDE I think, and it is generally more flexible if you just do the logic in the plugin.
At least for flutter specifically I am skeptical if we can do anything in package:test directly, that looks like very custom output, where they are handling the failure and then creating their own description... we might not even have the info for where it failed?
Oh yeah, that's a good point. I was thinking of something that works for both Dart/Flutter but that's probably not going to be possible here if Flutter is handling it. We do get structured error info Flutter that we might be able to use to pass info back there, although it would be nice to have similar for non-Flutter too.
In general, the only info we have is the stack trace, which we are already exposing, so that should give you all the info you need to try and reproduce the original failure line/col
Yeah, though the IDE code isn't in Dart, so can't use package:stack_trace so it would be less reliable. We do already try to parse stack traces in a few places, but it turns out they vary quite a bit (across Dart/Flutter, test, web/non-web) so having it down in one place (that can use the official package) would be much more reliable (as well as avoid the work being repeated across multiple tools).
I'm not convinced this is IDE-specific though.. I think any IDE could find the information useful (if it has some way to provide it in a useful way to the user).
It's slightly off-topic, but there's actually some other functionality in this area that we don't currently use (because we don't have structured info from package:test), like being able to tell VS Code what the expected/actual values are in a test expectation. I presume it will render a nice colourful diff (which is much easier to review than a text-based one) although I don't know of any frameworks using it yet. I don't think it's a good solution for Dart-Code to try and parse that info out of the error messages, so having a more structured failure event could be useful there too? (although this might highlight the difference in how Flutter reports its errors even more significant, and maybe we should try to improve that if we wanted to support this 🤔)
Ah yeah not being able to use package:stack_trace is unfortunate. It probably would be easier to implement in package:test then, even if I don't love the logic living there.
It's slightly off-topic, but there's actually some other functionality in this area that we don't currently use (because we don't have structured info from
package:test), like being able to tell VS Code what the expected/actual values are in a test expectation.
This would be cool - and we should be able to provide the actual/expected values (as their string descriptions?). Maybe we should roll out a few of these things all at once?
and we should be able to provide the actual/expected values (as their string descriptions?)
Not easily. We get a TestFailure exception with a String message. The actual and expected string representations are already encoded into the single message. We could consider adding optional "actual" and "expected" arguments to TestFailure - I suspect in both matcher and checks we could populate it.
Yes, that is exactly what I was thinking (add new optional fields to the exceptions we throw for failures)
Cool - well lemme do some playing around with this API soon just to make sure I understand all of its capabilities and how it renders things, and then I can give a list of exactly what data would be useful and what it would give us, and maybe it'll make sense to tackle all together. I'll also dig back into how Flutter's structured errors work to see if we can avoid two completely different implementations because of how it handles the failures itself.
I had a look at how this looks when providing VS Code an expected/actual value. It includes a nested diff editor in the result window:
It's helpful, although it also means the error message is less visible (and in some cases the description built by package:test is helpful). Without the expected/actual it looked like this:
So, I'm less sure about adding expected/actual for now. But improving the location would definitely be good.
That diff view would actually be super helpful for large strings, I consistently have to go use a separate diff viewer because when its a wall of text the current output is completely useless
Oh yeah, that's a good point - long strings get formatted with quotes around them and make it annoying to copy/paste into a test.
This issue was about location though - should I file another issue about collecting expected/actual info or should this one be for both?
(in both cases we need to ensure that data works when running via Flutter :-) )
A separate issue would probably be good, even if we do both at the same time, they can easily be separated