From a0e51da64347526e6e5ea33adfb56a3803d0131d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 21 May 2026 20:27:27 +0200 Subject: [PATCH] Add test demonstrating wrong base branch on ambiguous merge-base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetBaseBranch was determining its candidates purely by "this main branch contains the merge-base" — and then returning the first one without further discrimination. That equivalence class is too loose: multiple main branches can contain the merge-base even when one is clearly closer to the feature branch than another. The new test exercises that case: with main and develop both containing the branch's merge-base, the current code returns whichever git for-each-ref happens to list first, ignoring how close each candidate actually is. --- .../git_commands/branch_loader_test.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/pkg/commands/git_commands/branch_loader_test.go b/pkg/commands/git_commands/branch_loader_test.go index f20ce6186..339bfe59f 100644 --- a/pkg/commands/git_commands/branch_loader_test.go +++ b/pkg/commands/git_commands/branch_loader_test.go @@ -491,3 +491,48 @@ func TestGetBehindBaseBranchValuesForAllBranches_LegacyPath(t *testing.T) { runner.CheckForMissingCalls() } + +// When the merge-base is contained in more than one configured main branch, +// git for-each-ref returns those refs sorted alphabetically by refname, +// regardless of the order we pass them in. The chosen base should respect +// the user's configured order ("main" first), not the alphabetical accident. +// +// Demonstrates the bug; the expected behavior is asserted in the next commit. +func TestGetBaseBranch_AmbiguousPicksAlphabeticalNotConfigOrder(t *testing.T) { + mainBranchRefs := []string{"refs/heads/main", "refs/heads/develop"} + branch := &models.Branch{Name: "feat-x"} + + runner := oscommands.NewFakeRunner(t). + ExpectGitArgs( + []string{"merge-base", "refs/heads/feat-x", "refs/heads/main", "refs/heads/develop"}, + "abc123\n", nil). + ExpectGitArgs( + []string{ + "for-each-ref", "--contains", "abc123", "--format=%(refname)", + "refs/heads/main", "refs/heads/develop", + }, + "refs/heads/develop\nrefs/heads/main\n", nil) + + gitCommon := buildGitCommon(commonDeps{runner: runner}) + + loader := &BranchLoader{ + Common: gitCommon.Common, + GitCommon: gitCommon, + cmd: gitCommon.cmd, + } + + mainBranches := &MainBranches{ + c: gitCommon.Common, + cmd: gitCommon.cmd, + existingMainBranches: mainBranchRefs, + previousMainBranches: gitCommon.Common.UserConfig().Git.MainBranches, + } + + baseBranch, err := loader.GetBaseBranch(branch, mainBranches) + assert.NoError(t, err) + // Want: "refs/heads/main" (first in config order among the tied candidates). + // Have: "refs/heads/develop" (alphabetical first from for-each-ref). + assert.Equal(t, "refs/heads/develop", baseBranch) + + runner.CheckForMissingCalls() +}