Repository navigation
test: strengthen coverage of launcher/detect_project_type ahead of plenary.nvim removal (#317) - #328
Merged
Merged
Conversation
Enables injecting a stub job runner in tests (and, in a future PR, a
non-plenary implementation) without changing runtime behavior. The
module-level require("neotest-java.build_tool.launcher").launch_debug_test
call site is unchanged.
Also fixes a latent bug where a failed pcall(require, "dap.repl") left
deps.dap_repl set to the pcall error string (truthy) instead of nil,
which would crash on repl.clear()/repl.append() if launch_debug_test
were ever reached without nvim-dap installed.
…, exit Covers the DI seam added in the previous commit with a stubbed job runner: command/args/cwd passed through to the job, the returned event only resolves once stdout reports 'Listening', the terminated event fires on job exit, and nil stdout/stderr chunks are handled safely.
…ction Spawns a real java -agentlib:jdwp=...,suspend=n process through the real plenary.job (not stubbed) and asserts that launch_debug_test's 'Listening' stdout detection and process-exit event both work against genuine process I/O. This is the closest practical substitute for a full 'strategy = "dap"' e2e run without wiring up a real nvim-dap attach flow, and is the main safety net for PR 2's plenary.job removal. Skips gracefully (MiniTest.skip) when java is not on PATH, since the main test.yml CI job does not install Java.
…llback Covers all maven/gradle marker filenames and maven-over-gradle precedence at the top level, and the recursive scandir fallback (triggered when readdir errors, finds nothing, or the directory is empty), including precedence rules, error handling, and the exact scan options passed to plenary.scandir. No dedicated tests existed for this module before.
Exercises the real plenary.scandir recursive fallback (no stubs) against genuine nested directory trees created with vim.fn.mkdir, covering maven and gradle markers found several directories deep, an 'unknown' result when nothing matches, and confirming the fast top-level path still wins over a deeper conflicting marker.
rcasia
marked this pull request as ready for review
September 5, 2026 12:09
…lash literals
Windows CI failed on both new specs: Path("/fake/root"):to_string() and
Path("/fake/cwd"):to_string() normalize to backslashes on Windows, but
the assertions compared against hardcoded forward-slash string literals
instead of going back through Path. Per AGENTS.md's Path-handling
convention, cross-platform assertions must derive expected strings from
Path itself, not hardcode a separator.
10 tasks done
rcasia
added a commit
that referenced
this pull request
Sep 5, 2026
Mirrors the existing nio.wrap(vim.system) convention from command/command_executor.lua. The DI seam PR #328 added (BuildToolLauncher(deps).job_runner) made this a drop-in default swap — same contract (new(opts):start(), command/args/cwd/on_stdout/on_stderr/on_exit), no callers changed. Also fixed the latent dap.repl bug PR #328 documented: a failed pcall(require, 'dap.repl') left repl as the pcall's truthy error-message string instead of nil, which would have crashed any repl.clear()/append() call when dap.repl isn't installed.
rcasia
added a commit
that referenced
this pull request
Sep 5, 2026
…ew defaults Same real-JVM / real-filesystem coverage PR #328 added, now exercising the new vim.system-backed job runner and dir_scan-based fallback instead of the old plenary-based defaults.
rcasia
added a commit
that referenced
this pull request
Sep 5, 2026
Mirrors the existing nio.wrap(vim.system) convention from command/command_executor.lua. The DI seam PR #328 added (BuildToolLauncher(deps).job_runner) made this a drop-in default swap — same contract (new(opts):start(), command/args/cwd/on_stdout/on_stderr/on_exit), no callers changed. Also fixed the latent dap.repl bug PR #328 documented: a failed pcall(require, 'dap.repl') left repl as the pcall's truthy error-message string instead of nil, which would have crashed any repl.clear()/append() call when dap.repl isn't installed.
rcasia
added a commit
that referenced
this pull request
Sep 5, 2026
…ew defaults Same real-JVM / real-filesystem coverage PR #328 added, now exercising the new vim.system-backed job runner and dir_scan-based fallback instead of the old plenary-based defaults.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This is PR 1 of 2 addressing #317 (plenary.nvim is archived/unmaintained). PR 2 (branch
stack/plenary-2-remove-dependency, stacked on top of this one) will replace the two remaining plenary.nvim usages:lua/neotest-java/build_tool/launcher.lua— usesplenary.jobto spawn the debug JVM, wait for the JDWP "Listening" line on stdout, and optionally stream todap.repl.lua/neotest-java/util/detect_project_type.lua— usesplenary.scandiras a recursive fallback scan when the fast top-level check finds no build markers.This PR does not touch plenary or attempt the swap. It only strengthens tests around the current (plenary-based) behavior of both modules, so PR 2 has a safety net that proves it doesn't regress anything.
Gaps found
launcher.luahad zero dedicated tests (unit or otherwise) and no DI seam — it calledrequire("plenary.job")directly at module scope, making it impossible to unit test in isolation.detect_project_type.luaalready had a DI seam (scandir/readdirparams) but had zero dedicated tests at all — neither the fast top-level path nor the recursiveplenary.scandirfallback path were exercised anywhere in the suite.plenary.jobspawn + "Listening" stdout-detection + process-exit logic thatlaunch_debug_testrelies on.detect_project_type's recursive fallback against a real, nested multi-directory filesystem layout (only ever exercised, if at all, via stubs).What was done
launcher.lua(BuildToolLauncher(deps)factory with an injectablejob_runner, defaulting toplenary.job; also madedap_replinjectable to avoid a latent bug where a failedpcall(require, "dap.repl")left a truthy error string in place ofrepl). Keptrequire("neotest-java.build_tool.launcher").launch_debug_testworking exactly as before via a module-level singleton, and exposed.new(deps)for callers that want to inject.tests/unit/test_launcher_spec.lua— 5 fast unit tests using a stubbed job runner (start/stdout/stderr/exit wiring, "Listening" detection gating the returned event, nil-chunk handling, default-deps sanity check).tests/integration/test_launcher_spec.lua— spawns a real JVM (java -agentlib:jdwp=...,suspend=n -version) through the realplenary.job, and asserts the real "Listening" stdout detection and process-exit event both work. This is the closest practical substitute for a fullstrategy = "dap"e2e run without wiring up a real nvim-dap attach flow (which the CI's maintest.ymljob doesn't have Java for anyway — see note below).tests/unit/test_detect_project_type_spec.lua— 12 unit tests covering the fast top-level path (all maven/gradle marker filenames, maven-over-gradle precedence) and the fallback path (readdir errors, empty/no-match top level, precedence, scandir errors, and the exact scan opts passed toplenary.scandir).tests/integration/test_detect_project_type_spec.lua— 4 tests against a real nested filesystem (vim.fn.mkdir/vim.fn.tempname), exercising the realplenary.scandirfallback for both maven and gradle markers several directories deep, an "unknown" case, and confirming the fast path still wins when a marker exists at both levels.lua/neotest-java/types.lua(nio.control.Eventwas missingis_set) needed to type the new tests correctly.Known remaining gap (documented, not closed in this PR)
strategy = "dap"test throughneotest.run.run()(full jdtls + nvim-dap attach flow) does not exist and was not added here. The project's main CI (test.yml) has no Java, and thee2e-test.ymlharness (tests/e2e/run.lua) doesn't wire up nvim-dap or Neotest's dap strategy at all. Building that out would require installing nvim-dap in the e2e harness and is a bigger lift than fits this PR; the real-JVM integration test added here covers the actual behavior at risk from the plenary.nvim swap (job spawn, stdout "Listening" detection, process-exit) without that overhead. Flagging this so PR 2 (or a follow-up) can decide whether to invest in it.Test results
bash scripts/test: 189/189 passing (was 173 before this PR)stylua --check .: cleanluacheck lua/neotest-java/... tests/...: cleanlua-language-server --check .: clean