diff --git a/README.md b/README.md index 77ed874..edf5ace 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,7 @@ gh stack create Start tracking an existing branch by setting its parent. -By default, adopts the current branch. The parent must be either the trunk or another tracked branch. +By default, adopts the current branch. The parent must be either the trunk or another tracked branch. If the branch is already tracked, `adopt` updates its parent to the specified branch (or does nothing if the parent is unchanged). #### adopt Usage diff --git a/cmd/adopt.go b/cmd/adopt.go index 2bfa924..cde1451 100644 --- a/cmd/adopt.go +++ b/cmd/adopt.go @@ -62,9 +62,15 @@ func runAdopt(cmd *cobra.Command, args []string) error { return fmt.Errorf("branch %q does not exist", branchName) } - // Check if already tracked - if _, getParentErr := cfg.GetParent(branchName); getParentErr == nil { - return fmt.Errorf("branch %q is already tracked", branchName) + // A branch cannot be its own parent + if parent == branchName { + return fmt.Errorf("cannot adopt: branch %q cannot be its own parent", branchName) + } + + // Check if already tracked, capture old parent if so + oldParent, alreadyTracked := "", false + if p, getParentErr := cfg.GetParent(branchName); getParentErr == nil { + oldParent, alreadyTracked = p, true } // Validate parent is trunk or tracked @@ -94,6 +100,14 @@ func runAdopt(cmd *cobra.Command, args []string) error { } } + s := style.New() + + // No-op: already tracked with the same parent + if alreadyTracked && oldParent == parent { + fmt.Printf("%s Branch %s is already tracked with parent %s\n", s.WarningIcon(), s.Branch(branchName), s.Branch(parent)) + return nil + } + // Set parent if err := cfg.SetParent(branchName, parent); err != nil { return err @@ -105,7 +119,10 @@ func runAdopt(cmd *cobra.Command, args []string) error { _ = cfg.SetForkPoint(branchName, forkPoint) //nolint:errcheck // best effort } - s := style.New() - fmt.Printf("%s Adopted branch %s with parent %s\n", s.SuccessIcon(), s.Branch(branchName), s.Branch(parent)) + if alreadyTracked { + fmt.Printf("%s Updated branch %s parent from %s to %s\n", s.SuccessIcon(), s.Branch(branchName), s.Branch(oldParent), s.Branch(parent)) + } else { + fmt.Printf("%s Adopted branch %s with parent %s\n", s.SuccessIcon(), s.Branch(branchName), s.Branch(parent)) + } return nil } diff --git a/cmd/adopt_test.go b/cmd/adopt_test.go index 9f71a80..4d0df98 100644 --- a/cmd/adopt_test.go +++ b/cmd/adopt_test.go @@ -37,7 +37,7 @@ func TestAdoptBranch(t *testing.T) { } } -func TestAdoptRejectsAlreadyTracked(t *testing.T) { +func TestAdoptReparentsAlreadyTracked(t *testing.T) { dir := setupTestRepo(t) cfg, _ := config.Load(dir) @@ -46,14 +46,23 @@ func TestAdoptRejectsAlreadyTracked(t *testing.T) { trunk, _ := g.CurrentBranch() cfg.SetTrunk(trunk) - // Create and track a branch - g.CreateBranch("tracked-feature") - cfg.SetParent("tracked-feature", trunk) + // Create two branches; track feature-a under trunk + g.CreateBranch("feature-a") + cfg.SetParent("feature-a", trunk) + g.CreateBranch("feature-b") + cfg.SetParent("feature-b", trunk) + + // Simulate what runAdopt does when reparenting feature-b under feature-a + if err := cfg.SetParent("feature-b", "feature-a"); err != nil { + t.Fatalf("SetParent failed: %v", err) + } - // Trying to get parent should succeed (it's tracked) - _, err := cfg.GetParent("tracked-feature") + parent, err := cfg.GetParent("feature-b") if err != nil { - t.Error("expected branch to be tracked") + t.Fatalf("GetParent failed: %v", err) + } + if parent != "feature-a" { + t.Errorf("expected parent %q after reparent, got %q", "feature-a", parent) } } diff --git a/e2e/adopt_orphan_test.go b/e2e/adopt_orphan_test.go index 83e8ff9..8d86f0f 100644 --- a/e2e/adopt_orphan_test.go +++ b/e2e/adopt_orphan_test.go @@ -3,6 +3,84 @@ package e2e_test import "testing" +func TestAdoptReparentsTrackedBranch(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + // Create feat-a and feat-b both off trunk (main) + env.MustRun("create", "feat-a") + env.CreateCommit("feat-a work") + + env.Git("checkout", "main") + env.MustRun("create", "feat-b") + env.CreateCommit("feat-b work") + + // feat-b is currently tracked with parent main; reparent onto feat-a + result := env.MustRun("adopt", "--branch", "feat-b", "feat-a") + + env.AssertStackParent("feat-b", "feat-a") + if !result.ContainsStdout("feat-a") { + t.Errorf("expected stdout to mention feat-a, got: %s", result.Stdout) + } + if !result.ContainsStdout("main") { + t.Errorf("expected stdout to mention old parent main, got: %s", result.Stdout) + } +} + +func TestAdoptNoOpWhenParentUnchangedE2E(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.Git("checkout", "-b", "existing-branch") + env.CreateCommit("some work") + env.MustRun("adopt", "main") + + // Adopt again with same parent: should succeed and print a warning + result := env.Run("adopt", "main") + if !result.Success() { + t.Errorf("expected no-op adopt to succeed, got exit %d: %s", result.ExitCode, result.Stderr) + } + if !result.ContainsStdout("already tracked") { + t.Errorf("expected 'already tracked' in stdout, got: %s", result.Stdout) + } +} + +func TestAdoptSelfParentRejected(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + + result := env.Run("adopt", "--branch", "feat-a", "feat-a") + if result.Success() { + t.Error("expected self-parent adopt to fail, but it succeeded") + } + if !result.ContainsStderr("own parent") { + t.Errorf("expected 'own parent' in stderr, got: %s", result.Stderr) + } +} + +func TestAdoptReparentCycleDetected(t *testing.T) { + env := NewTestEnv(t) + env.MustRun("init") + + // Build trunk → feat-a → feat-b + env.MustRun("create", "feat-a") + env.CreateCommit("a work") + env.MustRun("create", "feat-b") + env.CreateCommit("b work") + + // Attempting to reparent feat-a under feat-b would create a cycle + result := env.Run("adopt", "--branch", "feat-a", "feat-b") + if result.Success() { + t.Error("expected cycle detection to fail, but adopt succeeded") + } + if !result.ContainsStderr("cycle") { + t.Errorf("expected 'cycle' in stderr, got: %s", result.Stderr) + } +} + func TestAdoptExistingBranch(t *testing.T) { env := NewTestEnv(t) env.MustRun("init")