diff --git a/docs/test-coverage-gaps.md b/docs/test-coverage-gaps.md new file mode 100644 index 0000000..9ecba46 --- /dev/null +++ b/docs/test-coverage-gaps.md @@ -0,0 +1,215 @@ +# git_pruner — test coverage gaps + +An analysis of `main_test.go` (2026-08-17) proposing tests for everyday git edge cases the +suite does not cover. Git-behavior claims below were reproduced in throwaway repositories +before being recorded, following the same rule as `improvements.md`. + +## Current coverage assessment + +The 25 existing tests are strong on the deletion-safety core: safe-vs-force semantics +ground-truthed against real git (`TestSafeDeletableMatchesGit`), gone-branch risk guarding, +deferred remote deletes, the force-retry flow, and rendering width invariants. + +What is missing is almost entirely **environmental variation**: every test runs in the same +happy-path repo shape — clean checkout on `main`, one healthy file-URL remote named +`origin`/`upstream`, born HEAD, no tags. + +--- + +## Tier 1 — expose confirmed or likely bugs — **DONE 2026-08-17** + +### 1. Tag shadowing a branch name *(was a confirmed bug — fixed)* + +A tag sharing a branch's name silently shadowed the branch: git's ref search order puts +`refs/tags/` ahead of `refs/heads/`, and `riskCommitCount` / `loadDiff` passed +bare names to `git cherry` and `git diff`. Reproduced: `git cherry main feat` reported +**zero** at-risk commits for a branch that had one, with only a `refname 'feat' is +ambiguous` warning on stderr — so the risk warning said nothing was at stake on exactly the +branch a `-D` was about to destroy. Tagging a release branch with its own name (`v1.2`, +`release-3`) makes this an everyday shape. + +Writing the test exposed a **deeper root cause than the bare-name calls**: `loadBranches` +read `%(refname:short)`, which returns the shortest *unambiguous* name. The moment the tag +exists, the branch loads as `heads/feature/x` rather than `feature/x`, which breaks every +name-keyed lookup and any ref rebuilt from the name. `%(upstream:short)` has the same +failure, disambiguating to `remotes/origin/x` and splitting into a bogus remote named +`remotes`. + +Fixed by reading full refs and stripping the namespace ourselves: + +- `shortRef()` strips `refs/heads/` / `refs/remotes/`; `loadBranches` now reads + `%(refname)` and `%(upstream)`, and `mergedSet` reads `%(refname)`. +- `branchRef()` qualifies a local branch name for git; used by `riskCommitCount` and + `loadDiff`. +- `qualifyRef()` probes `refs/heads/` then `refs/remotes/` for refs whose namespace is not + known statically (the base). Resolved once into the new `model.riskBaseRef`, since it + costs up to two `rev-parse` calls. + +`riskBase` stays short for display; git is handed the qualified form. `git branch -d` was +never affected — it always operates in `refs/heads/`. + +Covered by `TestTagShadowingBranchName` (`branch_name`, `upstream_name`, `base_name`). + +### 2. Selections survive `p` *(was a real wipe — fixed)* + +`fetchDoneMsg` called `applyBranches` with freshly loaded structs, so every manually +selected / `R`-armed branch was silently wiped when the user pressed `p`. A fetch mutates +nothing local, so the marks now survive it: `carryMarks()` copies `selected` and +`deleteRemote` onto the new set by name. + +One deliberate exception: an armed remote delete is **dropped** when the fetch reveals the +upstream is gone, because the push it would run can only fail. + +Covered by `TestFetchPreservesSelections` and `TestFetchDisarmsRemoteForGoneBranch`. + +### 3. Gone *current* branch *(behavior was already correct — now locked in)* + +The user is sitting on `feature/x`, the PR merged, the remote branch was deleted, and they +press `p`. The fetch handler's `isCurrent` guard already kept it out of auto-selection and +out of the "press d to prune" count, but nothing tested it. The test also pins the +second line of defence: selected by hand, git refuses the delete, and because the refusal +is not "not fully merged" it correctly does **not** raise the force prompt. + +Covered by `TestGoneCurrentBranchIsNotPruned`. + +--- + +## Tier 2 — everyday repo environments never constructed in tests — **DONE 2026-08-17** + +No new production bugs here: every shape already behaved correctly, and the tests now pin +that. Two of the predictions above were wrong about git's behavior and were corrected +against real repos before being written up. + +### 4. No remotes at all *(covered by `TestLocalOnlyRepo`)* + +A scratch or never-pushed project. `initialModel` works, `riskBase` falls back to local +`main` (with `riskBaseRef` `refs/heads/main`), `loadDiff` bases on local `main`, the risk +warning still names the real cost, and `p` succeeds as a no-op — `git fetch --all --prune` +exits 0 with no remotes configured. + +### 5. Unborn HEAD *(covered by `TestUnbornHeadRepo`)* + +`git init`, zero commits: empty branch list, no base resolves, `listView` renders the empty +message, and every cursor/selection key is a no-op rather than a panic. `git branch --merged +HEAD` hard-fails here (`fatal: malformed object name HEAD`); `mergedSet` absorbs it. + +### 6. Detached HEAD *(covered by `TestDetachedHead`)* + +Mid-bisect, mid-rebase, or on a checked-out tag. No branch is `isCurrent`, so `a` selects +everything — there is no current branch to spare — and `headMerged` is computed against the +detached commit. Deleting the branch HEAD is parked on is legal and safe: the commits stay +reachable from HEAD. + +Noted: `git branch --merged HEAD --format=%(refname)` emits a `(HEAD detached at …)` +pseudo-entry, which lands in `mergedSet` as a junk key. Harmless — branch names cannot +contain spaces, so it can never collide with a real lookup. + +### 7. Remote default is neither `main` nor `master`, `origin/HEAD` unset *(covered by `TestNonStandardDefaultBranch`)* + +E.g. `trunk`/`develop`. `origin/HEAD` only exists after a clone, not after `remote add` + +push — verified: `symbolic-ref refs/remotes/origin/HEAD` fails with `not a symbolic ref`. +With no `origin/main`, `origin/master`, or local `main`/`master` either, `riskBase == ""`, +which is the path both "no base branch to compare against" messages hang off. Now asserted +in `riskWarning`, `confirmView`, and `forcePromptView`. + +The second subtest covers `remoteDefault`'s `symbolic-ref` path, which had **no coverage at +all** — `setupRepo` never sets `origin/HEAD`. After `git remote set-head origin trunk` it +resolves to `origin/trunk`. + +### 8. Upstream's remote no longer exists *(covered by `TestUnreachableRemote`)* + +**Correction:** the prediction that `git remote remove origin` leaves the branch showing +"gone" is wrong. Removing a remote also unsets `branch..remote`/`merge` and deletes +its remote-tracking refs, so the branches simply become upstream-less — which is case 4, +not a distinct one. + +The case that does exercise the error path is a remote that is still configured but +**unreachable** (server moved, repo deleted, laptop offline). Remote-tracking refs are +local, so the upstream and all merge state survive; only network operations fail. The test +pins that a failed fetch reaches `m.err` and clears `fetching` so `p` can be pressed again, +and that a failed `push --delete` is captured in `remoteErr` and reported on the results +screen without taking the successful local delete down with it. + +This also closes Tier 3 item 11 (fetch failure), which shares the shape. + +--- + +## Tier 3 — everyday operations on the happy repo — **DONE 2026-08-17** + +### 9. Diverged branch (ahead AND behind) *(covered by `TestDivergedBranch`)* + +`trackRe`'s `behind` capture group and `sortAheadBehind` had only ever seen hand-built +structs. Now driven from real `[ahead 2, behind 1]` output, asserting both arrows render and +that a branch ahead of its upstream is not safely deletable. + +### 10. Remote delete race *(covered by `TestRemoteDeleteRace`)* + +**Correction:** the prediction that this surfaces an error is no longer true, because of the +qualified refspec introduced in the simplify pass (below). `git push origin --delete +refs/heads/x` treats an already-absent ref as a no-op and exits 0, where the bare form fails +with "remote ref does not exist". + +That is a deliberate trade: the bare form's error is more informative in this one case, but +the bare form *also silently deletes nothing* when the remote carries a tag of the same name +(git rejects it as "src refspec matches more than one"). Idempotence in the race is worth +more than a message, since the race ends in exactly the state the user armed. The test pins +the idempotent outcome; `TestUnreachableRemote` still covers genuine push failures. + +### 11. Fetch failure — **DONE**, see Tier 2 item 8 + +### 12. Current-branch UI protection *(covered by `TestCurrentBranchCannotBeMarked`)* + +`space`, `r`, and `a` all skipping `isCurrent` was only ever implicit in other tests. Now +explicit: the checked-out branch can never be marked or swept into a select-all. + +### 13. `loadDiff` HEAD fallback *(covered by `TestLoadDiffFallsBackToHead`)* + +Reached with `setupTrunkRepo`, where no remote default and no local `main`/`master` resolve. +Asserts the diff is still correct against HEAD and that the diff header names the base it +actually used. + +### 14. Odd-but-legal content *(covered by `TestUnicodeNameAndEmptySubject`)* + +An empty commit subject (`--allow-empty-message`) leaves a trailing empty field in the +NUL-delimited `for-each-ref` output — a stricter field count would drop the whole branch. +Combined with a multibyte branch name, pinned end to end through parse, render, and delete. + +--- + +## Ref handling — the rule this work established + +**Never use git's `%(refname:short)`, `%(upstream:short)`, or `symbolic-ref --short`.** They +return the shortest *unambiguous* name, which silently grows a `heads/` or `remotes/` prefix +the moment a tag shares the name. Instead: + +- Resolvers return **fully qualified refs**. `remoteDefault`, `localDefaultBranch` and + `baseBranch` all do; resolution is the only point where the namespace is known for + certain, so it is carried forward from there rather than re-guessed later. +- `shortRef()` shortens at the **display boundary** only. The model keeps both forms: + `riskBase`/`remoteDefault` for views, `riskBaseRef` for git. +- `branchRef()` qualifies a local branch name. Correct because `loadBranches` only ever + reads `refs/heads`, so the mapping is total. +- `refExists()` wraps the `rev-parse --verify --quiet` probe and must be given a qualified + ref — a bare name would match a tag, which is the bug it exists to avoid. + +An earlier iteration used a `qualifyRef()` helper that probed both namespaces at the point +of *use*. That was the wrong altitude: it cost up to two subprocesses per call, silently +fell through to the shadowable bare name when both probes missed, and left the real hole +open in `localDefaultBranch`. It has been deleted. + +## Status + +All three tiers are complete (2026-08-17): **14 tests added, 43 total**, passing under +`-race`, with `gofmt` and `go vet` clean. + +- Tier 1 fixed two real bugs — tag shadowing on the risk path, and `p` wiping selections. +- Tier 2 found no new bugs; it pins five repo environments that had no coverage, and + corrected two wrong predictions in this document against real git behavior. +- Tier 3 pins five everyday operations and corrected one more prediction. +- A `/simplify` pass then found **two further live instances of the tag-shadowing bug** that + Tier 1 had missed (`localDefaultBranch` and `push --delete`), removed the `qualifyRef` + probe, and brought startup back from 11 git subprocesses to 7 — the pre-change baseline. + +Fixtures compose rather than duplicate: `initRepo` (bare init + identity) → +`setupLocalRepo` (branch shapes) → `setupRepo` (+ `addOrigin` + a tracking branch), with +`setupTrunkRepo` reusing `initRepo` and `addOrigin`. diff --git a/main.go b/main.go index 38dfd42..d7e490e 100644 --- a/main.go +++ b/main.go @@ -133,6 +133,7 @@ type model struct { remoteDefault string // resolved remote default branch, e.g. "origin/main" riskBase string // ref that branch.riskCommits is measured against ("" if unresolved) + riskBaseRef string // riskBase fully qualified, so a same-named tag cannot shadow it spinnerFrame int // animation frame for the deleting spinner (deletion counts derive from results) @@ -192,11 +193,39 @@ func runGit(args ...string) (string, error) { return out.String(), nil } +// branchRef fully qualifies a local branch name. git's ref search order puts +// refs/tags/ ahead of refs/heads/, so a tag sharing a branch's name +// silently shadows the branch in any command handed the bare name — and tagging a +// release branch with its own name is ordinary practice. +func branchRef(name string) string { return "refs/heads/" + name } + +// shortRef strips the namespace from a full ref, yielding the plain branch name +// ("feature/x") or remote-tracking name ("origin/feature/x") the rest of the +// program keys on. git's own %(refname:short) cannot be used for this: it yields +// the shortest *unambiguous* name, which grows a "heads/" or "remotes/" prefix +// exactly when a tag shares the name — silently breaking every name-keyed lookup +// and every ref built back up from it. +func shortRef(ref string) string { + for _, prefix := range []string{"refs/heads/", "refs/remotes/"} { + if s, ok := strings.CutPrefix(ref, prefix); ok { + return s + } + } + return ref +} + +// refExists reports whether a ref resolves. Always pass a fully qualified ref: a +// bare name would also match a tag (see branchRef). +func refExists(ref string) bool { + _, err := runGit("rev-parse", "--verify", "--quiet", ref) + return err == nil +} + var trackRe = regexp.MustCompile(`ahead (\d+)|behind (\d+)`) func loadBranches() ([]branch, error) { - const format = "%(refname:short)%00%(objectname:short)%00%(committerdate:iso8601-strict)%00" + - "%(committerdate:relative)%00%(upstream:short)%00%(upstream:track)%00%(HEAD)%00%(contents:subject)" + const format = "%(refname)%00%(objectname:short)%00%(committerdate:iso8601-strict)%00" + + "%(committerdate:relative)%00%(upstream)%00%(upstream:track)%00%(HEAD)%00%(contents:subject)" out, err := runGit("for-each-ref", "--format="+format, "refs/heads") if err != nil { return nil, err @@ -211,10 +240,10 @@ func loadBranches() ([]branch, error) { continue } b := branch{ - name: f[0], + name: shortRef(f[0]), hash: f[1], committedRel: f[3], - upstream: f[4], + upstream: shortRef(f[4]), isCurrent: f[6] == "*", subject: f[7], } @@ -256,43 +285,52 @@ func remotes() []string { return names } -// localDefaultBranch returns a local main/master, skipping exclude so a branch -// is never compared against itself. Returns "" when neither exists. +// localDefaultBranch returns the ref of a local main/master, skipping exclude (a +// short branch name) so a branch is never compared against itself. Returns "" +// when neither exists. +// +// The resolvers below all return fully qualified refs, and the display layer +// shortens them with shortRef. Resolving is the only place the namespace is +// known for certain, so carrying it forward from here is what keeps a same-named +// tag from being measured in place of the branch further down. func localDefaultBranch(exclude string) string { for _, c := range []string{"main", "master"} { if c == exclude { continue } - if _, err := runGit("rev-parse", "--verify", "--quiet", c); err == nil { - return c + if ref := branchRef(c); refExists(ref) { + return ref } } return "" } // remoteDefault resolves the remote's default branch as a remote-tracking ref -// (e.g. "origin/main"): /HEAD if set, else /main, else -// /master, trying each remote in turn. Returns "" when none can be found. +// (e.g. "refs/remotes/origin/main"): /HEAD if set, else /main, +// else /master, trying each remote in turn. Returns "" when none can be +// found. func remoteDefault() string { for _, r := range remotes() { - if out, err := runGit("symbolic-ref", "--short", "refs/remotes/"+r+"/HEAD"); err == nil { + // Deliberately not symbolic-ref --short: it shortens to the shortest + // *unambiguous* name, which a same-named tag turns into "remotes/origin/main". + if out, err := runGit("symbolic-ref", "refs/remotes/"+r+"/HEAD"); err == nil { if s := strings.TrimSpace(out); s != "" { return s } } for _, c := range []string{r + "/main", r + "/master"} { - if _, err := runGit("rev-parse", "--verify", "--quiet", "refs/remotes/"+c); err == nil { - return c + if ref := "refs/remotes/" + c; refExists(ref) { + return ref } } } return "" } -// baseBranch returns a reference to diff a branch against: the remote default -// branch, else a local main/master, excluding name itself. +// baseBranch returns the ref to diff a branch against: the remote default branch, +// else a local main/master, excluding name itself. func baseBranch(name string) string { - if def := remoteDefault(); def != "" && def != name { + if def := remoteDefault(); def != "" { return def } return localDefaultBranch(name) @@ -306,10 +344,11 @@ func baseBranch(name string) string { // the warning is therefore worded as "not in ", not "will be lost". // Returns 0 when there is nothing to compare against. func riskCommitCount(name, base string) int { - if base == "" || base == name { + ref := branchRef(name) + if base == "" || base == ref { return 0 } - out, err := runGit("cherry", base, name) + out, err := runGit("cherry", base, ref) if err != nil { return 0 } @@ -326,21 +365,21 @@ func riskCommitCount(name, base string) int { // it reports into a set. func mergedSet(args ...string) map[string]bool { set := map[string]bool{} - out, err := runGit(append(args, "--format=%(refname:short)")...) + out, err := runGit(append(args, "--format=%(refname)")...) if err != nil { return set } for _, line := range strings.Split(out, "\n") { if s := strings.TrimSpace(line); s != "" { - set[s] = true + set[shortRef(s)] = true } } return set } // remoteMergedSet returns the set of remote-tracking branches (short names, e.g. -// "origin/feature") whose tip is merged into def. Operates on local -// remote-tracking refs, so it needs no network — it reflects the last fetch. +// "origin/feature") whose tip is merged into def, a qualified ref. Operates on +// local remote-tracking refs, so it needs no network — it reflects the last fetch. func remoteMergedSet(def string) map[string]bool { if def == "" { return map[string]bool{} @@ -394,14 +433,15 @@ func spinnerTickCmd() tea.Cmd { } // loadDiff returns the patch introduced on name relative to its merge-base with -// the repo's default branch — i.e. what the branch contains — and the base ref used. +// the repo's default branch — i.e. what the branch contains — and the base it was +// compared against, shortened for display. func loadDiff(name string) (diff, base string, err error) { - base = baseBranch(name) - if base == "" { - base = "HEAD" + ref := baseBranch(name) + if ref == "" { + ref = "HEAD" } - diff, err = runGit("diff", base+"..."+name) - return diff, base, err + diff, err = runGit("diff", ref+"..."+branchRef(name)) + return diff, shortRef(ref), err } // ---- model ---- @@ -518,7 +558,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } if branches, err := loadBranches(); err == nil { - m.applyBranches(branches) // preserves the cursor by name (fetch is non-destructive) + // A fetch is non-destructive, so both the cursor (by name, in + // sortBranches) and the user's pending marks survive it. + m.applyBranches(carryMarks(m.branches, branches)) } // Auto-select only gone branches that carry nothing missing from the // base. Ones holding unique commits are left unselected so discarding @@ -847,7 +889,9 @@ func deleteBranch(b branch, flag string, wantRemote bool) deleteResult { func pushRemoteDelete(res *deleteResult) { res.remoteSkipped = false res.remoteTried = true - if _, err := runGit("push", res.br.remoteName(), "--delete", res.br.remoteBranch()); err != nil { + // Qualify the remote branch: a remote carrying both a branch and a tag of that + // name rejects a bare refspec as matching more than one ref, deleting nothing. + if _, err := runGit("push", res.br.remoteName(), "--delete", branchRef(res.br.remoteBranch())); err != nil { res.remoteErr = err.Error() } else { res.remoteOK = true @@ -867,6 +911,25 @@ func (m *model) performDeletions() { m.reloadBranches() } +// carryMarks copies the user's pending selections from old onto a freshly loaded +// branch set, matching by name. Used on the fetch path, which reloads every +// branch struct but changes nothing the marks were made about. An armed remote +// delete is dropped when the fetch reveals the upstream is already gone: the push +// it would run can only fail. +func carryMarks(old, fresh []branch) []branch { + prev := make(map[string]branch, len(old)) + for _, b := range old { + prev[b.name] = b + } + for i := range fresh { + if p, ok := prev[fresh[i].name]; ok { + fresh[i].selected = p.selected + fresh[i].deleteRemote = p.deleteRemote && !fresh[i].gone + } + } + return fresh +} + // applyBranches installs a freshly-loaded branch set and recomputes everything // derived from it (name-width, merge info, sort order). Callers set their own // cursor policy around it. This is the single refresh core shared by @@ -892,14 +955,17 @@ func (m *model) reloadBranches() { // upstream is merged into it or whose tip is merged into HEAD, and measures what // deleting it would cost. Call after every branch (re)load. func (m *model) refreshMergeInfo() { - m.remoteDefault = remoteDefault() - merged := remoteMergedSet(m.remoteDefault) + defRef := remoteDefault() + merged := remoteMergedSet(defRef) headMerged := localMergedSet() - m.riskBase = m.remoteDefault - if m.riskBase == "" { - m.riskBase = localDefaultBranch("") + m.riskBaseRef = defRef + if m.riskBaseRef == "" { + m.riskBaseRef = localDefaultBranch("") } + // The refs drive git; the short forms are what the views print. + m.remoteDefault = shortRef(defRef) + m.riskBase = shortRef(m.riskBaseRef) for i := range m.branches { b := &m.branches[i] @@ -921,7 +987,7 @@ func (m *model) measureRisk(b *branch) { if b.riskMeasured { return } - b.riskCommits = riskCommitCount(b.name, m.riskBase) + b.riskCommits = riskCommitCount(b.name, m.riskBaseRef) b.riskMeasured = true } diff --git a/main_test.go b/main_test.go index 0bf3781..f72dace 100644 --- a/main_test.go +++ b/main_test.go @@ -51,21 +51,45 @@ func git(t *testing.T, dir string, args ...string) { } } -func setupRepo(t *testing.T) string { +// initRepo creates an empty repository on the named initial branch with a commit +// identity configured. Callers add the commits, remotes and branches they need. +func initRepo(t *testing.T, branch string) string { t.Helper() tmp := t.TempDir() - remote := t.TempDir() - git(t, remote, "init", "--bare", "-q") - git(t, tmp, "init", "-q", "-b", "main") + git(t, tmp, "init", "-q", "-b", branch) git(t, tmp, "config", "user.email", "t@t.t") git(t, tmp, "config", "user.name", "t") + return tmp +} + +// addOrigin gives dir a bare origin remote and pushes trunk to it. +func addOrigin(t *testing.T, dir, trunk string) { + t.Helper() + remote := t.TempDir() + git(t, remote, "init", "--bare", "-q", "-b", trunk) + git(t, dir, "remote", "add", "origin", remote) + git(t, dir, "push", "-q", "-u", "origin", trunk) +} + +// setupLocalRepo builds the branch shapes with no remote at all — a scratch +// project, or one that has simply never been pushed. +func setupLocalRepo(t *testing.T) string { + t.Helper() + tmp := initRepo(t, "main") commitFile(t, tmp, "a", "a") - git(t, tmp, "remote", "add", "origin", remote) - git(t, tmp, "push", "-q", "-u", "origin", "main") git(t, tmp, "branch", "feature/merged") // merged into main -> safe delete git(t, tmp, "checkout", "-q", "-b", "feature/unmerged") commitFile(t, tmp, "b", "b") - git(t, tmp, "checkout", "-q", "-b", "feature/tracked") + git(t, tmp, "checkout", "-q", "main") + return tmp +} + +// setupRepo is setupLocalRepo plus an origin and a branch tracking it. +func setupRepo(t *testing.T) string { + t.Helper() + tmp := setupLocalRepo(t) + addOrigin(t, tmp, "main") + git(t, tmp, "checkout", "-q", "-b", "feature/tracked", "feature/unmerged") git(t, tmp, "push", "-q", "-u", "origin", "feature/tracked") git(t, tmp, "checkout", "-q", "main") return tmp @@ -80,6 +104,14 @@ func chdir(t *testing.T, dir string) { t.Cleanup(func() { os.Chdir(old) }) } +func branchNames(bs []branch) []string { + out := make([]string, len(bs)) + for i, b := range bs { + out[i] = b.name + } + return out +} + func find(bs []branch, name string) *branch { for i := range bs { if bs[i].name == name { @@ -114,10 +146,7 @@ func TestLoadAndSort(t *testing.T) { m.field = sortName m.ascending = true m.sortBranches() - got := make([]string, len(m.branches)) - for i, b := range m.branches { - got[i] = b.name - } + got := branchNames(m.branches) want := []string{"feature/merged", "feature/tracked", "feature/unmerged", "main"} for i := range want { if got[i] != want[i] { @@ -726,10 +755,11 @@ func TestNonOriginRemoteResolves(t *testing.T) { git(t, repo, "remote", "rename", "origin", "upstream") git(t, repo, "fetch", "-q", "--all", "--prune") - if got := remoteDefault(); got != "upstream/main" { + // The resolvers hand back qualified refs; the model shortens them for display. + if got := remoteDefault(); got != "refs/remotes/upstream/main" { t.Fatalf("remoteDefault should resolve upstream/main, got %q", got) } - if got := baseBranch("feature/unmerged"); got != "upstream/main" { + if got := baseBranch("feature/unmerged"); got != "refs/remotes/upstream/main" { t.Fatalf("baseBranch should use the non-origin remote, got %q", got) } m, err := initialModel() @@ -739,6 +769,9 @@ func TestNonOriginRemoteResolves(t *testing.T) { if m.remoteDefault != "upstream/main" || m.riskBase != "upstream/main" { t.Fatalf("model should cache the resolved default: %q / %q", m.remoteDefault, m.riskBase) } + if m.riskBaseRef != "refs/remotes/upstream/main" { + t.Fatalf("model should cache the ref form too: %q", m.riskBaseRef) + } } // origin is preferred when several remotes are configured. @@ -1043,6 +1076,784 @@ func TestNonUnmergedFailureIsNotForceable(t *testing.T) { } } +// A tag sharing a branch's name shadows it: git resolves refs/tags/ before +// refs/heads/, so any bare name handed to git measures the tag instead. On +// the risk path that reports a branch's unmerged commits as already safe — +// precisely when -D is about to discard them. Release branches tagged with their +// own name (v1.2, release-3) make this an everyday shape. +func TestTagShadowingBranchName(t *testing.T) { + t.Run("branch_name", func(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + // Tag the base commit with the name of a branch that is one commit ahead. + git(t, repo, "tag", "feature/unmerged", "main") + + if n := riskCommitCount("feature/unmerged", "origin/main"); n != 1 { + t.Fatalf("want the branch's 1 unique commit measured, got %d (the tag was measured)", n) + } + diff, _, err := loadDiff("feature/unmerged") + if err != nil { + t.Fatalf("loadDiff: %v", err) + } + if !strings.Contains(diff, "+++ b/b") { + t.Fatalf("diff must show the branch's content, not the tag's:\n%s", diff) + } + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + // The branch must load under its plain name. %(refname:short) reports the + // shortest *unambiguous* name, which becomes "heads/feature/unmerged" the + // moment the tag exists — breaking every name-keyed lookup downstream. + b := find(m.branches, "feature/unmerged") + if b == nil { + t.Fatalf("branch must load under its plain name; loaded %v", branchNames(m.branches)) + } + + // End to end: the confirm screen must still state what the delete costs. + b.selected = true + nm, _ := m.updateList(key("d")) + m = nm.(model) + out := stripANSI(m.confirmView()) + if !strings.Contains(out, fmt.Sprintf("1 commit(s) not in %s", m.riskBase)) { + t.Fatalf("confirm view must state the shadowed branch's cost:\n%s", out) + } + + // And the delete must land on the branch, leaving the tag alone. + m.force = true + m.performDeletions() + if !m.results[0].localOK { + t.Fatalf("delete failed: %s", m.results[0].localErr) + } + if find(m.branches, "feature/unmerged") != nil { + t.Fatal("the branch should be gone") + } + if _, err := runGit("rev-parse", "--verify", "--quiet", "refs/tags/feature/unmerged"); err != nil { + t.Fatal("the tag must survive a branch delete") + } + }) + + // A tag named main/master, with no branch of that name, must not be adopted as + // the comparison base — `rev-parse --verify main` is satisfied by the tag. This + // was the same bug on the no-remote path: measured against the tag, a branch + // holding real work reported nothing at risk, so a -D drew no warning at all. + t.Run("local_default_name", func(t *testing.T) { + repo := initRepo(t, "trunk") + chdir(t, repo) + commitFile(t, repo, "a", "a") + git(t, repo, "checkout", "-q", "-b", "feature", "trunk") + commitFile(t, repo, "b", "b") + // The tag carries the feature tip, so measuring against it reports nothing. + git(t, repo, "tag", "main", "feature") + git(t, repo, "checkout", "-q", "trunk") + + if got := localDefaultBranch(""); got != "" { + t.Fatalf("a tag must not pose as the local default branch, got %q", got) + } + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + if m.riskBase != "" { + t.Fatalf("no honest base exists here, got %q", m.riskBase) + } + b := find(m.branches, "feature") + b.selected = true + m.force = true + m.measureSelectedRisk() + if w := m.riskWarning(*b); !strings.Contains(w, "no base branch to compare against") { + t.Fatalf("a -D with no measurable base must say so rather than stay silent: %q", w) + } + }) + + // A remote carrying both a branch and a tag of one name rejects a bare refspec + // as matching more than one ref, deleting nothing. + t.Run("remote_delete_refspec", func(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + git(t, repo, "tag", "feature/tracked", "main") + git(t, repo, "push", "-q", "origin", "refs/tags/feature/tracked") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + tb := find(m.branches, "feature/tracked") + tb.selected = true + tb.deleteRemote = true + m.force = true + m.performDeletions() + + r := m.results[0] + if !r.localOK { + t.Fatalf("local delete failed: %s", r.localErr) + } + if !r.remoteTried || !r.remoteOK { + t.Fatalf("the remote branch delete must succeed: tried=%v err=%s", r.remoteTried, r.remoteErr) + } + if remoteHasBranch(t, repo, "feature/tracked") { + t.Fatal("the remote branch should be gone") + } + // Only the branch was asked for; the tag must survive. + out, err := runGit("ls-remote", "--tags", "origin", "feature/tracked") + if err != nil || strings.TrimSpace(out) == "" { + t.Fatalf("the remote tag must survive a branch delete (err=%v out=%q)", err, out) + } + }) + + // The upstream is read as a short name too, so a tag named after a + // remote-tracking ref disambiguates it to "remotes/origin/…" — which would + // then be split into a bogus remote named "remotes". + t.Run("upstream_name", func(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + git(t, repo, "tag", "origin/feature/tracked", "main") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + b := find(m.branches, "feature/tracked") + if b == nil { + t.Fatalf("feature/tracked missing; loaded %v", branchNames(m.branches)) + } + if b.upstream != "origin/feature/tracked" { + t.Fatalf("upstream must load unqualified, got %q", b.upstream) + } + if b.remoteName() != "origin" || b.remoteBranch() != "feature/tracked" { + t.Fatalf("a shadowed upstream must still split correctly: %s / %s", + b.remoteName(), b.remoteBranch()) + } + }) + + // The base is resolved to a short name too, so a tag can shadow it the same way + // — and a wrong base silently changes every branch's measured risk. + t.Run("base_name", func(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + // A tag named after the remote-tracking base, pointing at the work that is + // supposed to be measured as missing from it. + git(t, repo, "tag", "origin/main", "feature/unmerged") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + if m.riskBase != "origin/main" { + t.Fatalf("precondition: base should be origin/main, got %q", m.riskBase) + } + if n := riskCommitCount("feature/unmerged", m.riskBaseRef); n != 1 { + t.Fatalf("want 1 commit at risk, got %d (the tag was used as the base)", n) + } + }) +} + +// A fetch mutates nothing local, so pressing `p` must not silently discard the +// selections the user already made — they are the whole reason to press d next. +func TestFetchPreservesSelections(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + find(m.branches, "feature/unmerged").selected = true + tb := find(m.branches, "feature/tracked") + tb.selected = true + tb.deleteRemote = true + + nm, _ := m.Update(fetchPruneCmd()()) + after := nm.(model) + + if b := find(after.branches, "feature/unmerged"); b == nil || !b.selected { + t.Fatalf("a manual selection must survive a fetch: %+v", b) + } + if b := find(after.branches, "feature/tracked"); b == nil || !b.selected || !b.deleteRemote { + t.Fatalf("an armed remote delete must survive a fetch: %+v", b) + } + if b := find(after.branches, "feature/merged"); b == nil || b.selected { + t.Fatalf("an untouched branch must stay unselected: %+v", b) + } +} + +// The armed remote delete is dropped when the fetch reveals the upstream is +// already gone: the push it would run can only fail, and the results screen would +// report that failure as if the user had asked for something impossible. +func TestFetchDisarmsRemoteForGoneBranch(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + tb := find(m.branches, "feature/tracked") + tb.selected = true + tb.deleteRemote = true + + // Someone else deletes the remote branch before this fetch lands. + git(t, repo, "push", "-q", "origin", "--delete", "feature/tracked") + + nm, _ := m.Update(fetchPruneCmd()()) + after := nm.(model) + + b := find(after.branches, "feature/tracked") + if b == nil || !b.gone { + t.Fatalf("precondition: feature/tracked should be gone after the fetch: %+v", b) + } + if !b.selected { + t.Fatalf("the selection itself must survive: %+v", b) + } + if b.deleteRemote { + t.Fatalf("the armed remote must be disarmed once the upstream is gone: %+v", b) + } +} + +// Everyday shape: the PR merged, the remote branch was deleted, and the user is +// still standing on that branch when they press `p`. The current branch cannot be +// deleted, so it must never be auto-selected — and must survive if it somehow is. +func TestGoneCurrentBranchIsNotPruned(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + git(t, repo, "checkout", "-q", "feature/tracked") + git(t, repo, "push", "-q", "origin", "--delete", "feature/tracked") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + nm, _ := m.Update(fetchPruneCmd()()) + m = nm.(model) + + b := find(m.branches, "feature/tracked") + if b == nil || !b.gone || !b.isCurrent { + t.Fatalf("precondition: want the gone current branch: %+v", b) + } + if b.selected { + t.Fatal("the checked-out branch must never be auto-selected for deletion") + } + // It cannot be pruned, so the status must not count it as one waiting to be. + if strings.Contains(m.status, "press d to prune") { + t.Fatalf("status must not offer to prune the current branch: %q", m.status) + } + + // Selected by hand, git refuses — a failure -D cannot rescue, so it must not + // raise the force prompt offering a retry that fails identically. + b.selected = true + m.performDeletions() + if len(m.results) != 1 || m.results[0].localOK { + t.Fatalf("deleting the checked-out branch must fail: %+v", m.results) + } + if m.results[0].forceable { + t.Fatalf("a checked-out branch cannot be rescued by -D: %q", m.results[0].localErr) + } + if find(m.branches, "feature/tracked") == nil { + t.Fatal("feature/tracked must survive the refused delete") + } +} + +// setupTrunkRepo builds a repo whose default branch is neither main nor master, +// with origin/HEAD unset — what `git remote add` + push produces, as opposed to a +// clone, which is the only thing that sets origin/HEAD. +func setupTrunkRepo(t *testing.T) string { + t.Helper() + tmp := initRepo(t, "trunk") + commitFile(t, tmp, "a", "a") + addOrigin(t, tmp, "trunk") + git(t, tmp, "checkout", "-q", "-b", "feature/unmerged") + commitFile(t, tmp, "b", "b") + git(t, tmp, "checkout", "-q", "trunk") + return tmp +} + +// A repo with no remote configured: every remote-derived lookup must degrade to +// the local default branch rather than going blank and silencing the warnings. +func TestLocalOnlyRepo(t *testing.T) { + repo := setupLocalRepo(t) + chdir(t, repo) + + if got := remotes(); len(got) != 0 { + t.Fatalf("no remotes should be configured, got %v", got) + } + if got := remoteDefault(); got != "" { + t.Fatalf("remoteDefault should be empty, got %q", got) + } + + m, err := initialModel() + if err != nil { + t.Fatalf("initialModel must work without a remote: %v", err) + } + if m.riskBase != "main" || m.riskBaseRef != "refs/heads/main" { + t.Fatalf("risk should fall back to the local default: %q / %q", m.riskBase, m.riskBaseRef) + } + if _, base, err := loadDiff("feature/unmerged"); err != nil || base != "main" { + t.Fatalf("diff base should fall back to local main, got %q err=%v", base, err) + } + + // The cost of a delete is still measured and still named. + b := find(m.branches, "feature/unmerged") + b.selected = true + m.measureSelectedRisk() + if b.riskCommits != 1 { + t.Fatalf("want 1 commit at risk against local main, got %d", b.riskCommits) + } + if w := m.riskWarning(*b); !strings.Contains(w, "1 commit(s) not in main") { + t.Fatalf("warning must name the local base: %q", w) + } + + // `p` is a harmless no-op rather than an error. + nm, _ := m.Update(fetchPruneCmd()()) + m = nm.(model) + if m.err != "" { + t.Fatalf("fetch must succeed with no remotes: %q", m.err) + } + if !strings.Contains(m.status, "no gone branches") { + t.Fatalf("status should report nothing to prune, got %q", m.status) + } + + // And an ordinary delete still works. feature/unmerged is still selected from + // above (it survives the fetch), so clear it first. + find(m.branches, "feature/unmerged").selected = false + find(m.branches, "feature/merged").selected = true + m.performDeletions() + if len(m.results) != 1 || !m.results[0].localOK { + t.Fatalf("merged branch should delete cleanly: %+v", m.results) + } +} + +// `git init` with nothing committed: the tool must open on an empty list rather +// than refusing to start or panicking on the missing HEAD. (git itself fails +// `branch --merged HEAD` here, which mergedSet has to absorb.) +func TestUnbornHeadRepo(t *testing.T) { + repo := initRepo(t, "main") + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatalf("initialModel must succeed in a fresh repo: %v", err) + } + if len(m.branches) != 0 { + t.Fatalf("a repo with no commits has no branches, got %v", branchNames(m.branches)) + } + if m.riskBase != "" || m.riskBaseRef != "" { + t.Fatalf("nothing can serve as a base: %q / %q", m.riskBase, m.riskBaseRef) + } + if m.cur() != nil { + t.Fatal("there is no branch under the cursor") + } + if out := stripANSI(m.listView()); !strings.Contains(out, "no local branches found") { + t.Fatalf("the empty list must say so:\n%s", out) + } + + // Every key that acts on the cursor or the selection must be a no-op here. + for _, k := range []string{"j", "k", "g", "G", " ", "r", "a", "n", "v", "d", "s", "o", "f"} { + nm, _ := m.updateList(key(k)) + m = nm.(model) + if m.state != stateList { + t.Fatalf("%q left the list view (state %v) with no branches", k, m.state) + } + } + if len(m.selectedBranches()) != 0 { + t.Fatal("nothing can be selected") + } + + // Including a fetch, which has no remote to talk to. + nm, _ := m.Update(fetchPruneCmd()()) + if got := nm.(model).err; got != "" { + t.Fatalf("fetch in a fresh repo should not error: %q", got) + } +} + +// Mid-bisect, mid-rebase, or on a checked-out tag, HEAD is on no branch at all. +// Nothing is current, so nothing carries the current-branch protection — and git +// will happily delete the branch HEAD is parked on. +func TestDetachedHead(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + git(t, repo, "checkout", "-q", "--detach", "main") + + m, err := initialModel() + if err != nil { + t.Fatalf("initialModel must work on a detached HEAD: %v", err) + } + for _, b := range m.branches { + if b.isCurrent { + t.Fatalf("no branch is current when HEAD is detached: %+v", b) + } + } + // git prints a "(HEAD detached at …)" pseudo-entry in --merged output; it must + // not be mistaken for a branch, and real branches must still be classified. + if len(m.branches) != 4 { + t.Fatalf("want the 4 real branches, got %v", branchNames(m.branches)) + } + if b := find(m.branches, "main"); b == nil || !b.headMerged { + t.Fatalf("main is merged into the detached HEAD: %+v", b) + } + if b := find(m.branches, "feature/unmerged"); b == nil || b.headMerged { + t.Fatalf("feature/unmerged is not merged into the detached HEAD: %+v", b) + } + + // With nothing current, `a` selects everything — there is no branch to spare. + nm, _ := m.updateList(key("a")) + m = nm.(model) + if got := len(m.selectedBranches()); got != len(m.branches) { + t.Fatalf("select-all should take all %d branches, got %d", len(m.branches), got) + } + + // Deleting the branch HEAD is parked on is legal while detached, and safe: + // the commits stay reachable from HEAD. + nm, _ = m.updateList(key("n")) + m = nm.(model) + find(m.branches, "main").selected = true + m.performDeletions() + if len(m.results) != 1 || !m.results[0].localOK { + t.Fatalf("main should delete cleanly while detached: %+v", m.results) + } + if find(m.branches, "main") != nil { + t.Fatal("main should be gone") + } +} + +// A repo whose default branch is neither main nor master. Without origin/HEAD +// there is nothing left to guess from, so no base resolves at all — the path +// every "no base branch to compare against" message hangs off. +func TestNonStandardDefaultBranch(t *testing.T) { + t.Run("unresolvable_base", func(t *testing.T) { + repo := setupTrunkRepo(t) + chdir(t, repo) + + if got := remoteDefault(); got != "" { + t.Fatalf("nothing should resolve without origin/HEAD or main/master, got %q", got) + } + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + if m.riskBase != "" { + t.Fatalf("no base should resolve, got %q", m.riskBase) + } + + b := find(m.branches, "feature/unmerged") + if b == nil { + t.Fatalf("feature/unmerged missing; loaded %v", branchNames(m.branches)) + } + b.selected = true + m.force = true + m.measureSelectedRisk() + if b.riskCommits != 0 { + t.Fatalf("nothing can be measured without a base, got %d", b.riskCommits) + } + + // A -D with nothing to measure against must say so rather than imply safety. + m.state = stateConfirm + if out := stripANSI(m.confirmView()); !strings.Contains(out, "no base branch to compare against") { + t.Fatalf("confirm view must carry the warning:\n%s", out) + } + + // The same holds on the force prompt reached after a refused safe delete. + m.force = false + m.performDeletions() + if len(m.forceableFailures()) != 1 { + t.Fatalf("the unmerged branch should be refused and forceable: %+v", m.results) + } + m.state = stateForcePrompt + if out := stripANSI(m.forcePromptView()); !strings.Contains(out, "no base branch to compare against") { + t.Fatalf("force prompt must carry the warning:\n%s", out) + } + }) + + // A clone sets origin/HEAD, which is the only thing that can name a default + // branch the tool would never guess. This is remoteDefault's symbolic-ref path. + t.Run("origin_head_resolves", func(t *testing.T) { + repo := setupTrunkRepo(t) + chdir(t, repo) + git(t, repo, "remote", "set-head", "origin", "trunk") + + if got := remoteDefault(); got != "refs/remotes/origin/trunk" { + t.Fatalf("origin/HEAD should name the default, got %q", got) + } + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + if m.riskBase != "origin/trunk" || m.riskBaseRef != "refs/remotes/origin/trunk" { + t.Fatalf("model should cache both forms: %q / %q", m.riskBase, m.riskBaseRef) + } + b := find(m.branches, "feature/unmerged") + b.selected = true + m.measureSelectedRisk() + if b.riskCommits != 1 { + t.Fatalf("risk should now be measurable against origin/trunk, got %d", b.riskCommits) + } + }) +} + +// The remote is still configured but unreachable — the server moved, the repo was +// deleted, or the laptop is offline. Remote-tracking refs are local, so everything +// on screen still resolves; only the operations that touch the network fail, and +// they must fail loudly without taking the local delete down with them. +func TestUnreachableRemote(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + git(t, repo, "remote", "set-url", "origin", t.TempDir()+"/gone.git") + + m, err := initialModel() + if err != nil { + t.Fatalf("initialModel must work against a dead remote: %v", err) + } + tb := find(m.branches, "feature/tracked") + if tb == nil || tb.upstream != "origin/feature/tracked" { + t.Fatalf("the upstream is recorded locally and must survive: %+v", tb) + } + + // A fetch is the first thing to fail, and the error must reach the user. + nm, _ := m.Update(fetchPruneCmd()()) + m = nm.(model) + if m.err == "" { + t.Fatal("a failed fetch must be reported") + } + if m.fetching { + t.Fatal("the fetch flag must clear so p can be pressed again") + } + + // The local delete still succeeds; the armed push fails and is reported. + tb = find(m.branches, "feature/tracked") + tb.selected = true + tb.deleteRemote = true + m.performDeletions() + + r := m.results[0] + if !r.localOK { + t.Fatalf("the local delete must still land: %s", r.localErr) + } + if !r.remoteTried || r.remoteOK || r.remoteErr == "" { + t.Fatalf("the push must have been tried, failed, and captured: %+v", r) + } + if r.remoteSkipped { + t.Fatalf("the push was attempted, not deferred: %+v", r) + } + out := stripANSI(m.resultView()) + if !strings.Contains(out, "deleted local feature/tracked") { + t.Fatalf("results must report the successful local delete:\n%s", out) + } + if !strings.Contains(out, "remote feature/tracked:") { + t.Fatalf("results must report the failed push:\n%s", out) + } +} + +// setupDiverged gives feature/tracked commits its upstream lacks and the upstream +// a commit it lacks, so git reports "[ahead 2, behind 1]" — ahead by the "b" it +// already carried from setupRepo plus "mine", behind by "theirs". +func setupDiverged(t *testing.T, repo string) { + t.Helper() + git(t, repo, "checkout", "-q", "feature/tracked") + commitFile(t, repo, "mine", "mine") + // Publish a different commit as the upstream's tip, from a branch that never + // touches the local one. + git(t, repo, "checkout", "-q", "-b", "theirs", "main") + commitFile(t, repo, "theirs", "theirs") + git(t, repo, "push", "-qf", "origin", "theirs:feature/tracked") + git(t, repo, "checkout", "-q", "main") + git(t, repo, "branch", "-qD", "theirs") + git(t, repo, "fetch", "-q", "--all", "--prune") +} + +// Both halves of the track string come from one git field, but only the ahead +// half has ever been exercised against real git output. +func TestDivergedBranch(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + setupDiverged(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + b := find(m.branches, "feature/tracked") + if b == nil || b.ahead != 2 || b.behind != 1 { + t.Fatalf("want ahead=2 behind=1 parsed from real git: %+v", b) + } + if track := stripANSI(m.trackStr(*b)); !strings.Contains(track, "↑2") || !strings.Contains(track, "↓1") { + t.Fatalf("the track column must show both directions, got %q", track) + } + // Holding a commit its upstream lacks, it is not safely deletable. + if b.safeDeletable() { + t.Fatalf("a branch ahead of its upstream must not be safely deletable: %+v", b) + } + + // sortAheadBehind orders on (ahead - behind), which nets to 0 here. + m.field = sortAheadBehind + m.ascending = true + m.sortBranches() + if len(m.branches) != 4 { + t.Fatalf("the sort must keep every branch: %v", branchNames(m.branches)) + } +} + +// Someone else deleted the remote branch between the last fetch and this delete. +// Nothing local says so, so the push is still attempted — and because the refspec +// is qualified, git treats deleting an already-absent ref as a no-op rather than +// an error. The end state is exactly what the user armed, so this reports success. +// (A bare refspec would fail here with "remote ref does not exist", but it also +// silently deletes nothing when a tag shadows the branch — see +// TestTagShadowingBranchName/remote_delete_refspec. Idempotence is the better +// trade.) +func TestRemoteDeleteRace(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + // Delete it straight on the bare remote, leaving our remote-tracking ref stale + // — `push --delete` from here would prune that ref and give the game away. + out, err := runGit("remote", "get-url", "origin") + if err != nil { + t.Fatal(err) + } + git(t, strings.TrimSpace(out), "branch", "-qD", "feature/tracked") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + tb := find(m.branches, "feature/tracked") + if tb == nil || tb.gone || tb.upstream != "origin/feature/tracked" { + t.Fatalf("without a fetch the branch still looks healthy: %+v", tb) + } + tb.selected = true + tb.deleteRemote = true + m.performDeletions() + + r := m.results[0] + if !r.localOK { + t.Fatalf("the local delete must still succeed: %s", r.localErr) + } + if !r.remoteTried || !r.remoteOK { + t.Fatalf("the already-absent remote must resolve cleanly: %+v", r) + } + if remoteHasBranch(t, repo, "feature/tracked") { + t.Fatal("the remote branch must be absent afterwards") + } + if o := stripANSI(m.resultView()); !strings.Contains(o, "deleted remote feature/tracked") { + t.Fatalf("results must report the remote as dealt with:\n%s", o) + } +} + +// The checked-out branch cannot be deleted, so no key may ever mark it. +func TestCurrentBranchCannotBeMarked(t *testing.T) { + repo := setupRepo(t) + chdir(t, repo) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + for i := range m.branches { + if m.branches[i].isCurrent { + m.cursor = i + } + } + if m.cur() == nil || !m.cur().isCurrent { + t.Fatal("precondition: the cursor should sit on the current branch") + } + for _, k := range []string{" ", "r"} { + nm, _ := m.updateList(key(k)) + m = nm.(model) + if b := m.cur(); b.selected || b.deleteRemote { + t.Fatalf("%q marked the current branch: %+v", k, b) + } + } + + // Select-all spares it too. + nm, _ := m.updateList(key("a")) + m = nm.(model) + if got, want := len(m.selectedBranches()), len(m.branches)-1; got != want { + t.Fatalf("select-all should take %d of %d branches, got %d", want, len(m.branches), got) + } + for _, b := range m.selectedBranches() { + if b.isCurrent { + t.Fatalf("select-all must spare the current branch: %+v", b) + } + } +} + +// With no remote default and no local main/master, there is nothing left to diff +// against but HEAD. +func TestLoadDiffFallsBackToHead(t *testing.T) { + repo := setupTrunkRepo(t) // default branch "trunk", origin/HEAD unset + chdir(t, repo) + + if got := baseBranch("feature/unmerged"); got != "" { + t.Fatalf("no base should resolve, got %q", got) + } + diff, base, err := loadDiff("feature/unmerged") + if err != nil { + t.Fatalf("loadDiff: %v", err) + } + if base != "HEAD" { + t.Fatalf("the base should fall back to HEAD, got %q", base) + } + if !strings.Contains(diff, "+++ b/b") { + t.Fatalf("the diff against HEAD must still show the branch's work:\n%s", diff) + } + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + m.diffBranch, m.diffBase = "feature/unmerged", base + m.diffLines = strings.Split(strings.TrimRight(diff, "\n"), "\n") + m.state = stateDiff + if o := stripANSI(m.diffView()); !strings.Contains(o, "vs HEAD") { + t.Fatalf("the diff header must name the base it actually used:\n%s", o) + } +} + +// Legal but unusual content has to survive the NUL-delimited parse: an empty +// commit subject leaves a trailing empty field, which a stricter field count +// would drop the whole branch over, and a multibyte name must reach the renderer +// and the git call intact. +func TestUnicodeNameAndEmptySubject(t *testing.T) { + repo := setupLocalRepo(t) + chdir(t, repo) + + const name = "feature/café-日本語" + git(t, repo, "checkout", "-q", "-b", name) + if err := os.WriteFile(repo+"/c", []byte("c"), 0o644); err != nil { + t.Fatal(err) + } + git(t, repo, "add", "c") + git(t, repo, "commit", "-q", "--allow-empty-message", "-m", "") + git(t, repo, "checkout", "-q", "main") + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + b := find(m.branches, name) + if b == nil { + t.Fatalf("the branch must survive the parse; loaded %v", branchNames(m.branches)) + } + if b.subject != "" { + t.Fatalf("want an empty subject, got %q", b.subject) + } + if !strings.Contains(stripANSI(m.listView()), "café") { + t.Fatalf("the row must carry the branch name:\n%s", stripANSI(m.listView())) + } + + // And it deletes end to end, the name surviving the round trip to git. + b.selected = true + m.force = true + m.performDeletions() + if !m.results[0].localOK { + t.Fatalf("delete failed: %s", m.results[0].localErr) + } + if find(m.branches, name) != nil { + t.Fatal("the branch should be gone") + } +} + // The version string is well-formed even when VCS build info is absent. func TestVersionString(t *testing.T) { s := versionString()