Provide a better way to match up (and run?) tests using the analyzer outline info (literal strings for test/group names)

[DanTup]

I'm not sure there's a good solution here, but I thought it was worth starting a discussion.

-- wall of text for some background --

As part of migrating Dart-Code to use the new VS Code test runner, I hit some issues with some functionality like "Run Test at Cursor". In the original custom test runner, when a user invoked this command I would look up the analysis servers Outline node for the current cursor position, and if it was a Test or Group, extract the test name string, naively replace any ${?foo}? with .* and use that as a regex to run the test.

For the new VS Code test runner, the Run Test at Cursor command is handled by VS Code and I can't do this. Instead, I have to give it all of the test Ranges up-front when discovering the tests. On its own, this is isn't difficult - I walk the analyzers Outline node and produce all of the tests with their ranges.

The problem comes when users start running tests that have dynamic names. I can't just throw the tree away and rebuild from the analyzers Outline info as the dynamic test nodes (and the ability to see their results) would disappear. In the old test runner, I would therefore only use the Outline info once, then always use the test results (with some logic to handle partial runs that resulted in not removing deleted tests from the tree unless you ran the whole file).

So the new solution requires merging both the tree from the analyzer and the results from running tests, including rebuilding nodes from the analyzer as new Outlines arrive. For any test I discover (from the analyzer or test results) I tag where it came from and I only ever remove nodes from the source being rebuilt (eg. new Outlines can only remove nodes tagged with the Outline source, not Results, and vice-versa). Dynamic nodes are now nested under their analyzer nodes so that it's clearer where they came from (and so that if the source test is removed from the file, the results disappear with them):

This solution is all mostly working, but it has some issues:

• Nesting the test results under the analyzer node requires is "guessing" which test results are actually for that test (and this involves turning the analyzers test name into a regex and then matching it against the node). This is the same regex we build when trying to run-test-at-cursor.. It mostly works, but I'm certain there are edge cases that could easily break it
• The Ranges of dynamic test nodes are not correctly maintained because when a dynamic test is nested (eg. the name of a parent group is not constant) the test will be nested under the dynamic group and not the original analyzer node

---

So the request is for a reliable way to match a test up in a set of results with the Outline node in the analyzer that represented it. I don't know if this would be best done with changes in package:test or the analysis server.

One thought I had, was having the test reporter include the literal string from the source file for each test/group ("literalName"?). I suspect that's not trivial (since it's not available at runtime), although @jacob314 mentioned it could be possible with a kernel transformer or something (something I'm not particularly familiar with).

Although it's possible we could use the locations of the tests to do this matching up, it wouldn't have any protection against the file being out of sync with what the test runner ran (we can't be certain we have a snapshot of the outline at the moment the test runner read it from disk), whereas matching names exactly would allow for this (or even allow matching nods up if the location has shifted due to edits).

Slightly related - if the test runner has this info, it could also allow for us to pass a --literal-name "group 1 test $condition" filter which would also remove the need for us to "guess" the right regex to run a test given its outline node.

@jakemac53 @natebosch @bwilkerson for opinions.
@stevemessick I'm not sure what IntelliJ does in this area (or whether it's any better or worse than what I'm currently doing in VS Code)

[jakemac53]

• Nesting the test results under the analyzer node requires is "guessing" which test results are actually for that test (and this involves turning the analyzers test name into a regex and then matching it against the node). This is the same regex we build when trying to run-test-at-cursor.. It mostly works, but I'm certain there are edge cases that could easily break it

Ya you could at least run more tests than you really wanted to, but it probably works most of the time.

• The Ranges of dynamic test nodes are not correctly maintained because when a dynamic test is nested (eg. the name of a parent group is not constant) the test will be nested under the dynamic group and not the original analyzer node

Can you explain that a bit more or show an example? It sounds to me actually like it might be doing the "correct" thing?

One thought I had, was having the test reporter include the literal string from the source file for each test/group ("literalName"?). I suspect that's not trivial (since it's not available at runtime), although @jacob314 mentioned it could be possible with a kernel transformer or something (something I'm not particularly familiar with).

I don't think a kernel transformer is justified here - those come with a very high maintenance cost as well as potential performance cost, and should only be used as a last resort imo. It would likely be possible but probably not trivial to use the analyzer to get this info in the same way VsCode does - but we would have to match up the locations in that case.

Although it's possible we could use the locations of the tests to do this matching up, it wouldn't have any protection against the file being out of sync with what the test runner ran (we can't be certain we have a snapshot of the outline at the moment the test runner read it from disk), whereas matching names exactly would allow for this (or even allow matching nods up if the location has shifted due to edits).

Unless there is a long lag time in terms of getting the updated outline from the analyzer after a save, I would think this shouldn't happen often? There is a general problem here anyways with test names today in any case, although they are probably less likely to change.

[natebosch]

I do think it would be interesting to explore more deeply the idea of using line numbers to match these up. If we collect a StackTrace.current inside our test function, I wonder if we could reliably map back to a line number in the users test file.

[jakemac53]

Ya I agree in general that exploring using line locations seems like a pretty good solution.

[DanTup]

Can you explain that a bit more or show an example? It sounds to me actually like it might be doing the "correct" thing?

With the new changes, if you have a test like "Foo $bar" (where $bar can be "one" or "two" at runtime) then we will insert a node called "Foo $bar" from the analysis outline, and then when we get the test results, we'll figure out that "Foo one" and "Foo two" relate to "Foo $bar" and nest them:

my_test.dart
    Foo $bar       (source: Outline)
        Foo one       (source: Results)
        Foo two       (source: Results)

If the user starts typing in the file, we will maintain the Range of "Foo $bar" in the tree because we match the node up. However we don't maintain the Range against its children, so clicking on them will jump to the old location. Although, now I'm thinking it might be completely safe (or at least, mostly) that when we update the Range of an Outline-source node and it has Results-source children with the same range, we should just update them all the same.

I do think it would be interesting to explore more deeply the idea of using line numbers to match these up. If we collect a StackTrace.current inside our test function, I wonder if we could reliably map back to a line number in the users test file.

Do you mean for VS Code to match the results back up to nodes?

We actually do already get line numbers back from package:test (actually we get two, one is where test() was called, and one is the first frame in the suite file (eg. so that testWidgets() in Flutter doesn't try to jump the user into the Flutter framework when they want to navigate to their test). We could use that and make some assumptions (for example, locate the Outline node on the same line), but it does come with some edge cases (like if the data is out of date or - very unlikely - there were multiple tests on the same line).

To be clear, I think the things above work for the huge majority of cases. I'm just trying to avoid some edge cases. For example, doing the above, we still have the situation where the users file looks like (this is an exaggerated example):

group('my $foo group $bar', // ...
test('my $foo test $bar', // ...

And they click the "Run" CodeLens above the test(), and from the Outline information, we have to figure out the correct regex to send to package:test for it to run the test they picked. To a user it seems obvious that it should just run exactly that test, but VS Code is now constructing a regex by replacing $xxx with .* to get "my .* group .* my .* test .*" and while this generally works, there's a whole host of things that could be following a $ that can trip this up (for example ${"}"}).

For a user running a test at a command line, using the resolved test name seems completely reasonable. But when they want to point at a test in the the source code and say "run that", it doesn't work so well.

If the line number suggestion above was actually to allow us to say "run the test on line 5" or "run the group on line 3", then that might not bad a bad idea 🤔

[jakemac53]

Do you mean for VS Code to match the results back up to nodes?

This came up a bit in one of the other issues - what I mean is actually allowing you to run a test based on its location in the file instead of by name - something like --test-location <test_file>#<line>:<column> (or maybe the file name is passed separately). The column could be optional. It would probably correspond to the location in the users test file (I think we refer to this as the "root" location?). We could match it against any line in the stack trace even though.

[DanTup]

Gotcha! I do think that could work. It would be important that we keep all the ranges of the test nodes up-to-date in the tree correctly to use this though, as if we send the wrong locations it's more likely to do unexpected things than when using names.

If it's possible to support multiple suites are once, it probably makes sense to keep the file/line/col together. Something VS Code's new test runner supports but we don't handle great is selecting many tests and trying to run them all. Right now we group them by suite and enumerate them spawning a debug session for each suite (sequentially), but it'd be nice if we did them all in one (it would avoid paying startup costs for some things multiple times).

[jakemac53]

It would be important that we keep all the ranges of the test nodes up-to-date in the tree correctly to use this though, as if we send the wrong locations it's more likely to do unexpected things than when using names.

Yes, there would be this risk, but I think it should be minimal. We could try and report a good error too - since you are specifying an exact place you expect a test to be, if there isn't one there we can fail. We might still run the wrong test if a test exists at that location but is different from the one you expected to run, but at least it would report the test that did run correctly?

Ultimately the scale of the risk here depends on how quickly you get new outlines from the analysis server probably.

If it's possible to support multiple suites are once, it probably makes sense to keep the file/line/col together.

Ya I think it should be easily doable, and I agree with the advantages you outlined.