Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterized Tests #57

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

Parameterized Tests #57

wants to merge 31 commits into from

Conversation

gollth
Copy link

@gollth gollth commented Oct 20, 2023

Hi @rouge8

first thanks for this adapter for the great neotest! I'm using it quite intensively. In our projects though, we make heavy use of parameterized tests, so I'm hoping for a solution to #1

After some research how other adapters handle this (e.g. neotest-jest & neotest-python) I tried my luck and implemented a possible way. According to the disucssion in Capture and interpolate dynamic tests there are two preferred ways of how to discover nested/parameterized tests:

  1. Use tree-sitter to parse the syntax tree and find out which parameters belong to which test
  2. Let the adapter hook into the build tool (cargo nextest list in our case) and somehow relate the output back to positions in the test tree. This is how neotest-jest and neotest-python are doing it.

I tried now the first approach and got quite some milage out of it:

test-case rstest
test-case rstest

This is my first ever Lua development, so there is probably a ton wrong with my style. But I wanted to discuss with you first if this approach generally makes sense. Let me know what you think

Discussion

Current Idea:

  • Since in rust the parameters are usually in form of attributes before the actually test function, I think there is no way to parse them in the correct tree structure in one pass. As far as I understood it neotest internally will place subtests only into their parents if their range is a subset of their parents range, which is impossible if the nested test's range is above the actual function.
  • To compensate that, I tried what neotest-python was doing, i.e. a two pass query:
    • First parse tests normally, but capture an additional group parameterized if the test is parameterized at all (i.e. has an #[test_case(...)] or a #[rstest] #[case(...)] macro before it.
    • Then in the second state, for each parameterized test, shoot another tree sitter query, which detects each test case (potentially with its name). For each case, add a new subtest below the parent.
  • Both work with async tests as well with both #[tokio::test] and #[async_std::test] runners

There are many edge cases though, which are hard (impossible?) to overcome with plain treesitter:

  • test-case supports two "modes": named and unnamed. The named one (e.g. #[test_case(some arg ; "test_name")] is relatively easy to parse in tree sitter. However when you don't have a unique case name, test_case builds the name out of the parameters. Also there is no way to distinguish the two cases because in tree sitter at this level everything is only a (token_tree)
  • rstest value lists .... yeah... this one is straight impossible to parse with tree-sitter alone...

@gollth
Copy link
Author

gollth commented Oct 20, 2023

Main reason I went with this approach instead of the custom parsing of cargo nextest list is that I don't know how to correlate case positions back to attributes in the code. Imagine cargo gives me foo::bar::baz and I know there is a test function called foo::bar, I can easily build the tree (I guess?), but I don't know where the location of baz is... This is working quite nicely with tree-sitter, though

@gollth
Copy link
Author

gollth commented Oct 25, 2023

I looked into this again and added the first approach (i.e. calling cargo nextest list and parsing the output to discover the test cases. This has other tradeoffs, though:

  • Test case location can mostly mapped to each test, not the individual macro where each is defined (in contrast to the treesitter approach which is capable of doing so)
  • Requires the project to (re)-compile before test cases can be discovered
  • Is able to detect basically any subtest even those coming from #[rstest::values(...)] and #[rstest::files(...)].

I think it make sense to let the user choose a "strategy" of test case discovery. I included an option now in the setup args or neotest-rust and a table in the README outlining pros and cons of both strategies.

WDYT?

Copy link
Owner

@rouge8 rouge8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really impressive! Unfortunately I'm leaving for a vacation in a couple days so I don't have a ton of time to review at the moment. To make reviewing easier when I get a chance, can you fix the existing tests and add new ones?

@gollth
Copy link
Author

gollth commented Oct 25, 2023

Thanks =) Sure, I'll have a look at the tests in the next days.

@gollth gollth force-pushed the param branch 3 times, most recently from ba0aa02 to 394bdd0 Compare October 29, 2023 10:35
@gollth gollth marked this pull request as ready for review October 29, 2023 10:42
@gollth
Copy link
Author

gollth commented Oct 29, 2023

Hi @rouge8 I added tests for the new functionality. I also had to install cargo nextool now in the CI because the new tests require them when testing the parameterized_test_discovery="cargo". Let me know what you think =)

@rouge8
Copy link
Owner

rouge8 commented Oct 30, 2023

Hi @rouge8 I added tests for the new functionality. I also had to install cargo nextool now in the CI because the new tests require them when testing the parameterized_test_discovery="cargo". Let me know what you think =)

Wow, thanks! I'm currently on vacation so it might take me a while to get to this, but I'm excited!

@andy-bell101
Copy link
Contributor

What's the latest on this? I started implementing this myself before noticing this PR. Is the plan still to merge this eventually?

@rouge8
Copy link
Owner

rouge8 commented Dec 12, 2023

Yes, I plan to review it. Unfortunately I don’t write much Rust these days so it’s not super pressing…

@andy-bell101
Copy link
Contributor

Fair enough

Copy link
Owner

@rouge8 rouge8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting an error with the cargo strategy when in one of my projects:

|| Error executing luv callback:
|| ...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: Async task failed without callback: The coroutine failed with this message: 
|| ...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:317: bad argument #1 to 'ipairs' (table expected, got nil)
|| stack traceback:
|| 	[C]: in function 'ipairs'
|| 	...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:317: in function 'cargo'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:330: in function 'discover_positions'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:91: in function 'is_test_file'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:348: in function '_get_adapter'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:205: in function 'get_position'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:152: in function 'get_nearest'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:485: in function <...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:484>
|| stack traceback:
|| 	[C]: in function 'error'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: in function 'close_task'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:117: in function 'cb'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:181: in function 'waiter'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:82: in function 'wake'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:92: in function 'set'
|| 	...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:69: in function <...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:64>
|| Error executing luv callback:
|| ...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: Async task failed without callback: The coroutine failed with this message: 
|| ...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:317: bad argument #1 to 'ipairs' (table expected, got nil)
|| stack traceback:
|| 	[C]: in function 'ipairs'
|| 	...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:317: in function 'cargo'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:330: in function 'discover_positions'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:91: in function 'is_test_file'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:348: in function 'get_adapter'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:358: in function '_set_focused_file'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:497: in function '_start'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:188: in function '_ensure_started'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:200: in function <...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:199>
|| 	...share/nvim/plugged/neotest/lua/neotest/consumers/run.lua:38: in function 'get_tree_from_args'
|| 	...share/nvim/plugged/neotest/lua/neotest/consumers/run.lua:68: in function 'func'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:168: in function <...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:167>
|| stack traceback:
|| 	[C]: in function 'error'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: in function 'close_task'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:117: in function 'cb'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:181: in function 'waiter'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:82: in function 'wake'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:92: in function 'set'
|| 	...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:69: in function <...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:64>

@gollth
Copy link
Author

gollth commented Jan 21, 2024

Hmm, can you give some more info please?

  1. Which rust package are you testing?
  2. What is your neotest config?
  3. What is the output of cargo nextest list --message-format json --manifest-path <PATH/TO/RUST/PACKAGE/YOU/ARE/TESTING>/Cargo.toml?

@rouge8
Copy link
Owner

rouge8 commented Jan 21, 2024

  1. It's a private project unfortunately.
  2. neotest config:
-- Neotest
local neotest = require("neotest")
neotest.setup({
  discovery = { enabled = true },
  adapters = {
      require("neotest-python")({
          args = { "-v" },
      }),
      require("neotest-rust")({}),
      require("neotest-plenary")({}),
  },
})

local bufopts = { noremap = true, silent = true }
-- Run the nearest test
vim.keymap.set("n", "<leader>t", neotest.run.run, bufopts)
-- Run all tests in the file
vim.keymap.set("n", "<leader>T", function()
  neotest.run.run(vim.fn.expand("%"))
end, bufopts)
-- View the test output
vim.keymap.set("n", "<leader>to", neotest.output.open, bufopts)
-- View the test summary
vim.keymap.set("n", "<leader>ts", neotest.summary.open, bufopts)
  1. tests:
{"rust-build-meta":{"target-directory":"/Users/andy/tmp/rust-builds","base-output-directories":["debug"],"non-test-binaries":{},"build-script-out-dirs":{},"linked-paths":["debug/build/zstd-sys-f82c3314ab16f949/out"],"target-platforms":[{"triple":"aarch64-apple-darwin","target-features":["aes","crc","dit","dotprod","dpb","dpb2","fcma","fhm","flagm","fp16","frintts","jsconv","lor","lse","neon","paca","pacg","pan","pmuv3","ras","rcpc","rcpc2","rdm","sb","sha2","sha3","ssbs","vh"]}],"target-platform":null},"test-count":29,"rust-suites":{"wanikani-apprentice::bin/wanikani-apprentice":{"package-name":"wanikani-apprentice","binary-id":"wanikani-apprentice::bin/wanikani-apprentice","binary-name":"wanikani-apprentice","package-id":"wanikani-apprentice 0.1.0 (path+file:///Users/andy/Dropbox/Projects/wanikani-apprentice)","kind":"bin","binary-path":"/Users/andy/tmp/rust-builds/debug/deps/wanikani_apprentice-048373458715c33b","build-platform":"target","cwd":"/Users/andy/Dropbox/Projects/wanikani-apprentice","status":"listed","testcases":{"tests::assignments::logged_out_redirect":{"ignored":false,"filter-match":{"status":"matches"}},"tests::index::logged_in":{"ignored":false,"filter-match":{"status":"matches"}},"tests::index::logged_out":{"ignored":false,"filter-match":{"status":"matches"}},"tests::lb_heartbeat::ignores_host_header":{"ignored":false,"filter-match":{"status":"matches"}},"tests::lb_heartbeat::ok":{"ignored":false,"filter-match":{"status":"matches"}},"tests::login::already_logged_in":{"ignored":false,"filter-match":{"status":"matches"}},"tests::login::invalid_api_key":{"ignored":false,"filter-match":{"status":"matches"}},"tests::login::valid_api_key":{"ignored":false,"filter-match":{"status":"matches"}},"tests::logout":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_500":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_display_time_remaining::case_1":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_display_time_remaining::case_2":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_display_time_remaining::case_3":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_display_time_remaining::case_4":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_display_time_remaining::case_5":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_display_time_remaining::case_6":{"ignored":false,"filter-match":{"status":"matches"}},"tests::test_radical_svg":{"ignored":false,"filter-match":{"status":"matches"}},"tests::trusted_host_header":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_assignments":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_assignments_unknown_subject::case_1":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_assignments_unknown_subject::case_2":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_assignments_unknown_subject::case_3":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_assignments_unknown_subject::case_4":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_kana_vocabulary":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_kanji":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_radicals":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_radicals_with_character_images":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_username":{"ignored":false,"filter-match":{"status":"matches"}},"wanikani::tests::test_vocabulary":{"ignored":false,"filter-match":{"status":"matches"}}}}}}                                                                                                           

@gollth
Copy link
Author

gollth commented Jan 22, 2024

Interesting, its seems like the test package name that M._binary_path() spits out is somehow unknown to cargo nextest, i.e. it is not part of cargo nextest list ... | jq '."rust-suites"', but this is hard to know without knowing the package under test.

Can you reproduce the issue, with any of these packages?

@rouge8
Copy link
Owner

rouge8 commented Jan 23, 2024

I can't reproduce it with the test package unfortunately. I decided to open source the repo I was testing with: https://github.com/rouge8/wanikani-apprentice/blob/main/src/main.rs#L793

Imaginge you have the following tests:

```rust
#[test]
fn foo_bar() {}

#[test_case(1 ; "one")]
#[test_case(2 ; "two")]
fn foo() {}
```

Selecting `foo` in the summary and running it would also run `foo_bar` since we
only match the _prefix_ of the test. To exactly match this test we have to include
"end of line" ($) char in the match, but optionally also match `::.*` so we include
all subtests as well
gollth and others added 11 commits January 24, 2024 14:49
…utable

Without modules being included in the module tree `cargo` is not able to compile
them nor execute their tests
The cargo nextest -E ... filter now also optionally matches subtests (with "(::.*)?")
and was missing in some tests
Somehow the alternative [(...) (...)] syntax of treesitter does not work with our
query to match both scoped and unscoped identifiers. Quick solution is just to
duplicate both parts of the entire query and provide them as separate patterns
This is needed for the new `param_discovery_mode="cargo"` in the tests
@gollth
Copy link
Author

gollth commented Jan 24, 2024

Hey @rouge8,

thanks for sharing your project. You were onto something, it actually was a problem with the M._binary_name() function. I forgot the (prob. not that uncommon) case, where tests are placed in src/main.rs. Cargo nextests identifies this with <package>::bin/<package>, not just <package> how I exspected. I also fixed another bug, where plain async tests without any cases would be picked up by the discovery. Can you please retry with your project?

@rouge8
Copy link
Owner

rouge8 commented Jan 24, 2024

Hey @rouge8,

thanks for sharing your project. You were onto something, it actually was a problem with the M._binary_name() function. I forgot the (prob. not that uncommon) case, where tests are placed in src/main.rs. Cargo nextests identifies this with <package>::bin/<package>, not just <package> how I exspected. I also fixed another bug, where plain async tests without any cases would be picked up by the discovery. Can you please retry with your project?

Now I'm getting "No tests found" in that file and a bunch of errors when I try to run the tests in https://github.com/rouge8/wanikani-apprentice/blob/main/src/wanikani.rs. It also looks like CI is broken...

README.md Outdated Show resolved Hide resolved
Thore Goll and others added 3 commits January 25, 2024 09:57
Cargo nextest will identify the tests of translation unit `src/main.rs`
as `<package>::bin/<package>`, not as unit tests.
@gollth
Copy link
Author

gollth commented Jan 25, 2024

Okay took me a bit to figure out that there was a regression with treesitter between nvim 0.9.0 & nightly. Refactored the treesitter logic slightly to also work for 0.9.0. This also fixes the CI again. Can you please retry and see if it works now for you?

Regarding the "No tests found" I sometimes experience the same behavior, but this must be sth. inside neotest itself. Reopening the buffer and starting test discovery again, usually helps

@rouge8
Copy link
Owner

rouge8 commented Jan 25, 2024

I'm still seeing errors in https://github.com/rouge8/wanikani-apprentice/blob/main/src/wanikani.rs:

|| Error executing luv callback:
|| ...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: Async task failed without callback: The coroutine failed with this message: 
|| ...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:323: bad argument #1 to 'ipairs' (table expected, got nil)
|| stack traceback:
|| 	[C]: in function 'ipairs'
|| 	...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:323: in function 'cargo'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:364: in function 'discover_positions'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:93: in function 'is_test_file'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:348: in function '_get_adapter'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:205: in function 'get_position'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:152: in function 'get_nearest'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:485: in function <...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:484>
|| stack traceback:
|| 	[C]: in function 'error'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: in function 'close_task'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:117: in function 'cb'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:181: in function 'waiter'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:82: in function 'wake'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:92: in function 'set'
|| 	...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:69: in function <...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:64>
|| Error executing luv callback:
|| ...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: Async task failed without callback: The coroutine failed with this message: 
|| ...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:323: bad argument #1 to 'ipairs' (table expected, got nil)
|| stack traceback:
|| 	[C]: in function 'ipairs'
|| 	...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:323: in function 'cargo'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:364: in function 'discover_positions'
|| 	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:93: in function 'is_test_file'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:348: in function 'get_adapter'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:358: in function '_set_focused_file'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:497: in function '_start'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:188: in function '_ensure_started'
|| 	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:200: in function <...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:199>
|| 	...share/nvim/plugged/neotest/lua/neotest/consumers/run.lua:38: in function 'get_tree_from_args'
|| 	...share/nvim/plugged/neotest/lua/neotest/consumers/run.lua:68: in function 'func'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:168: in function <...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:167>
|| stack traceback:
|| 	[C]: in function 'error'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:95: in function 'close_task'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:117: in function 'cb'
|| 	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:181: in function 'waiter'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:82: in function 'wake'
|| 	...dy/.local/share/nvim/plugged/neotest/lua/nio/control.lua:92: in function 'set'
|| 	...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:69: in function <...re/nvim/plugged/neotest/lua/neotest/lib/process/init.lua:64>

And getting "No tests found" with Cargo in https://github.com/rouge8/wanikani-apprentice/blob/main/src/main.rs#L793

Would you be open to removing the Cargo test discovery from this PR and re-opening that in a separate PR once this is merged? I still need to make sure I understand the code, but the Treesitter discovery seems much more reliable and I'd feel comfortable merging that.

@gollth
Copy link
Author

gollth commented Jan 27, 2024

I'm still seeing errors...

Hmm, I tried in an Ubuntu 22:04 docker container, with nvim 0.9.0 (the same as in the CI) starting like: nvim -u tests/minimal_init.vim wanikani-apprentice/src/main.rs and then loading the plugin with :lua require("neotest").setup({ adapters = { require("neotest-rust") { args = { "--no-capture" }, parameterized_test_discovery = "cargo" } } }) and could discover and run the tests in main.rs just fine (beside closing and reopening the buffer once as discussed above). Can it be your config somehow? Can you reproduce the issue with the same setup?

Would you be open to removing the Cargo test discovery from this PR and re-opening that in a separate PR once this is merged?

In theory yes, but both strategies are rather intermixed now (especially in the README). Would be alot of effort for me TBH... We could mark the "cargo" strategy "experimental" in the README maybe? The default is "none" anyway right now, so this the whole feature would be opt-in anyway. Then we could get feedback from users, see how it sticks.

I still need to make sure I understand the code, but the Treesitter discovery seems much more reliable and I'd feel comfortable merging that.

The treesitter approach is actually more complicated 😅 With the cargo strategy I just call cargo nextest list to get all the test names of the crate:

local command = "cargo nextest list --message-format json --manifest-path "
.. workspace
.. Path.path.sep
.. "Cargo.toml"

...interprete the resulting output...

local json = vim.json.decode(output.stdout)

...transform some special syntax for cargo nextest to produce test IDs that can be passed later back to cargo to run the tests...

--- Given a certain `path` to a rust file, the path to its package `workspace` and
--- the contents of the package's `cargo_toml`, guess its [test binary name](https://nexte.st/book/running.html)
---
--- unit tests: <package>/src/<path> -> <package>
--- integration: <package>/tests/<mod>.rs -> <package>::<mod>
--- binary: <package>/src/bin/<bin>.rs -> <package>::bin/<bin>
--- binary: <package>/src/main.rs -> <package>::bin/<package>
--- example: <package>/examples/<bin>.rs -> <package>::example/<bin>
--- @param path string
--- @param workspace string|nil root of the project (containing Cargo.toml)
--- @return string|nil
function M._binary_name(path, workspace)

... and finally just go through all the (parent) tests detected in the first pass (in init.lua) and augment them with their parameterized children:

for _, value in positions:iter_nodes() do
local data = value:data()
if data.type == "test" and data.parameterization ~= nil then
for _, case in ipairs(tests[target]) do
if case:match("^" .. data.id .. "::") then
-- `case` is a parameterized version of `value`, so add it as child
local name = name_mapper(case:gsub("^" .. data.id .. "::", ""), data.parameterization, path)
if name ~= nil then
value:add_child(
case,
value:new({
type = "test",
id = case,
name = name,
range = data.range,
path = path,
}, {}, value._key, {}, {})
)
end
end
end
end
end

@rouge8
Copy link
Owner

rouge8 commented Jan 27, 2024

With tests/minimal_init.vim and nvim 0.9.5 on macOS nvim crashes and exits with code 134 when I try to run the tests 🙃

@gollth
Copy link
Author

gollth commented Jan 27, 2024

Can you reproduce the issue with the same setup?

???

@rouge8
Copy link
Owner

rouge8 commented Jan 27, 2024

Can you reproduce the issue with the same setup?

???

Can you create a Dockerfile that installs the appropriate nvim, cargo nextest, etc.? If you do I'll try that.

Regarding the "No tests found" I sometimes experience the same behavior, but this must be sth. inside neotest itself. Reopening the buffer and starting test discovery again, usually helps

I'm not convinced this is inside Neotest because it happens for me consistently with the cargo discovery but not the treesitter discovery. I have the neotest log output from trying to use cargo discovery on https://github.com/rouge8/wanikani-apprentice/blob/main/src/main.rs#L793:

WARN | 2024-01-27T10:25:52Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:52Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:53Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:53Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:53Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:53Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:53Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:54Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:54Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
ERROR | 2024-01-27T10:25:54Z-0800 | ...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:311 | Couldn't find positions in path /Users/andy/Dropbox/Projects/wanikani-apprentice ...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:323: bad argument #1 to 'ipairs' (table expected, got nil)
stack traceback:
	[C]: in function 'ipairs'
	...ndy/projects/neotest-rust/lua/neotest-rust/discovery.lua:323: in function 'cargo'
	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:364: in function 'discover_positions'
	/Users/andy/projects/neotest-rust/lua/neotest-rust/init.lua:93: in function 'f'
	.../nvim/plugged/neotest/lua/neotest/lib/func_util/init.lua:61: in function 'filter_list'
	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:283: in function <...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:266>
	[C]: in function 'xpcall'
	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:266: in function '_update_positions'
	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:539: in function '_update_adapters'
	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:493: in function '_start'
	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:188: in function '_ensure_started'
	...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:200: in function <...l/share/nvim/plugged/neotest/lua/neotest/client/init.lua:199>
	...share/nvim/plugged/neotest/lua/neotest/consumers/run.lua:38: in function 'get_tree_from_args'
	...share/nvim/plugged/neotest/lua/neotest/consumers/run.lua:68: in function 'func'
	...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:168: in function <...andy/.local/share/nvim/plugged/neotest/lua/nio/tasks.lua:167>
WARN | 2024-01-27T10:25:54Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:54Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls
WARN | 2024-01-27T10:25:54Z-0800 | ...nvim/plugged/neotest/lua/neotest/lib/treesitter/init.lua:183 | Using `build_position` or `position_id` functions with subprocess parsing is not supported, switch to using strings for remote calls

@rouge8
Copy link
Owner

rouge8 commented Jan 27, 2024

I did some more testing... The cargo strategy works in main.rs when I have neotest's discovery disabled, but consistently returns "No tests found" when discovery is enabled, probably due to that error in the logs. https://github.com/rouge8/wanikani-apprentice/blob/main/src/wanikani.rs has the ipairs error in both cases

@gollth
Copy link
Author

gollth commented Feb 5, 2024

Sorry @rouge8 for the late reply, was busy with work. I quickly threw together a Dockerfile:

FROM ubuntu:22.04

RUN apt-get update && apt-get -y install git build-essential curl libssl-dev pkg-config && rm -rf /var/lib/apt/lists/*

# Neovim
RUN mkdir -p _neovim
RUN curl -sL https://github.com/neovim/neovim/releases/download/v0.9.5/nvim-linux64.tar.gz | tar xzf - --strip-components=1 -C "${PWD}/_neovim"
RUN git clone https://github.com/LazyVim/starter ~/.config/nvim
ENV PATH="${PWD}/_neovim/bin:${PATH}"

# Rust
RUN curl https://sh.rustup.rs -sSf | bash -s -- -y
ENV PATH="/root/.cargo/bin:${PATH}"
RUN cargo install cargo-nextest

# Treesitter
RUN echo 'return { {"gollth/neotest-rust", branch = "param"}, { "nvim-neotest/neotest", config = function() require("neotest").setup({ adapters = { require("neotest-rust")({ dap_adapter = "codelldb", parameterized_test_discovery = "cargo", }), } }) end } }' > ~/.config/nvim/lua/plugins/neotest.lua
RUN nvim --headless -c "TSInstallSync rust | quit"

# Project
RUN git clone https://github.com/rouge8/wanikani-apprentice.git ~/wanikani-apprentice
RUN cargo nextest --manifest-path ~/wanikani-apprentice/Cargo.toml list
CMD nvim

When you run it it opens nvim with the necessary config and test strategy already set to "cargo". Now open a file containing tests in nvim (e.g. ~/wanikani-apprentice/src/main.rs) and run :lua require("neotest").summary.toggle(). Again you might have to close and reopen the file if the tests are not immediately found.

@theoribeiro
Copy link

This is pretty awesome, @gollth. Just tested this PR and it's working great on my config. Hopefully it will get merged soon.

Thanks for your work @gollth and @rouge8

@gollth
Copy link
Author

gollth commented Apr 15, 2024

@rouge8 were you able to sort out your config?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants