Repository navigation
Conversation
`test_current_or_recover_returns_repo_when_cwd_exists` asserted the function's success branch by relying on the test binary inheriting a git repo as its CWD. The Nix flake build sandbox strips `.git` from the extracted source (`pkgs.lib.cleanSource`), so `Repository::current()` errors and the assertion panics — the failure tend tried to silence in the closed PR #2633. Extract the success/recovery wiring into `or_recover()`, which takes the `Repository::current()` result and the recovery closure as parameters. Production code threads its real values through unchanged. Tests pass explicit inputs for all three branches (success, recovery, neither), giving better coverage than the original single-branch happy-path check while removing the dependency on inherited CWD. Closes #2632 Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
left a comment
worktrunk-bot
left a comment
There was a problem hiding this comment.
codecov/patch fails on the panic!() line — it's defensive but unreachable. or_recover(Ok(...), ...) matches the Ok arm before recover() is invoked, so the closure body is never executed. Per CLAUDE.md ("If code is unused, remove it"), replace the panic-closure with || None. The test still validates the success branch via assert!(!recovered) and the repo-path assertion; the type-level structure of or_recover already guarantees recover() isn't called on the Ok arm, so the panic is redundant.
codecov/patch failed — see follow-up review for fix
`command_exists` shelled out to `which` (Unix) / `where` (Windows). The Nix flake build sandbox doesn't ship those binaries on PATH, so the helper silently returned `false` for every command — `detect_llm_tool` would always claim no tool was available — and `test_command_exists_known_command` panicked because `which which` exited non-zero. Switch to the `which` crate (already a workspace dependency, used by `shell_exec`, `priority`, `completion`, and `shell`) which walks PATH directly without a child process. Splitting out `command_exists_in` lets the positive-case test assert against a tempdir-controlled search path instead of the inherited PATH, removing the environment-coupling that made the original test brittle. Co-Authored-By: Claude <noreply@anthropic.com>
worktrunk-bot
left a comment
worktrunk-bot
left a comment
There was a problem hiding this comment.
codecov/patch failed: 1 miss in commit_generation.rs (the cfg!(windows) branch in the new test) and 1 miss in recover.rs (the previously-flagged panic!() line — open suggestion in the prior thread still stands).
Inline suggestion below for the new miss; the runtime cfg!(windows) keeps both string literals in every build, so Linux coverage marks the Windows arm uncovered. Switching to attribute cfg resolves it.
codecov/patch failed — see follow-up review for the gap
- Drop the panic-closure in `test_or_recover_uses_current_repo_without_recovering`. The success branch never invokes the recovery closure, so the body was unreachable defensive code that codecov flagged as a miss. - Switch the bin-name selection in `test_command_exists_in_finds_binary_on_explicit_path` from runtime `cfg!(windows)` to attribute `#[cfg(...)]`. The runtime form keeps both literals in every build, so coverage marked the inactive arm uncovered. Co-Authored-By: Claude <noreply@anthropic.com>
2 participants
Summary
Fix two
nix-flakenightly check regressions — both pre-existing failures in the build sandbox wherepkgs.lib.cleanSourcestrips.gitand minimal PATH lacks standard tools. Each fixes the current "first failure"; further nix-flake test failures remain (snapshot mismatches in integration tests with implicit CWD assumptions) and are a separate scope.git::recover::tests::test_current_or_recover_returns_repo_when_cwd_exists(Nightly tests failed #2632) — extractsor_recover()fromcurrent_or_recover()so the wiring can be tested with explicitRepository::current()and recovery-closure inputs. Replaces the single env-coupled assertion with three branch-coverage tests (success / recovers / propagates error).output::commit_generation::tests::test_command_exists_known_command— switchescommand_existsfrom shelling out to thewhich/wherebinary (absent in the Nix build sandbox) to thewhichcrate (already a workspace dep). Splits outcommand_exists_inso the positive-case test asserts against a tempdir-controlled search path instead of inherited PATH.Why this is a fix, not a test skip
Following the rule established by the closure of #2633 and codified in running-tend: no
Repository::current().is_err() else returnguards. Each commit either fixes production code (command_existsno longer needs an external binary) or restructures the test so its setup doesn't depend on environment that isn't guaranteed.Test plan
cargo run -- hook pre-merge --yes— 3526 tests pass, all lints clean.nix-flakenightly run on the branch confirms both targeted tests now pass; remaining failures (e.g.test_config_init_already_exists,test_config_show_*) are pre-existing and unrelated.🤖 Generated with Claude Code