bazelbuild / vscode-bazel

Bazel support for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=BazelBuild.vscode-bazel
Apache License 2.0
231 stars 76 forks source link

Make test explorer functionality optional #403

Open mnoah1 opened 1 week ago

mnoah1 commented 1 week ago

I see that new functionality for the test explorer has been added to display coverage. Can this functionality be placed behind a setting? We use this Bazel extension primarily for build file syntax highlighting, but are using a different solution for test case execution to better work with our large monorepo setup, and it can result in user confusion when there are too many extensions contributing to the test explorer. Other extensions that contribute to test explorer typically offer either a way to opt out of this functionality via a setting (e.g. Python, Go, etc), or package this functionality in a separate test runner extension (e.g. Java).

vogelsgesang commented 1 week ago

Hi @mnoah1,

Can this functionality be placed behind a setting?

Yes, that is certainly possible. I assume you are using the release versions from Market Place and not a more recent version from master? If so, I will make sure to find some solution before cutting the next release.

it can result in user confusion when there are too many extensions contributing to the test explorer

Can you elaborate a bit more on what exactly leads to user confusion in your case?

I personally already noticed that I am seeing the "Test Explorer" side bar now, even if there are no other test providers, and hence no tests are actually available in the Test Explorer, but I am not sure if you are referring to the same source of confusion?

The reason I am asking is that I am considering an alternative solution: Instead of providing a setting to disable the Test Explorer integration altogether, register the test provider only lazily, i.e. after the first coverage-related command was executed. Would that also solve your issue?

mnoah1 commented 1 week ago

Yes, that is certainly possible. I assume you are using the release versions from Market Place and not a more recent version from master? If so, I will make sure to find some solution before cutting the next release.

Yes, we currently use the version from the marketplace so it's fine for now.

Can you elaborate a bit more on what exactly leads to user confusion in your case?

When there are multiple test providers, context related items (e.g. context menus, run arrows next to tests, "run all tests" commands) will offer the tests contributed by both extensions. Also, in the tree of tests, there will be separate root nodes containing tests from each extension.

So the user has to be a bit more explicit when making their selection, the number of options can become a bit noisy, and if users aren't seeing expected behavior, it can due to clicking the test item contributed by the other extension. We are planning to have our users primarily using this test runner extension that we developed to integrate with the bazel-bsp project from JetBrains, which is similar but allows browsing/running of individual test cases in a target.

The reason I am asking is that I am considering an alternative solution: Instead of providing a setting to disable the Test Explorer integration altogether, register the test provider only lazily, i.e. after the first coverage-related command was executed. Would that also solve your issue?

That could be ok, but I do see some concern where users try to run the coverage command and don't realize it has the side effect of also initializing the test explorer. For larger organizations that provide a managed remote setup to users, having a setting that can just be left disabled helps minimize users accidentally enabling features or not realizing that that they're selecting a functionality that is not a preferred option in their environment.