jfpedroza / neotest-elixir

Neotest adapter for Elixir
MIT License
38 stars 10 forks source link

make root directory more specific #7

Closed ajayvigneshk closed 1 year ago

ajayvigneshk commented 1 year ago

When the entire suite is run, the is_test_file is called for a lot of files outside the test, like _build and other unnecessary files. This leads to a very high CPU utilization at the start of the run of a suite. As per mix docs, the default pattern is test/**/*_test.exs. So in the PR, I've made the root as test dir within mix root. NOTE: If the suite is run with some test file loaded in a buffer, neotest still does some "discovery" causing is_test_file to be called for all files(To be more precise, even then there's a lesser number of calls to is_test_file, since there're duplicate calls). There's an issue that seems to be about addressing this. But if the suite is run without any test file loaded on buffer, we can clearly see calls to is_test_file from only those within test directory

jfpedroza commented 1 year ago

Makes sense. For the issue there is some progress in https://github.com/nvim-neotest/neotest/pull/100 but proposed dirs option is not there so this is probably the only way to narrow the searched files.

jfpedroza commented 1 year ago

With this now you have to do lua require("neotest").run.run("test") instead of lua require("neotest").run.run(vim.fn.getcwd()), which is the default for the Neotest command 🤔

Running the other command returns no test files.

ajayvigneshk commented 1 year ago

That's weird. Let me check this. Can you share the commands and their mappings that you use?

jfpedroza commented 1 year ago
command! NeotestSummary lua require("neotest").summary.toggle()
command! NeotestFile lua require("neotest").run.run(vim.fn.expand("%"))
command! Neotest lua require("neotest").run.run(vim.fn.getcwd())
command! NeotestNearest lua require("neotest").run.run()
command! NeotestAttach lua require("neotest").run.attach()
command! NeotestOutput lua require("neotest").output.open()

I copied them from the issue template in the Neotest repo.

ajayvigneshk commented 1 year ago
arg /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests
mix_root /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests
return /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests/test
is_test_file /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests/mix.exs false
is_test_file /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests/test/manual_test.exs true
is_test_file /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests/test/neotest_elixir_tests_test.exs true
is_test_file /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests/test/test_helper.exs false
is_test_file /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests/mix.exs false
is_test_file /Users/ajayvigneshk/personal/projects/vim_plugins/neotest_elixir_tests false
No tests found

Added some debugging console statements and the results are a bit weird. It clearly finds certain test files (with true), but says No tests found. This is for Neotest command

jfpedroza commented 1 year ago

I believe it says no tests found because the project root is not part of the tree. The tree starts in the test directory.

ajayvigneshk commented 1 year ago

Yeah that's my suspicion too. But it's weird because the root is a directory and not a file. So is_test_file being called for it seems incorrect. And on master, is_test_file is not called for the root directory (actual project root)

jfpedroza commented 1 year ago

The docs for run.run() say in receives a position, and directories are also position, if they aren't part of the tree, nothing will be executed.

I just saw there is a suite option that maybe is what should be used. Haven't tested it yet.

ajayvigneshk commented 1 year ago

While raising the PR, I tried the suite option only, not the command that passes in cwd. But I think the ability to run tests in a directory is also important

ajayvigneshk commented 1 year ago

Just for the record, marking is_test_file as true for the project root dir also doesn't help 🤷🏻‍♂️ .

ajayvigneshk commented 1 year ago

After checking the neotest repository, it seems like numerous files invoking is_test_file is not an issue with respect to the high CPU / lag when running the entire test suite. So this PR would not be very useful

jfpedroza commented 1 year ago

Also, with the new project config feature, the root of the tree is used for configuration, so you would have to do setup_project("/some/project/test", ...), which is weird.

ajayvigneshk commented 1 year ago

Yeah, that'll be weird. The interface says

---@return string | nil @Absolute root dir of test suite

for function NeotestAdapter.root(dir) end

I think it would be nice to distinguish project root and test root.