Skip to content

Commit 4e3f724

Browse files
committed
refactor: enhance testing and command structure
- Updated CLAUDE.md to clarify testing procedures, including automated BATS tests and manual smoke tests. - Introduced new functions in `lib/args.sh` for validating positional arguments, improving error handling in command scripts. - Refactored `lib/copy.sh` to streamline file copying logic with a new `_expand_and_copy_pattern` function, enhancing maintainability. - Improved `create_worktree` and related functions in `lib/core.sh` for better folder name resolution and branch checking. - Added comprehensive tests for new and existing functionalities, ensuring robust validation of command behaviors and error handling.
1 parent 7fdcadf commit 4e3f724

19 files changed

+898
-182
lines changed

CLAUDE.md

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
1414

1515
## Testing
1616

17-
This project uses **BATS tests** for core functions and **manual smoke tests** for end-to-end workflows. After any change:
17+
This project uses **BATS tests** for core functions and **manual smoke tests** for end-to-end workflows. CI runs ShellCheck + BATS automatically on PRs (`.github/workflows/lint.yml`).
1818

1919
1. Run automated tests: `bats tests/`
20-
2. Run relevant manual smoke tests:
20+
2. Run a single test file: `bats tests/config.bats`
21+
3. Run a single test by name: `bats tests/config.bats --filter "cfg_map_to_file_key"`
22+
4. Run relevant manual smoke tests:
2123

2224
```bash
2325
./bin/gtr new test-feature # Create worktree
@@ -30,6 +32,8 @@ This project uses **BATS tests** for core functions and **manual smoke tests** f
3032

3133
For exhaustive manual testing (hooks, copy patterns, adapters, `--force`, `--from-current`, etc.), see the full checklist in CONTRIBUTING.md or `.github/instructions/testing.instructions.md`.
3234

35+
**Test files**: `adapters`, `config`, `copy_safety`, `integration_lifecycle`, `parse_args`, `provider`, `resolve_base_dir`, `sanitize_branch_name` (all in `tests/`). Shared fixtures in `tests/test_helper.bash`.
36+
3337
**Tip**: Use a disposable repo for testing to avoid polluting your working tree:
3438

3539
```bash
@@ -42,20 +46,25 @@ mkdir -p /tmp/gtr-test && cd /tmp/gtr-test && git init && git commit --allow-emp
4246
### Binary Structure
4347

4448
- `bin/git-gtr` — Thin wrapper enabling `git gtr` subcommand invocation
45-
- `bin/gtr` — Entry point (~105 lines): sources libraries and commands, contains `main()` dispatcher
49+
- `bin/gtr` — Entry point: sources libraries and commands, contains `main()` dispatcher
4650

4751
### Module Structure
4852

49-
| File | Purpose |
50-
| ------------------ | ----------------------------------------------------------------------------------------------------------- |
51-
| `lib/core.sh` | Worktree CRUD: `create_worktree`, `remove_worktree`, `list_worktrees`, `resolve_target`, `resolve_base_dir` |
52-
| `lib/config.sh` | Git config wrapper with precedence: `cfg_get`, `cfg_default`, `cfg_get_all` |
53-
| `lib/copy.sh` | File/directory copying with glob patterns: `copy_patterns`, `copy_directories` |
54-
| `lib/hooks.sh` | Hook execution: `run_hooks_in` for postCreate/preRemove/postRemove |
55-
| `lib/ui.sh` | Logging (`log_error`, `log_info`, `log_warn`), prompts, formatting |
56-
| `lib/platform.sh` | OS detection, GUI helpers |
57-
| `lib/adapters.sh` | Adapter registry, builder functions, generic fallbacks, loader functions |
58-
| `lib/commands/*.sh` | One file per subcommand: `cmd_create`, `cmd_remove`, etc. (16 files) |
53+
| File | Purpose |
54+
| ------------------- | ----------------------------------------------------------------------------------------------------------- |
55+
| `lib/ui.sh` | Logging (`log_error`, `log_info`, `log_warn`), prompts, formatting |
56+
| `lib/args.sh` | Shared argument parser: flag specs (`--flag`, `--flag: val`, aliases), populates `_arg_*` vars |
57+
| `lib/config.sh` | Git config wrapper with precedence: `cfg_get`, `cfg_default`, `cfg_get_all` |
58+
| `lib/platform.sh` | OS detection, GUI helpers |
59+
| `lib/core.sh` | Worktree CRUD: `create_worktree`, `remove_worktree`, `list_worktrees`, `resolve_target`, `resolve_base_dir` |
60+
| `lib/copy.sh` | File/directory copying with glob patterns: `copy_patterns`, `copy_directories` |
61+
| `lib/hooks.sh` | Hook execution: `run_hooks_in` for postCreate/preRemove/postRemove |
62+
| `lib/provider.sh` | Remote hosting detection (GitHub/GitLab) and CLI integration for `clean --merged` |
63+
| `lib/adapters.sh` | Adapter registry, builder functions, generic fallbacks, loader functions |
64+
| `lib/launch.sh` | Editor/AI launch orchestration: `_open_editor`, `_auto_launch_editor`, `_auto_launch_ai` |
65+
| `lib/commands/*.sh` | One file per subcommand: `cmd_create`, `cmd_remove`, etc. (16 files) |
66+
67+
Libraries are sourced in the order listed above (ui → args → config → ... → launch → commands/\*.sh glob).
5968

6069
### Adapters
6170

@@ -135,6 +144,17 @@ When adding commands or flags, update all three files:
135144
- `completions/_git-gtr` (Zsh)
136145
- `completions/git-gtr.fish` (Fish)
137146

147+
## Critical Gotcha: `set -e`
148+
149+
`bin/gtr` runs with `set -e`. Any unguarded non-zero return silently exits the entire script. When calling functions that may `return 1`, guard with `|| true`:
150+
151+
```bash
152+
result=$(my_func) || true # Prevents silent exit
153+
if my_func; then ...; fi # Also safe (if guards the return)
154+
```
155+
156+
This is the most common source of subtle bugs in this codebase.
157+
138158
## Code Style
139159

140160
- Shebang: `#!/usr/bin/env bash`

lib/args.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,13 @@ EOF
123123
esac
124124
done
125125
}
126+
127+
# Validate that at least N positional arguments were provided.
128+
# Call after parse_args. Exits with error message if insufficient args.
129+
# Usage: require_args <min_count> "<usage message>"
130+
require_args() {
131+
if [ "${#_pa_positional[@]}" -lt "$1" ]; then
132+
log_error "$2"
133+
exit 1
134+
fi
135+
}

lib/commands/ai.sh

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,12 @@
44
# shellcheck disable=SC2154 # _arg_* _pa_* set by parse_args, _ctx_* set by resolve_*
55
cmd_ai() {
66
parse_args "--ai: value" "$@"
7+
require_args 1 "Usage: git gtr ai <id|branch> [--ai <name>] [-- args...]"
78

8-
local identifier="${_pa_positional[0]:-}"
9+
local identifier="${_pa_positional[0]}"
910
local ai_tool="${_arg_ai:-}"
1011
local -a ai_args=("${_pa_passthrough[@]}")
1112

12-
if [ -z "$identifier" ]; then
13-
log_error "Usage: git gtr ai <id|branch> [--ai <name>] [-- args...]"
14-
exit 1
15-
fi
16-
1713
# Get AI tool from flag or config (with .gtrconfig support)
1814
if [ -z "$ai_tool" ]; then
1915
ai_tool=$(_cfg_ai_default)

lib/commands/clean.sh

Lines changed: 57 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,76 +1,86 @@
11
#!/usr/bin/env bash
22

33
# Clean command (remove prunable worktrees)
4+
5+
# Detect hosting provider with error messaging.
6+
# Prints provider name on success; returns 1 on failure.
7+
_clean_detect_provider() {
8+
local provider
9+
provider=$(detect_provider) || true
10+
if [ -n "$provider" ]; then
11+
printf "%s" "$provider"
12+
return 0
13+
fi
14+
15+
local remote_url
16+
remote_url=$(git remote get-url origin 2>/dev/null || true)
17+
if [ -z "$remote_url" ]; then
18+
log_error "No remote URL configured for 'origin'"
19+
else
20+
# Sanitize URL to avoid leaking embedded credentials (e.g., https://token@host/...)
21+
local safe_url="${remote_url%%@*}"
22+
if [ "$safe_url" != "$remote_url" ]; then
23+
safe_url="<redacted>@${remote_url#*@}"
24+
fi
25+
log_error "Could not detect hosting provider from remote URL: $safe_url"
26+
log_info "Set manually: git gtr config set gtr.provider github (or gitlab)"
27+
fi
28+
return 1
29+
}
30+
31+
# Check if a worktree should be skipped during merged cleanup.
32+
# Returns 0 if should skip, 1 if should process.
33+
# Usage: _clean_should_skip <dir> <branch>
34+
_clean_should_skip() {
35+
local dir="$1" branch="$2"
36+
37+
if [ -z "$branch" ] || [ "$branch" = "(detached)" ]; then
38+
log_warn "Skipping $dir (detached HEAD)"
39+
return 0
40+
fi
41+
42+
if ! git -C "$dir" diff --quiet 2>/dev/null || \
43+
! git -C "$dir" diff --cached --quiet 2>/dev/null; then
44+
log_warn "Skipping $branch (has uncommitted changes)"
45+
return 0
46+
fi
47+
48+
if [ -n "$(git -C "$dir" ls-files --others --exclude-standard 2>/dev/null)" ]; then
49+
log_warn "Skipping $branch (has untracked files)"
50+
return 0
51+
fi
52+
53+
return 1
54+
}
55+
456
# Remove worktrees whose PRs/MRs are merged (handles squash merges)
557
# Usage: _clean_merged repo_root base_dir prefix yes_mode dry_run
658
_clean_merged() {
759
local repo_root="$1" base_dir="$2" prefix="$3" yes_mode="$4" dry_run="$5"
860

961
log_step "Checking for worktrees with merged PRs/MRs..."
1062

11-
# Detect hosting provider (GitHub, GitLab, etc.)
1263
local provider
13-
provider=$(detect_provider) || true
14-
if [ -z "$provider" ]; then
15-
local remote_url
16-
remote_url=$(git remote get-url origin 2>/dev/null || true)
17-
if [ -z "$remote_url" ]; then
18-
log_error "No remote URL configured for 'origin'"
19-
else
20-
# Sanitize URL to avoid leaking embedded credentials (e.g., https://token@host/...)
21-
local safe_url="${remote_url%%@*}"
22-
if [ "$safe_url" != "$remote_url" ]; then
23-
safe_url="<redacted>@${remote_url#*@}"
24-
fi
25-
log_error "Could not detect hosting provider from remote URL: $safe_url"
26-
log_info "Set manually: git gtr config set gtr.provider github (or gitlab)"
27-
fi
28-
exit 1
29-
fi
30-
31-
# Ensure provider CLI is available and authenticated
64+
provider=$(_clean_detect_provider) || exit 1
3265
ensure_provider_cli "$provider" || exit 1
3366

34-
# Fetch latest from origin
3567
log_step "Fetching from origin..."
3668
git fetch origin --prune 2>/dev/null || log_warn "Could not fetch from origin"
3769

38-
local removed=0
39-
local skipped=0
40-
41-
# Get main repo branch to exclude it
70+
local removed=0 skipped=0
4271
local main_branch
4372
main_branch=$(current_branch "$repo_root")
4473

45-
# Iterate through worktree directories
4674
for dir in "$base_dir/${prefix}"*; do
4775
[ -d "$dir" ] || continue
4876

4977
local branch
5078
branch=$(current_branch "$dir") || true
5179

52-
if [ -z "$branch" ] || [ "$branch" = "(detached)" ]; then
53-
log_warn "Skipping $dir (detached HEAD)"
54-
skipped=$((skipped + 1))
55-
continue
56-
fi
57-
58-
# Skip if same as main repo branch
59-
if [ "$branch" = "$main_branch" ]; then
60-
continue
61-
fi
62-
63-
# Check if worktree has uncommitted changes
64-
if ! git -C "$dir" diff --quiet 2>/dev/null || \
65-
! git -C "$dir" diff --cached --quiet 2>/dev/null; then
66-
log_warn "Skipping $branch (has uncommitted changes)"
67-
skipped=$((skipped + 1))
68-
continue
69-
fi
80+
# Skip main repo branch silently (not counted)
81+
[ "$branch" = "$main_branch" ] && continue
7082

71-
# Check for untracked files
72-
if [ -n "$(git -C "$dir" ls-files --others --exclude-standard 2>/dev/null)" ]; then
73-
log_warn "Skipping $branch (has untracked files)"
83+
if _clean_should_skip "$dir" "$branch"; then
7484
skipped=$((skipped + 1))
7585
continue
7686
fi
@@ -83,7 +93,6 @@ _clean_merged() {
8393
elif [ "$yes_mode" -eq 1 ] || prompt_yes_no "Remove worktree and delete branch '$branch'?"; then
8494
log_step "Removing worktree: $branch"
8595

86-
# Run pre-remove hooks (skip worktree on failure)
8796
if ! run_hooks_in preRemove "$dir" \
8897
REPO_ROOT="$repo_root" \
8998
WORKTREE_PATH="$dir" \
@@ -94,11 +103,9 @@ _clean_merged() {
94103
fi
95104

96105
if remove_worktree "$dir" 0; then
97-
# Delete the local branch (safe: already merged)
98106
git branch -d "$branch" 2>/dev/null || git branch -D "$branch" 2>/dev/null || true
99107
removed=$((removed + 1))
100108

101-
# Run post-remove hooks (worktree already removed, don't abort)
102109
if ! run_hooks postRemove \
103110
REPO_ROOT="$repo_root" \
104111
WORKTREE_PATH="$dir" \
@@ -111,7 +118,6 @@ _clean_merged() {
111118
skipped=$((skipped + 1))
112119
fi
113120
fi
114-
# Branches without merged PRs are silently skipped (this is the normal case)
115121
done
116122

117123
echo ""

lib/commands/editor.sh

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,11 @@
44
# shellcheck disable=SC2154 # _arg_* _pa_* set by parse_args, _ctx_* set by resolve_*
55
cmd_editor() {
66
parse_args "--editor: value" "$@"
7+
require_args 1 "Usage: git gtr editor <id|branch> [--editor <name>]"
78

8-
local identifier="${_pa_positional[0]:-}"
9+
local identifier="${_pa_positional[0]}"
910
local editor="${_arg_editor:-}"
1011

11-
if [ -z "$identifier" ]; then
12-
log_error "Usage: git gtr editor <id|branch> [--editor <name>]"
13-
exit 1
14-
fi
15-
1612
# Get editor from flag or config (with .gtrconfig support)
1713
if [ -z "$editor" ]; then
1814
editor=$(_cfg_editor_default)

lib/commands/go.sh

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@
44
# shellcheck disable=SC2154 # _pa_* set by parse_args, _ctx_* set by resolve_*
55
cmd_go() {
66
parse_args "" "$@"
7-
8-
if [ "${#_pa_positional[@]}" -ne 1 ]; then
9-
log_error "Usage: git gtr go <id|branch>"
10-
exit 1
11-
fi
7+
require_args 1 "Usage: git gtr go <id|branch>"
128

139
local identifier="${_pa_positional[0]}"
1410
resolve_repo_context || exit 1

lib/commands/remove.sh

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,12 @@ cmd_remove() {
99
--force"
1010
parse_args "$_spec" "$@"
1111

12+
require_args 1 "Usage: git gtr rm <id|branch> [<id|branch>...] [--delete-branch] [--force] [--yes]"
13+
1214
local delete_branch="${_arg_delete_branch:-0}"
1315
local yes_mode="${_arg_yes:-0}"
1416
local force="${_arg_force:-0}"
1517

16-
if [ ${#_pa_positional[@]} -eq 0 ]; then
17-
log_error "Usage: git gtr rm <id|branch> [<id|branch>...] [--delete-branch] [--force] [--yes]"
18-
exit 1
19-
fi
20-
2118
resolve_repo_context || exit 1
2219

2320
local repo_root="$_ctx_repo_root" base_dir="$_ctx_base_dir" prefix="$_ctx_prefix"

lib/commands/rename.sh

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,13 @@ cmd_rename() {
88
--yes"
99
parse_args "$_spec" "$@"
1010

11-
local old_identifier="${_pa_positional[0]:-}"
12-
local new_name="${_pa_positional[1]:-}"
11+
require_args 2 "Usage: git gtr mv <old> <new> [--force] [--yes]"
12+
13+
local old_identifier="${_pa_positional[0]}"
14+
local new_name="${_pa_positional[1]}"
1315
local force="${_arg_force:-0}"
1416
local yes_mode="${_arg_yes:-0}"
1517

16-
# Validate arguments
17-
if [ -z "$old_identifier" ] || [ -z "$new_name" ]; then
18-
log_error "Usage: git gtr mv <old> <new> [--force] [--yes]"
19-
exit 1
20-
fi
21-
2218
resolve_repo_context || exit 1
2319

2420
local repo_root="$_ctx_repo_root" base_dir="$_ctx_base_dir" prefix="$_ctx_prefix"

0 commit comments

Comments
 (0)