Skip to content

fix(tests): canonicalize the temp repo path in the integration helper - #200

Merged
helizaga merged 1 commit into
mainfrom
tommy/fix-macos-clean-path-test
Sep 15, 2026
Merged

helizaga merged 1 commit into
mainfrom
tommy/fix-macos-clean-path-test

Conversation

@helizaga

@helizaga helizaga commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Description

cmd_clean --merged uses nested registered worktree path fails on macOS and passes in CI. setup_integration_repo took its path straight from mktemp -d, which on macOS returns /var/folders/... while /var is a symlink to /private/var. git canonicalizes when it registers a worktree, so the preRemove hook received the /private/var spelling and the assertion compared it against the /var one:

EXPECTED: preRemove:/var/folders/.../tmp.vC6q33uqBL-worktrees/jsmith/my-feature
ACTUAL  : preRemove:/private/var/folders/.../tmp.vC6q33uqBL-worktrees/jsmith/my-feature

Resolving the path once in the helper fixes it for every integration test rather than patching the single assertion. Linux is unaffected, because mktemp -d returns /tmp with no symlink to resolve, which is why CI was green throughout.

Motivation

The suite was permanently red on macOS, which trains contributors to ignore failures and hides real regressions behind an expected one.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature
  • Breaking change
  • Documentation update
  • Code refactoring (no functional changes)

This is the test being wrong, not the product

Worth stating explicitly, since "make the failing assertion pass" can hide a defect. Passing a /var path to git worktree add makes git itself register the resolved one:

passed to git : /var/folders/.../tmp.MDxNeFBZZ2/wt
git registered: /private/var/folders/.../tmp.MDxNeFBZZ2/wt

So the canonical path a hook receives is git's own behavior and the correct thing for cmd_clean to pass along. The test's expectation was the part that was wrong.

Testing

Tested on:

  • macOS (26.6.2, git 2.54.0, bash 5.3.15)
  • Linux
  • Windows (Git Bash)

Linux coverage comes from the CI Tests job on this PR.

Automated gates

  • bats tests/ — 561 of 561 pass, up from 560 of 561. This is the first fully green local run.
  • shellcheck bin/gtr bin/git-gtr lib/*.sh lib/commands/*.sh adapters/editor/*.sh adapters/ai/*.sh — clean.
  • ./scripts/generate-completions.sh --check — up to date.

Teardown still removes the temporary directories; no strays remain after a run.

Breaking Changes

  • This PR introduces breaking changes

None. Test-only; no shipped code paths change.

Checklist

  • I have read CONTRIBUTING.md
  • My code follows the project's style guidelines (cd -P … && pwd matches the existing idiom in bin/gtr and bin/git-gtr)
  • I have performed manual testing on at least one platform
  • I have updated documentation if needed (not applicable)
  • My changes work on multiple platforms (or I've noted platform-specific behavior)
  • I have added/updated shell completions (not applicable; --check passes)
  • I have tested with both git gtr and ./bin/gtr
  • No new external dependencies are introduced
  • All existing functionality still works

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed path handling in macOS integration tests to consistently resolve temporary repository locations.
    • Resolved cleanup failures in nested worktree scenarios caused by differences between symbolic and canonical path formats.
  • Documentation

    • Added an unreleased changelog entry describing the path canonicalization fix.

cmd_clean --merged uses nested registered worktree path failed on macOS
and passed in CI. setup_integration_repo took its path straight from
mktemp -d, which on macOS returns /var/folders/... while /var is a
symlink to /private/var. git canonicalizes when it registers a worktree,
so the hook received /private/var/... and the assertion compared it
against the /var/... spelling.

Verified this is the test's expectation being wrong rather than a
product bug: passing /var/... to `git worktree add` makes git itself
register /private/var/..., so the canonical path a hook receives is
git's own behavior.

Resolving the path in the helper fixes it for every integration test
rather than patching the one assertion. Linux is unaffected, since
mktemp -d returns /tmp with no symlink to resolve.

Full suite now passes locally: 561/561, previously 560/561.
@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 14dae5ab-870f-42df-8028-036890b0f450

📥 Commits

Reviewing files that changed from the base of the PR and between f4df634 and 804aa4f.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • tests/test_helper.bash

Included review availability: Your plan provides up to 100 included reviews per hour; 98 remain after this review.


Walkthrough

The integration helper now canonicalizes temporary repository paths before storing them. This aligns macOS paths with Git’s registered worktree paths. The changelog documents the fix. Product code remains unchanged.

Changes

Integration test path canonicalization

Layer / File(s) Summary
Canonicalize temporary repository paths
tests/test_helper.bash, CHANGELOG.md
setup_integration_repo resolves the temporary repository path with cd -P before assigning TEST_REPO. The changelog records the macOS nested worktree cleanup test fix.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: natoboram

Merge Risk: ⚪ Minimal · up to 804aa

This test-only change aligns macOS path spellings without changing product behavior.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: canonicalizing the temporary repository path in the integration test helper.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tommy/fix-macos-clean-path-test

A rabbit hops where temp paths wind
Git finds the true path every time
Mac links no longer cause a fight
Green test rows bloom in morning light
The helper keeps the trail just right

Comment @coderabbitai help to get the list of available commands.

@helizaga
helizaga merged commit d576398 into main Sep 15, 2026
4 checks passed
@helizaga
helizaga deleted the tommy/fix-macos-clean-path-test branch September 15, 2026 01:12
ddullah added a commit to TigerEye-Enterprise/git-worktree-runner that referenced this pull request Sep 28, 2026
* Detect and recover locked worktree entries with missing directories (coderabbitai#182)

git worktree prune skips locked entries by design, so a locked worktree
whose directory was deleted (e.g. a crashed agent session) lingers in the
registry and keeps its branch checked out. clean now detects this, offers
to unlock and prune (auto-confirmed by --force/--yes, previewed by
--dry-run), and prints the manual recovery command when declined.

* feat(clean): support closed PR cleanup (coderabbitai#183)

* feat(clean): support closed PR cleanup

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* fix(clean): address closed cleanup review nits

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* fix(clean): match GitLab head_sha fallback

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

---------

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* Release v2.8.0

* Replace Homebrew bump action with direct formula update (coderabbitai#184)

* Replace Homebrew bump action with direct formula update

* Allow manual formula bump via workflow_dispatch

* Return non-zero when git gtr rm fails (coderabbitai#190)

* Fix rm exit status on removal failures

* Test public rm failure status

* chore: prepare v2.8.1 release

* fix(copy): bound includeDirs discovery (coderabbitai#191)

* fix(copy): bound includeDirs discovery

* test(copy): allow filesystem result order

* fix(copy): preserve basename fallback

* chore: prepare v2.8.2 release

* feat: inherit sparse-checkout in new worktrees (coderabbitai#186)

* feat: inherit sparse-checkout in new worktrees

When creating a new worktree from one with sparse-checkout enabled,
the new worktree inherits the cone pattern automatically. Controlled
by gtr.sparse.inherit config (default on) and --sparse/--no-sparse
flags. Adds reusable helpers for sparse-checkout replication.

* fix(sparse): address CodeRabbit review feedback on sparse-checkout inheritance

- Preserve slash-separated branch paths in _worktree_path_for_ref
- Allow fallback to top-level worktree when matching worktree is not sparse
- Add Git 2.25+ guard for sparse-checkout support with full checkout fallback
- Fix non-cone mode to use init --no-cone (git defaults to cone mode)
- Improve error handling: failed sparse inheritance now falls back to full
  checkout and hard-errors if that fails, instead of leaving --no-checkout
  worktree empty
- Add tests for slash refs, non-cone inheritance, and sparse config precedence

* fix: harden sparse-checkout inheritance

* fix: support pre-2.20 config lookup

* fix: align sparse sources with git refs

---------

Co-authored-by: Tom Elizaga <tom.elizaga@gmail.com>

* Add PR worktree checkout command (coderabbitai#187)

* feat: add pull request worktree command

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* fix: make pr worktrees gh-aware

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* fix: harden pr worktree checkout

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* fix: address pr review feedback

Assisted-by: pi:gpt-5.5
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

---------

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-authored-by: Tom Elizaga <tom.elizaga@gmail.com>

* Run CI on fork pull requests (coderabbitai#185)

* Run CI on fork pull requests

* fix(ci): minimize fork pull request permissions

* chore: prepare v2.9.0 release

* feat: add machine-readable worktree creation for agents (coderabbitai#192)

Adds stable porcelain output and hook disposition reporting for shell-native agent integrations, with tests and documentation.

* chore: prepare v2.10.0 release

* Refresh editor and AI adapter guidance (coderabbitai#193)

* Refresh editor and AI adapter guidance

* Address PR review: mark legacy adapter rows

* chore: prepare v2.11.0 release

* Use preinstalled ShellCheck in CI (coderabbitai#194)

ci: use preinstalled ShellCheck

* docs: add maintainers section to README (coderabbitai#195)

The repository has a CODEOWNERS entry but nothing in the README identifies
who maintains the project, so anyone landing on it has to check the
contributors graph to find out.

Also links the contributor list, since much of the adapter, platform and
shell-integration surface came from outside contributions.

* fix: make GTR_DEBUG actually report the failing location (coderabbitai#199)

bin/git-gtr installed an ERR trap when GTR_DEBUG was set but ran under 'set -e' alone. An ERR trap is inherited by functions, command substitutions and subshells only under 'set -E', and every command runs inside main() and a cmd_* handler, so the trap never fired.

Switch the option line to 'set -eE'. With no ERR trap installed the option has no effect, so the default path is unchanged.

Adds tests/debug_trap.bats covering the function and subshell contexts, plus silence on success, on a handled error path, and when GTR_DEBUG is unset.

* docs: refresh agent guides, README flags, and troubleshooting for v2.11 (coderabbitai#197)

Brings AGENTS.md, CLAUDE.md, README and the docs/ pages in line with what shipped in v2.9 through v2.11.

Fixes two errors: AGENTS.md pointed at a nonexistent adapters/ai/Codex.sh, and docs/troubleshooting.md told users to run 'bash -x git gtr', which cannot work because git is a binary. The architecture diagram there also had bin/git-gtr and bin/gtr the wrong way round.

Adds --sparse/--no-sparse to the README, the missing GTR_* fallback variables and the direct-read variables to docs/configuration.md, and a side-effect section to docs/agent-usage.md. AGENTS.md and CLAUDE.md now cover pr, trust, clean --closed, sparse inheritance, postCd dispatch and the current test layout.

* docs: rewrite Copilot instructions and align contributor guides with the current layout (coderabbitai#198)

The Copilot instructions predated the lib/ modularization: they described bin/gtr as a 961-line monolith with an 'open' command, cmd_* functions inside bin/gtr, six lib files, and GTR_VERSION on line 8. Rewritten as a condensed guide matching bin/git-gtr, the ten lib modules plus 18 command files, the adapter registry, pr/trust/cd handling, the --porcelain contract and the CI gates.

The per-pattern instruction files and CONTRIBUTING.md told contributors to hand-edit the three completion files, which are generated by scripts/generate-completions.sh and verified by CI. Every such place now says to edit the source and regenerate.

Also corrects four claims an independent verification pass disproved: the GTR_DEBUG behavior, the postCd dispatch paths, what 'git gtr completion zsh' prints, and the cmd_<command> dispatch rule.

* chore: prepare v2.11.1 release

* fix(tests): canonicalize the temp repo path in the integration helper (coderabbitai#200)

setup_integration_repo took its path straight from mktemp -d, which on macOS returns /var/folders/... while /var is a symlink to /private/var. git canonicalizes when it registers a worktree, so the preRemove hook received the /private/var spelling and 'cmd_clean --merged uses nested registered worktree path' compared it against the /var one.

This is the test's expectation being wrong rather than a product bug: passing a /var path to 'git worktree add' makes git itself register the resolved one.

Resolving the path once in the helper fixes it for every integration test. Linux is unaffected, since mktemp -d returns /tmp with no symlink, which is why CI stayed green.

Local suite now passes 561/561, previously 560/561.

* chore: normalize Windows shell execution for TigerEye

(cherry picked from commit 26ceb92)

* fix: make trust hashing portable and fail closed (#3)

Bead: TE-7sebjb
(cherry picked from commit 11a06a8)

---------

Co-authored-by: Tom Elizaga <tom.elizaga@gmail.com>
Co-authored-by: scarf <greenscarf005@gmail.com>
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-authored-by: Adam Wettreich <80975389+adamwett@users.noreply.github.com>
Co-authored-by: BashNetCorp <your.email@example.com>
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.

1 participant