feat: Handle vitest dependency in workspace package

[bjornsnoen]

In a monorepo where tests are managed in a package, the root package.json is unlikely to have vitest as a dependency.
This handles that case by parsing each workspace's package.json looking for a vitest dependency.

[marilari88]

Hi @bjornsnoen! Thank you once again for your contribution. I truly appreciate it. Could you please provide more details on your intentions with this PR? I'm a bit confused. Your modifications seem to solely impact the adapter.is_test_file, yet you're testing the adapter.root. Additionally, the spec-monorepo-no-root-dep doesn't have any vitest dependencies listed in both packages/apps package.json, and it's not utilized for testing in any scenario. Nevertheless, if am not wrong monorepos lacking vitest dependencies in the root should already be supported. Do you have a monorepo example that I can check it out?

[bjornsnoen]

Hi @bjornsnoen! Thank you once again for your contribution. I truly appreciate it. Could you please provide more details on your intentions with this PR? I'm a bit confused. Your modifications seem to solely impact the adapter.is_test_file, yet you're testing the adapter.root. Additionally, the spec-monorepo-no-root-dep doesn't have any vitest dependencies listed in both packages/apps package.json, and it's not utilized for testing in any scenario. Nevertheless, if am not wrong monorepos lacking vitest dependencies in the root should already be supported. Do you have a monorepo example that I can check it out?

Indeed, the test is not working as intended. I wrote the code first and made sure it worked for my use case, then I added the test, but the test is certainly wrong. Our monorepo is private but I'll set up one that I can hopefully reproduce the original issue with later today.

[bjornsnoen]

One of the problems is with the test suite. In tests/init_spec.vim the plugin is required before the calls to vim.api.nvim_set_current_dir, which means the rootPackageJson field in lua/neotest-vitest/init.lua is plugin-src-dir/package.json and not plugin-src-dir/spec-dir/package.json. This works as one would expect in most real life cases because vim would be started in the project root, but for the tests it is not working as intended. We must get around this by not relying on a module level field.

I'll see about rewriting that part but I'm not usually writing lua so it might not end up being the cleanest solution possible.

[bjornsnoen]

Note that the relative path here was wrong due to the nvim_set_current_dir. It was causing match_root_pattern to look for wholepath/spec-monorepo/spec-monorepo/packages/example/basic.test.ts, and by luck it resolved upwards until it hit spec-monorepo which has vitest, but would miss wholepath/spec-monorepo/packages/example/package.json which might have had vitest.

[bjornsnoen]

PR updated. Test spec now includes a monorepo that would previously fail to detect that vitest was available.

[marilari88]

Can you explain to me the reason you need this file listed in the summary even though it does not contain tests run by Vite?

[marilari88]

@bjornsnoen I apologize, but I'm still having trouble grasping the initial issue. After testing the spec_monorepo_no_root_dep with the latest production version of neotest-vitest (branch main), it appears to function correctly for me. The plugin.is_test_file function is executed against each file, and within this function, the adapter already scans for vitest dependencies in the nearest parent and root directory. Consequently, only tests within the package containing vitest are matched. I believe this aligns with the expected behavior.

[bjornsnoen]

@bjornsnoen I apologize, but I'm still having trouble grasping the initial issue. After testing the spec_monorepo_no_root_dep with the latest production version of neotest-vitest (branch main), it appears to function correctly for me. The plugin.is_test_file function is executed against each file, and within this function, the adapter already scans for vitest dependencies in the nearest parent and root directory. Consequently, only tests within the package containing vitest are matched. I believe this aligns with the expected behavior.

Yes as you say it checks the nearest parent and root directory but not neighboring workspaces. If you ran yarn install from the root package, you would have vitest available project wide, but this plugin would not let you run test in neighbor workspaces because it thinks vitest isn't installed, as it is not required by the neighboring package or the root package.
As the test I added shows, after the changes in this PR are added, you can run the test in the example-no-vitest package even though neither it nor the root package have vitest, but the example package does.

[marilari88]

Why don't you have vitest in the neighbor package if you need it to run test? It sounds weird to me

[bjornsnoen]

Why don't you have vitest in the neighbor package if you need it to run test? It sounds weird to me

This way I have all test deps declared in one package instead of many, and I have test scaffolding code in that logical grouping. It also makes it much easier to upgrade test deps because I don't have to make sure all the workspaces agree on a version. It may not be to everyone's taste, but it is a way you can use vitest that is not currently supported by this plugin.

[marilari88]

Hi @bjornsnoen,

I believe your changes could potentially have unintended consequences for other monorepos. I'm particularly concerned about cases where different test runners are used, but there may be other scenarios to consider as well. I wouldn't recommend leaving is_test_file as you've modified it as the default behavior. Perhaps we should explore other ways to address the problem.

I understand that you have a package, let's call it X, containing vitest installed, and all other packages/apps have X listed in their package.json as a devDependency. So, why don't we introduce a new configuration parameter to specify X as an additional dependency to search for? For example, we could add a parameter called vitest_wrap_dependency. This approach would give users more control and flexibility, allowing them to define their own dependencies to consider when identifying test files. It also reduces the risk of unintended side effects in other monorepos.

Let me know what you think about this suggestion.

[bjornsnoen]

Hi @marilari88
That's a fair concern that I hadn't considered. Your understanding of the setup we have is not completely right, the other packages don't have any dependency on the package X, so that solution will not work.

An alternative would be to try to identify conflicts, but that would mean knowing about all other test frameworks and would be vulnerable to new frameworks coming out. As I see it we're left with three options:

1. Plugin configuration to optionally globally enable the code in this PR
2. Project configuration via some known file
3. Project configuration via environment variable

I have no idea how well supported environment variables are in neovim plugins, but that would be my go-to solution here. I'm fully open to other suggestions.