From 33e81f717d02295e8ae10a9a1fce047436a8da07 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 4 Jan 2025 15:27:03 +0100 Subject: [PATCH 1/3] Extend reset/rebase test to use upstream branch name that is different from local one The easiest way to do that is to rename the local branch after pushing. This shows various levels of brokenness for the reset and rebase to upstream commands: both menu entries display the wrong upstream branch name in the menu (the local one rather than the remote one); executing the rebase command works correctly though, the rebase command uses the right branch name. Resetting fails, though. We'll fix this in the next commit. --- pkg/integration/components/shell.go | 4 ++++ .../tests/branch/rebase_to_upstream.go | 12 +++++++---- .../tests/branch/reset_to_upstream.go | 21 ++++++++++++++++--- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index 01a9caf3a..4fb2d5f52 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -142,6 +142,10 @@ func (self *Shell) NewBranchFrom(name string, from string) *Shell { return self.RunCommand([]string{"git", "checkout", "-b", name, from}) } +func (self *Shell) RenameCurrentBranch(newName string) *Shell { + return self.RunCommand([]string{"git", "branch", "-m", newName}) +} + func (self *Shell) Checkout(name string) *Shell { return self.RunCommand([]string{"git", "checkout", name}) } diff --git a/pkg/integration/tests/branch/rebase_to_upstream.go b/pkg/integration/tests/branch/rebase_to_upstream.go index f8b2d6fd1..a47da2b1a 100644 --- a/pkg/integration/tests/branch/rebase_to_upstream.go +++ b/pkg/integration/tests/branch/rebase_to_upstream.go @@ -16,8 +16,9 @@ var RebaseToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ EmptyCommit("ensure-master"). EmptyCommit("to-be-added"). // <- this will only exist remotely PushBranchAndSetUpstream("origin", "master"). + RenameCurrentBranch("master-local"). HardReset("HEAD~1"). - NewBranchFrom("base-branch", "master"). + NewBranchFrom("base-branch", "master-local"). EmptyCommit("base-branch-commit"). NewBranch("target"). EmptyCommit("target-commit") @@ -34,13 +35,13 @@ var RebaseToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Lines( Contains("target").IsSelected(), Contains("base-branch"), - Contains("master"), + Contains("master-local"), ). SelectNextItem(). Lines( Contains("target"), Contains("base-branch").IsSelected(), - Contains("master"), + Contains("master-local"), ). Press(keys.Branches.SetUpstream). Tap(func() { @@ -58,13 +59,16 @@ var RebaseToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Lines( Contains("target"), Contains("base-branch"), - Contains("master").IsSelected(), + Contains("master-local").IsSelected(), ). Press(keys.Branches.SetUpstream). Tap(func() { t.ExpectPopup().Menu(). Title(Equals("Upstream options")). + /* EXPECTED: Select(Contains("Rebase checked-out branch onto origin/master...")). + ACTUAL: */ + Select(Contains("Rebase checked-out branch onto origin/master-local...")). Confirm() t.ExpectPopup().Menu(). Title(Equals("Rebase 'target'")). diff --git a/pkg/integration/tests/branch/reset_to_upstream.go b/pkg/integration/tests/branch/reset_to_upstream.go index 3cdbb561d..75dcfd3bd 100644 --- a/pkg/integration/tests/branch/reset_to_upstream.go +++ b/pkg/integration/tests/branch/reset_to_upstream.go @@ -19,6 +19,7 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ NewBranch("soft-branch"). EmptyCommit("soft commit"). PushBranchAndSetUpstream("origin", "soft-branch"). + RenameCurrentBranch("soft-branch-local"). NewBranch("base"). EmptyCommit("base-branch commit"). CreateFile("file-1", "content"). @@ -33,7 +34,7 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Focus(). Lines( Contains("base").IsSelected(), - Contains("soft-branch"), + Contains("soft-branch-local"), Contains("hard-branch"), ). Press(keys.Branches.SetUpstream). @@ -51,21 +52,34 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ SelectNextItem(). Lines( Contains("base"), - Contains("soft-branch").IsSelected(), + Contains("soft-branch-local").IsSelected(), Contains("hard-branch"), ). Press(keys.Branches.SetUpstream). Tap(func() { t.ExpectPopup().Menu(). Title(Equals("Upstream options")). + /* EXPECTED: Select(Contains("Reset checked-out branch onto origin/soft-branch...")). + ACTUAL: */ + Select(Contains("Reset checked-out branch onto origin/soft-branch-local...")). Confirm() t.ExpectPopup().Menu(). + /* EXPECTED: Title(Equals("Reset to origin/soft-branch")). + ACTUAL: */ + Title(Equals("Reset to origin/soft-branch-local")). Select(Contains("Soft reset")). Confirm() + + // Bug: the command fails + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("fatal: ambiguous argument 'origin/soft-branch-local': unknown revision or path not in the working tree.")). + Confirm() }) + /* Since the command failed, the following assertions are not valid t.Views().Commits().Lines( Contains("soft commit"), Contains("hard commit"), @@ -74,13 +88,14 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Contains("file-1").Contains("A"), Contains("file-2").Contains("A"), ) + */ // hard reset t.Views().Branches(). Focus(). Lines( Contains("base"), - Contains("soft-branch").IsSelected(), + Contains("soft-branch-local").IsSelected(), Contains("hard-branch"), ). NavigateToLine(Contains("hard-branch")). From 009062534e4974ba174d86830db26c93c777839c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 4 Jan 2025 15:18:22 +0100 Subject: [PATCH 2/3] Fix resetting or rebasing a branch to its upstream when the upstream branch name is different --- pkg/gui/controllers/branches_controller.go | 2 +- pkg/integration/tests/branch/rebase_to_upstream.go | 3 --- pkg/integration/tests/branch/reset_to_upstream.go | 14 -------------- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 9a1530971..5a0a87fc6 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -294,7 +294,7 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc } upstream := lo.Ternary(selectedBranch.RemoteBranchStoredLocally(), - fmt.Sprintf("%s/%s", selectedBranch.UpstreamRemote, selectedBranch.Name), + selectedBranch.ShortUpstreamRefName(), self.c.Tr.UpstreamGenericName) upstreamResetOptions := utils.ResolvePlaceholderString( self.c.Tr.ViewUpstreamResetOptions, diff --git a/pkg/integration/tests/branch/rebase_to_upstream.go b/pkg/integration/tests/branch/rebase_to_upstream.go index a47da2b1a..397a79d1e 100644 --- a/pkg/integration/tests/branch/rebase_to_upstream.go +++ b/pkg/integration/tests/branch/rebase_to_upstream.go @@ -65,10 +65,7 @@ var RebaseToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Tap(func() { t.ExpectPopup().Menu(). Title(Equals("Upstream options")). - /* EXPECTED: Select(Contains("Rebase checked-out branch onto origin/master...")). - ACTUAL: */ - Select(Contains("Rebase checked-out branch onto origin/master-local...")). Confirm() t.ExpectPopup().Menu(). Title(Equals("Rebase 'target'")). diff --git a/pkg/integration/tests/branch/reset_to_upstream.go b/pkg/integration/tests/branch/reset_to_upstream.go index 75dcfd3bd..c2ca41f4a 100644 --- a/pkg/integration/tests/branch/reset_to_upstream.go +++ b/pkg/integration/tests/branch/reset_to_upstream.go @@ -59,27 +59,14 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Tap(func() { t.ExpectPopup().Menu(). Title(Equals("Upstream options")). - /* EXPECTED: Select(Contains("Reset checked-out branch onto origin/soft-branch...")). - ACTUAL: */ - Select(Contains("Reset checked-out branch onto origin/soft-branch-local...")). Confirm() t.ExpectPopup().Menu(). - /* EXPECTED: Title(Equals("Reset to origin/soft-branch")). - ACTUAL: */ - Title(Equals("Reset to origin/soft-branch-local")). Select(Contains("Soft reset")). Confirm() - - // Bug: the command fails - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("fatal: ambiguous argument 'origin/soft-branch-local': unknown revision or path not in the working tree.")). - Confirm() }) - /* Since the command failed, the following assertions are not valid t.Views().Commits().Lines( Contains("soft commit"), Contains("hard commit"), @@ -88,7 +75,6 @@ var ResetToUpstream = NewIntegrationTest(NewIntegrationTestArgs{ Contains("file-1").Contains("A"), Contains("file-2").Contains("A"), ) - */ // hard reset t.Views().Branches(). From 53b1e1211074ceaa3098b607e2dedd8fabad37ff Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 4 Jan 2025 15:19:33 +0100 Subject: [PATCH 3/3] Cleanup: use the upstream local variable consistently We need to move it closer to the beginning of the method to use it everywhere. --- pkg/gui/controllers/branches_controller.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 5a0a87fc6..244092681 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -194,6 +194,10 @@ func (self *BranchesController) GetOnRenderToMain() func() { } func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branch) error { + upstream := lo.Ternary(selectedBranch.RemoteBranchStoredLocally(), + selectedBranch.ShortUpstreamRefName(), + self.c.Tr.UpstreamGenericName) + viewDivergenceItem := &types.MenuItem{ LabelColumns: []string{self.c.Tr.ViewDivergenceFromUpstream}, OnPress: func() error { @@ -204,7 +208,7 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc return self.c.Helpers().SubCommits.ViewSubCommits(helpers.ViewSubCommitsOpts{ Ref: branch, - TitleRef: fmt.Sprintf("%s <-> %s", branch.RefName(), branch.ShortUpstreamRefName()), + TitleRef: fmt.Sprintf("%s <-> %s", branch.RefName(), upstream), RefToShowDivergenceFrom: branch.FullUpstreamRefName(), Context: self.context(), ShowBranchHeads: false, @@ -293,9 +297,6 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc Key: 's', } - upstream := lo.Ternary(selectedBranch.RemoteBranchStoredLocally(), - selectedBranch.ShortUpstreamRefName(), - self.c.Tr.UpstreamGenericName) upstreamResetOptions := utils.ResolvePlaceholderString( self.c.Tr.ViewUpstreamResetOptions, map[string]string{"upstream": upstream}, @@ -332,7 +333,7 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc LabelColumns: []string{upstreamRebaseOptions}, OpensMenu: true, OnPress: func() error { - if err := self.c.Helpers().MergeAndRebase.RebaseOntoRef(selectedBranch.ShortUpstreamRefName()); err != nil { + if err := self.c.Helpers().MergeAndRebase.RebaseOntoRef(upstream); err != nil { return err } return nil