matepek / vscode-catch2-test-adapter

Catch2, Google Test and doctest Adapter for the VSCode
https://marketplace.visualstudio.com/items?itemName=matepek.vscode-catch2-test-adapter
MIT License
210 stars 52 forks source link

Should files.watcherExclude be honored when finding tests? #426

Closed geertj closed 8 months ago

geertj commented 9 months ago

This is a new issue that is a follow-up of #420. I added my comment there, but that issue is closed and I cannot re-open it. My assumption is that folks are not seeing the update there.

The context of #420 is that files.watcherExclude is taken into account when finding tests since 4.8.0. I did some more investigation and it appears there are actually two settings: files.watcherExclude and files.exclude. On 4.7.0, both are ignored when finding tests. Since 4.8.0, both settings must not contain a pattern that matches your test executables, otherwise, they are not found.

In our CMake use case, we have a build/ symlink in each package root that points to a directory structure containing the build artifacts, including test binaries. This is a standard used by our internal build system. I'd prefer to not show the build/ directory in the file browser. Also, we have an in-house VSCode plugin that adds the build/ directory to files.watcherExclude automatically every time that plugin is updated, so even if I remove it, it will come back. In our case, the build/ directory is usually small enough to be watchable (we don't use a monorepo), and it is the only place where our tests are built. Ideally, there would be a way for TestMate to scan this directory.

The context for the change in behavior seems to be #414. As I understand it, the use case there is that the build directory is too large to be scanned, and the solution is to copy the tests to a different directory that is much smaller and is scannable. The user then points test.executables to that smaller directory instead, while presumably keeping the build directory excluded. What is not clear to me is why TestMate has to honor files.exclude for this use case, since test.executables doesn't point to a large directory anymore?

I hope this description of our use case was helpful.

troettge-cognex commented 8 months ago

IMHO, this is a very valid follow-up on the mentioned change.

In our project, we have a similar situation like described by @geertj, but in addition our build directory is in fact not so small that it should be added to the watcher. However, as soon as you add build to files.watcherExclude, you now have no chance of reaching the test executables anymore. One option that I could see is if files.watcherExclude had an option to define a general exclude=true rule and provide a more specific exclude=false rule that would bring back the file watching mechanism for files matching this more specific pattern. Based on experiments, this doesn't seem to work, though.

With the current implementation, we are forced to remove our files.watcherExclude statement for the build folder and result in resource and performance issues in VSCode in general.

matepek commented 8 months ago

I see. btw one can easy downgrade the extension to a lower version in case..

matepek commented 8 months ago

I had to realise changing the default behaviour was a mistake. v4.10.0 reverts v4.8.0: files.watcherExclude is not applied by default anymore. Added new setting: testMate.test.advancedExecutables -> exclude which can be "files.watcherExclude" or any setting path.

geertj commented 8 months ago

Thank you @matepek!

troettge-cognex commented 8 months ago

I see. btw one can easy downgrade the extension to a lower version in case..

Sure, but then we would be missing out on any other cool new feature. So we'd rather not be stuck with an older version 😄