ionide / ionide-vscode-fsharp

VS Code plugin for F# development
http://ionide.io
MIT License
858 stars 277 forks source link

Suffix location matching (Match test locations where test name and code structure differ) #1922

Closed farlee2121 closed 1 year ago

farlee2121 commented 1 year ago

WHAT

More tests have locations

Improve matching of test explorer items to code locations. Specifically, detect tests that are logically grouped by a testList, but not directly located under that same testList in the code.

Example

Before, these tests didn't have features based on code location suffix-match-before

After suffix-match-after

Mislocated tests don't pollute the explorer

Also prevent mislocated tests from showing in the test explorer. The production version already prevented these on initial discovery, but they would still show up if the tests were renamed. Now the incorrect test items shouldn't show at all.

For example, the Not Directly in Parent test should be nested under Composed test lists? and Composed test lists - 2 and is not a root level test.

Before top-level-clutter

After top-level-clutter-solved

HOW

If an explorer test can't be located in code, check if any of the code test ids are a suffix of the explorer test id.

Known Issues

  1. It still has issues locating tests under parents that can't be matched. For example, if the parent test name contains a separator character like + or ..
  2. Test located by this new approach do not get live updates, which could be confusing for users.

Alternative Approaches

I hoped to solve this within the FSAC testDetected endpoint. This would have made it simple to support live explorer updates, and the code analysis results would have been more aligned with the logical structure of the tests.

The basic idea was to try resolving any referenced values within an expecto testList. However, it doesn't look like this can be done with the untyped expression used for detecting tests. Using the typed expressions seemed like a heavy update that could cause performance issues with how frequently this analysis is run.

I didn't measure the performance of using the typed expressions though. It may still be viable. I also may have missed options for resolving symbols in the untyped AST.

Performance

I tested this on large numbers of tests in several repos, since it runs every time the code updates. I didn't observe any slowdowns, but I'm 100% confident this can't cause performance issues.