swellaby/vscode-rust-test-adapter

Incorrect additional tests being executed when triggering run for suite

Closed this issue · 0 comments

Currently, libtest supports filtering tests by either exact test spec matches (via --exact) or the default mode which is a "contains".

https://github.com/rust-lang/libtest/blob/dbf328db62eacebe1aa719fa806c729bafa319b3/libtest/lib.rs#L1378-L1396

When a user triggers a test run from a suite node in the test adapter view, we currently invoke cargo test with the associated args to run all the test cases under that suite tree since libtest only supports a single arg value for the test spec.

For example if the test mod name (and associated suite label in the tree) is foo, running the foo suite issues a command something like:

cargo test -p mypacakge --lib -- foo::

We do not individually invoke cargo test for each individual test case. We experimented with that approach early on, but found that launching a cargo command for each individual test case was exceptionally resource intensive to the point of making VS Code/the machine unusable.

The challenge with the current approach is that due to libtests filtering approach, any tests that contain that value anywhere in the full test path will be executed.

As an example, if the test listing for a workspace is like this:

test foo::test_one
test foo::test_two
test bar::foo::test_baz
test other::test_add

Triggering a run for the foo suite/node would result in the first three of those tests being executed which is undesired/unexpected (this happens because the bar::foo::test_baz test does contain foo::).

Long term/strategically, the best way to address this would be upstream in libtest to either have it support some additional filtering options (like startsWith or supporting Regex). However, we'll still need a tactical interim workaround because even if such a libtest addition would be accepted, it'd be a long time before the solution could be developed and then released and then aged enough for the Rust Test Adapter to assume that patched version of libtest exists/is available for the majority of users.

Some potential work around options I can think of:

  1. Keep the test execution piece in the adapter as-is, but modify the parsing/updating of the test nodes in the tree to filter out the unexpected tests (bar::foo::test_baz from the example above). This is rather wasteful but probably the easiest to implement.
  2. Enhance the tree construction process with a new phase to identitfy the various --skip args that would be needed for each suite node in the tree to ensure the correct cargo test command can be constructed. This would probably be the "right" solution as far as continuing to let libtest handle most of the work while ensuring that we only trigger the exection of the correct tests. However, it'd definitely add some load to the initial tree construction process as each test suite node would have to be visited after the initial tree was built.
  3. Revist the test execution strategy and reconsider executing test cases one by one, perhaps with some throttling/batching to minimize resource utilization.