Repository navigation
Add runtime discovery, parameterized test support, and isolated execution - #41
Conversation
…est harness (alfaix#1) Fix the treesitter query so tests are discovered again on current cpp parsers. Add :ConfigureGtestAll, which maps every test file to its executable from compile_commands.json and the CTest registry (gtest_discover_tests). Rebuild the test harness on vim.pack (Neovim 0.12+) with an isolated .tests/ dir, and update CI to match.
Run all TEST_P instances from a single neotest source position and combine their results. Match parameterized names when auto-configuring executables from CTest, and add unit and integration coverage.
Discover compiled parameterized cases as children of their source tests. Run selected leaves by exact Google Test name, including DAP runs. Report leaf results while preserving grouped parent results.
Check summary leaves, buffer-nearest selection, and exact DAP filters. Verify selected, parent, and file runs against compiled cases.
Discover TYPED_TEST and TYPED_TEST_P declarations as source parents and attach a separate Neotest leaf for each case reported by the compiled Google Test binary. Preserve full runtime names for exact run and DAP filters, executable mapping, and independent result reporting. Map typed cases back to their source declarations across custom type names and multiple type-pattern instantiations. Update CTest executable selection to recognize their runtime names, and document the discovery and file-save refresh behavior. Add a compiled fixture and unit and integration coverage for parsing, inventory, CTest mapping, summary leaves, buffer selection, parent and file runs, DAP specs, and per-case failure diagnostics. Verified with make unit-test, offline make integration-test (18/18), changed-file stylua --check, and git diff --check.
Add process_isolation to adapter setup and allow per-run overrides while keeping grouped execution as the default and preserving DAP behavior. Expand a selection into exact compiled cases, each with its own Google Test filter and JSON report. Run the cases sequentially through one Neotest custom strategy, then merge their results into the existing position tree. This lets later cases report after an earlier case fails or crashes and avoids marking cases as missing while they are pending. Require a refreshed runtime inventory for generated cases. Add a fixture that exposes shared process state, an assertion failure, a crash, and a later passing case. Cover exact filters, report paths, generated leaves, parent status, and the grouped default. Document the opt-in setting and its per-case startup cost. Verified with make unit-test, the offline integration suite (20/20), stylua --check, and git diff --check.
There was a problem hiding this comment.
Hey! Thanks a lot for the PR, really. Haven't been doing a lot of manual C++ coding with all the claudes lately, so the plugin grew abandoned with almost more forks than stars. Coming back to using my hands lately, but so far only fixed the CI, so really appreciate the contribution.
There are a couple issues, such as conflicts, CI missing older gtest versions, etc. I got halfway through the review, and for now have to stop, plan to come back to it in ~2 weeks (work trip starting tomorrow, sorry!). If you prefer, I can also fix stuff myself & merge when I'm back.
Claude got all the way and found some issues, which I haven't checked manually so feel free to ignore for now.
Warning: maybe AI slop
Verdict: do not merge as it is. The features work and the tests are real, but the branch conflicts with main, reverts parts of #40, and has two defects in the new code paths. Nothing was posted to GitHub.
I ran the PR's suite in a scratch worktree (Neovim 0.13-dev, GoogleTest main): 131 unit and 20 integration tests pass. The PR has no CI run. I left a local branch pr-41; delete it with git branch -D pr-41.
Blockers
- Merge conflicts with main. The branch is based on b66f1d3, before #40. Five files conflict: workflow.yaml, Makefile, neotest_adapter.lua, parse.lua, and tests/unit/minimal_init.lua.
- CI regression. The PR replaces the workflow with one job on stable Neovim. The GoogleTest version matrix and the nightly leg are gone, and the tree-sitter CLI comes from releases/latest.
- README badge points to the fork. It now reads github.com/Andeshog/neotest-gtest/....
- parse.lua query conflicts with the #40 fix. The PR removes the anchors and comment handling from the query, where main now checks parameters in Lua. The PR also keeps a stale comment ("Only have two parameters (. anchors)") and replaces vim.treesitter.get_node_text with a hand-written version.
Correctness
- An isolated run loses all results if one runtime name is stale (confirmed by a probe). init.lua:54 asserts that each isolated case has a result. If the binary was rebuilt and a case no longer exists, GoogleTest exits 0 with an empty report and the assert throws. The PR description says the opposite.
- Each save of a test file gives a "stale inventory" warning and removes the runtime leaves. discover_positions skips the inventory when the source is newer than the executable, which is always true directly after a save. The leaves return only after a rebuild and a second save.
- Discovery runs the test executable with no cache and no timeout. --gtest_list_tests runs once per file with generated tests, on each discovery. Many files that share one binary start that binary many times, and a binary that hangs blocks discovery.
- A passing case can be reported as failed. In report.lua, a result for a runtime case with no tree position marks the parent as failed ("Runtime case has no position"). This happens after a rebuild that adds an instantiation.
- inventory.map_source asserts on an ambiguous mapping. The call in discover_positions is not protected, so the file fails discovery. This case is rare.
- A stopped isolated run reports the remaining cases as failed, not skipped (exit code 130 path).
- Type names that contain a . get no leaf. The pattern in map_source rejects them without a message. This is minor.
- Behavior change from the fork commits: a run with some unmapped nodes now runs the mapped ones and warns. Before, it raised an error. Decide if you want this.
Feature completeness
- Discovery is two steps. Tree-sitter finds the source declarations. Then, for files with a configured executable, --gtest_list_tests supplies the runtime names, which become child nodes of the declaration.
- CMake without CTest: automatic mapping does not work. :ConfigureGtestAll needs compile_commands.json and ctest --show-only=json-v1 entries that contain --gtest_filter=. In practice this means gtest_discover_tests or gtest_add_tests. A plain add_test,
or no enable_testing(), gives "no binary contains its tests". I read this from the code and did not run that - Manual :ConfigureGtest still works without CTest, and the runtime leaves do not need CTest.
- No support for multi-config generators (ctest -C), although the description claims expanded configuration handling.
- Macro support:
┌─────────────────────────────────────────────────┬───────────┬────────────────────────────────────────────────────────┐
│ Macro │ Supported │ Note │
├─────────────────────────────────────────────────┼───────────┼────────────────────────────────────────────────────────┤
│ TEST, TEST_F │ Yes │ Unchanged │
├─────────────────────────────────────────────────┼───────────┼────────────────────────────────────────────────
│ TEST_P │ Yes │ Wildcard filter without inventory, exact names with it │
├─────────────────────────────────────────────────┼───────────┼────────────────────────────────────────────────────────┤
│ TYPED_TEST, TYPED_TEST_P │ Yes │ Same
├─────────────────────────────────────────────────┼───────────┼────────────────────────────────────────────────────────┤
│ Custom name generators, multiple instantiations │ Yes │ Covered by integration tests │
└─────────────────────────────────────────────────┴───────────┴────────────────────────────────────────────────────────┘
- Isolation runs one process per case in sequence, with no live output stream. It uses the private module neotest.client.strategies.integrated.
Tests
The test side comes from a subagent that read the diff; I did not check each item myself.
- Good: the unit tests for ctest.lua and inventory.lua test pure functions with no mocks. The integration tests run real binaries, including a crash under isolation.
- Existing helpers were loosened. controller.lua now ignores runtime leaves in the published-results check. tests/utils/report.lua has a new heuristic that skips the header check. mock_project.lua no longer asserts that exe2node is non-nil.
- Not tested: --gtest_list_tests failure, missing ctest, missing compile_commands.json, stop or cancel of an isolated run, stale inventory (defects 1, 2 and 4), and the "unmapped nodes" double path.
- Flake risk: five poll loops of 100 × 20 ms, and two integration tests of 130 to 150 lines that each assert many behaviors.
- Test bootstrap clones plugins at HEAD with vim.pack.add at test time, so it needs network access and nothing is pinned.
Slop check
Low in the code, high in the description.
- The description overstates the code. It claims "ambiguous mappings" are handled (it is an assert), that results are not lost (defect 1), and that build configurations are expanded (no such code).
- Duplication: the check kind == "TEST_P" or kind == "TYPED_TEST" or kind == "TYPED_TEST_P" appears six times in four files. parse.lua has a helper for it that nothing else uses.
- Dead code: _find_node_by_name tests the same key twice, so the third branch cannot run. The last _raise_gtest can only be reached from old test specs.
- Structure: _fail_unreported_positions is about 100 lines with three separate loops over the same list. discover_positions reads and parses each file twice. _reset_throttle is a test-only export in production code.
Watch configured binaries and rediscover tests without another ConfigureGtest or source buffer update. Use Neovim 0.12 capture shapes and add a timeout to compiled test inventory. Document runtime name mapping and cover rebuild, disk-only source changes, and timeout behavior.
Guard GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST, which was introduced after 1.10. Keep the empty fixture in source discovery while allowing the oldest CI matrix entry to compile and run.
|
Hey, I have addressed all the issues. CI also passes now. I had to make some of the conditional tests conditional everywhere. I agree that the whole rescan is fragile. I do a lot of daily c++ work across projects and variety of the features (except all the emmylua/doxy) in this PR were my own working copies for a very long time now. so at least for my use cases, they are battle tested. PS: |
Share the generated-test kind check and remove unreachable lookup logic. Enrich parsed positions without reading the source again, and process unreported results in one pass. Preserve the throttle reset used by tests.
The isolated spec returned its own strategy function, so neotest ignored a custom strategy passed to the run, and each case always ran with the integrated strategy and an empty strategy config. Pass the run strategy to the isolated spec and resolve it the same way neotest does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A stale runtime name matches no case, so Google Test exits 0 with an empty report, and an assert threw away the results of the whole run. Report only that case as failed. Cases that did not start before a stop were reported as failed. Report them as skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed a couple bugs that Claude spotted:
Have a couple organizational issues still, but will figure them out later on. I don't like that we're handling isolation when it seems like a Some typings are incosistent, but they were incosistent before you :) So I'll clean it up in the project as a whole. |
For over a year, I've been maintaining and using a growing collection of local patches to
neotest-gtest, primarily for my work on MayaFlux, a fairly large C++ project with extensive Google Test and CTest usage.Initially, these were small fixes for things I needed in my day-to-day workflow. Over time, they grew to cover parameterized and typed tests, runtime discovery, individual test execution, result reporting, process isolation, and various CTest integration issues.
I held off on upstreaming them partly because some of the changes felt a little opinionated, and partly because each patch addressed a fairly specific need. But the collection kept growing, and maintaining it across updates became increasingly cumbersome. At some point, carrying around an expanding set of patches became less reasonable than putting the whole thing together properly.
This PR is that consolidation.
I've used andeshog/neotest-gtest as the starting point, incorporating its existing improvements and preserving the commit history and authorship.
What's changed
Runtime discovery and source mapping
TEST_P) and typed test families, including instantiation prefixes and custom names.Execution and reporting
Optional process isolation
CTest integration
Tests and documentation
Validation
These features aren't entirely new experiments. Most originated as patches I've been using in my own development environment for a considerable time.
The consolidated implementation is now running in my regular MayaFlux development workflow, including parameterized and typed tests, individual execution, debugging, CTest integration, and isolated runs.
I've also added automated tests for the new functionality.
Notes
I realize this is a fairly large PR, and some of the decisions may be more opinionated than the original adapter intended. I don't necessarily expect every choice to be adopted unchanged.
My main intention is to contribute back a set of improvements that have been useful in practice, rather than continue maintaining them indefinitely as private patches.
Credit also goes to the work already done in andeshog's fork, which provided a much better starting point for bringing these changes together.
P.S. If this PR is too much to take on, whether in scope or in ongoing maintenance, I'm also perfectly happy to maintain my own fork rather than add to your workload. Maintaining a fork is radically simpler for me than the increasingly unwieldy patchset + pinned versions workflow I've been using for the past year. I wanted to offer the changes upstream first, but there's absolutely no expectation that you take on the additional maintenance burden.
TEST_P#13