From 5c41d5f2b6ab84c2f0f7a3dc888acddfd914b162 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Mon, 22 Sep 2025 22:13:23 +1000 Subject: [PATCH 01/11] Improve squash merge handling --- .../Commands/Helpers/StackActionsTests.cs | 166 +++++++++--------- .../Helpers/TestGitRepositoryBuilder.cs | 48 +++++ .../Integration/StackActionsTests.cs | 53 ++++++ src/Stack/Commands/Helpers/StackActions.cs | 55 +++++- src/Stack/Git/GitClient.cs | 36 ++++ 5 files changed, 268 insertions(+), 90 deletions(-) diff --git a/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs b/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs index faedcead..7ad5636e 100644 --- a/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs +++ b/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs @@ -162,89 +162,89 @@ public async Task UpdateStack_UsingRebase_WhenConflictResolved_CompletesSuccessf gitClient.Received().ChangeBranch(feature); } - [Fact] - public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_RebasesOntoTheParentBranchToAvoidConflicts() - { - // Arrange - var sourceBranch = Some.BranchName(); - var branch1 = Some.BranchName(); - var branch2 = Some.BranchName(); - var gitClient = Substitute.For(); - var gitHubClient = Substitute.For(); - var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); - - // Setup branch statuses to simulate the scenario - gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - { - { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", false, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, // remote branch deleted - { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - }); - gitClient.IsAncestor(branch2, branch1).Returns(true); - - var stack = new Config.Stack( - "Stack1", - Some.HttpsUri().ToString(), - sourceBranch, - new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } - ); - - var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - var factory = Substitute.For(); - factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - // Act - await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); - - // Assert - gitClient.Received().ChangeBranch(branch2); - gitClient.Received().RebaseOntoNewParent(sourceBranch, branch1); - } - - [Fact] - public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_ButTheTargetBranchHasAlreadyHadAdditionalCommitsMergedInto_DoesNotRebaseOntoTheParentBranch() - { - // Arrange - var sourceBranch = Some.BranchName(); - var branch1 = Some.BranchName(); - var branch2 = Some.BranchName(); - var changedFilePath = Some.Name(); - - var gitClient = Substitute.For(); - var gitHubClient = Substitute.For(); - var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); - - // Setup branch statuses to simulate the scenario - gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - { - { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", false, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, // remote branch deleted - { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - }); - gitClient.IsAncestor(branch2, branch1).Returns(false); - - var stack = new Config.Stack( - "Stack1", - Some.HttpsUri().ToString(), - sourceBranch, - new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } - ); - - var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - var factory = Substitute.For(); - factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - // Act - await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); - - // Assert - gitClient.Received().ChangeBranch(branch2); - gitClient.Received().RebaseFromLocalSourceBranch(sourceBranch); - } + // [Fact] + // public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_RebasesOntoTheParentBranchToAvoidConflicts() + // { + // // Arrange + // var sourceBranch = Some.BranchName(); + // var branch1 = Some.BranchName(); + // var branch2 = Some.BranchName(); + // var gitClient = Substitute.For(); + // var gitHubClient = Substitute.For(); + // var logger = XUnitLogger.CreateLogger(testOutputHelper); + // var console = new TestDisplayProvider(testOutputHelper); + + // // Setup branch statuses to simulate the scenario + // gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary + // { + // { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, + // { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", false, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, // remote branch deleted + // { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } + // }); + // gitClient.IsAncestor(branch2, branch1).Returns(true); + + // var stack = new Config.Stack( + // "Stack1", + // Some.HttpsUri().ToString(), + // sourceBranch, + // new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } + // ); + + // var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; + // var factory = Substitute.For(); + // factory.Create(executionContext.WorkingDirectory).Returns(gitClient); + // var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + + // // Act + // await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); + + // // Assert + // gitClient.Received().ChangeBranch(branch2); + // gitClient.Received().RebaseOntoNewParent(sourceBranch, branch1); + // } + + // [Fact] + // public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_ButTheTargetBranchHasAlreadyHadAdditionalCommitsMergedInto_DoesNotRebaseOntoTheParentBranch() + // { + // // Arrange + // var sourceBranch = Some.BranchName(); + // var branch1 = Some.BranchName(); + // var branch2 = Some.BranchName(); + // var changedFilePath = Some.Name(); + + // var gitClient = Substitute.For(); + // var gitHubClient = Substitute.For(); + // var logger = XUnitLogger.CreateLogger(testOutputHelper); + // var console = new TestDisplayProvider(testOutputHelper); + + // // Setup branch statuses to simulate the scenario + // gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary + // { + // { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, + // { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", false, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, // remote branch deleted + // { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } + // }); + // gitClient.IsAncestor(branch2, branch1).Returns(false); + + // var stack = new Config.Stack( + // "Stack1", + // Some.HttpsUri().ToString(), + // sourceBranch, + // new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } + // ); + + // var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; + // var factory = Substitute.For(); + // factory.Create(executionContext.WorkingDirectory).Returns(gitClient); + // var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + + // // Act + // await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); + + // // Assert + // gitClient.Received().ChangeBranch(branch2); + // gitClient.Received().RebaseFromLocalSourceBranch(sourceBranch); + // } [Fact] public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_AndLocalBranchIsDeleted_DoesNotRebaseOntoTheParentBranch() diff --git a/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs b/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs index 9a48e73c..538f1bc9 100644 --- a/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs +++ b/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs @@ -445,6 +445,54 @@ public void CreateCommitOnRemoteTrackingBranch(string branchName, string message LocalRepository.Refs.UpdateTarget(remoteBranch.Reference, commit.Id); } + /// + /// Creates a "squash merge" style commit on the target branch whose tree matches the tip of the branch being squashed. + /// This simulates a PR being squash merged into the target branch (e.g. source) without preserving the original commits. + /// The newly created commit is added to both the local and remote tracking refs of the target branch (if a remote exists). + /// + /// The feature/stack branch whose cumulative changes will be squashed. + /// The branch to receive the squash commit (typically the source branch). + /// The commit message for the squash commit. + /// The created squash commit. + public LibGit2Sharp.Commit CreateSquashCommitFromBranchOntoBranch(string branchToSquashName, string targetBranchName, string message) + { + var branchToSquash = LocalRepository.Branches[branchToSquashName]; + var targetBranch = LocalRepository.Branches[targetBranchName]; + + if (branchToSquash is null) + { + throw new ArgumentException($"Branch '{branchToSquashName}' does not exist", nameof(branchToSquashName)); + } + + if (targetBranch is null) + { + throw new ArgumentException($"Target branch '{targetBranchName}' does not exist", nameof(targetBranchName)); + } + + var signature = new Signature(Some.Name(), Some.Email(), DateTimeOffset.Now); + var parent = targetBranch.Tip; + var tree = branchToSquash.Tip.Tree; // Use the full tree of the branch being squashed + + var squashCommit = LocalRepository.ObjectDatabase.CreateCommit( + signature, + signature, + message, + tree, + new[] { parent }, + false); + + // Fast-forward the local target branch to the squash commit + LocalRepository.Refs.UpdateTarget(targetBranch.Reference, squashCommit.Id); + + // Fast-forward the remote tracking branch if it exists + if (targetBranch.TrackedBranch is not null) + { + LocalRepository.Refs.UpdateTarget(targetBranch.TrackedBranch.Reference, squashCommit.Id); + } + + return squashCommit; + } + public Worktree CreateWorktree(string branchName) { // Validate branch exists diff --git a/src/Stack.Tests/Integration/StackActionsTests.cs b/src/Stack.Tests/Integration/StackActionsTests.cs index 5ac7e682..9fe26bee 100644 --- a/src/Stack.Tests/Integration/StackActionsTests.cs +++ b/src/Stack.Tests/Integration/StackActionsTests.cs @@ -414,6 +414,59 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndBranchCheckedOutInWorkt childCommits.Should().Contain(c => c.MessageShort == "Parent branch change", "child branch should contain parent changes after rebase (message match due to rewritten SHA)"); } + [Fact] + public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMerged_RemovesOriginalCommitsAndKeepsSquashCommit() + { + // Arrange + var sourceBranch = Some.BranchName(); + var firstBranch = Some.BranchName(); + var secondBranch = Some.BranchName(); + var thirdBranch = Some.BranchName(); + + using var repo = new TestGitRepositoryBuilder() + .WithBranch(builder => builder.WithName(sourceBranch).PushToRemote().WithNumberOfEmptyCommits(1)) + .WithBranch(builder => builder.WithName(firstBranch).FromSourceBranch(sourceBranch).WithNumberOfEmptyCommits(3).PushToRemote()) + .WithBranch(builder => builder.WithName(secondBranch).FromSourceBranch(firstBranch).WithNumberOfEmptyCommits(1).PushToRemote()) + .WithBranch(builder => builder.WithName(thirdBranch).FromSourceBranch(secondBranch).WithNumberOfEmptyCommits(1).PushToRemote()) + .Build(); + + var logger = XUnitLogger.CreateLogger(testOutputHelper); + var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); + var gitHubClient = Substitute.For(); + var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; + var gitClientFactory = new TestGitClientFactory(testOutputHelper); + + var tipOfFirstBranch = repo.GetTipOfBranch(firstBranch); + + // Simulate squash merge of the first branch into the source branch on the remote (keeping first branch locally untouched) + gitClient.ChangeBranch(sourceBranch); + var squashCommit = repo.CreateSquashCommitFromBranchOntoBranch(firstBranch, sourceBranch, "Squash merge first branch"); + + // Delete the remote tracking branch for firstBranch to simulate PR being closed/merged & branch deleted + repo.DeleteRemoteTrackingBranch(firstBranch); + + var stack = new TestStackBuilder() + .WithSourceBranch(sourceBranch) + .WithBranch(b => b.WithName(firstBranch).WithChildBranch(c => c.WithName(secondBranch).WithChildBranch(d => d.WithName(thirdBranch)))) + .Build(); + + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + + // Act + await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); + + // Assert + var secondBranchCommits = repo.GetCommitsReachableFromBranch(secondBranch); + var thirdBranchCommits = repo.GetCommitsReachableFromBranch(thirdBranch); + + secondBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Second branch should contain squash commit from source"); + thirdBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Third branch should contain squash commit from source"); + secondBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Second branch should not contain tip commit from first branch"); + thirdBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Third branch should not contain tip commit from first branch"); + } + [Fact] public void PushChanges_WhenChangesExistOnCurrentBranch_PushesChangesCorrectly() { diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index eceb1b8c..2c075aba 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -317,17 +317,58 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List b.Name == lowestInactiveBranchToReParentFrom) : null; - var shouldRebaseOntoParent = lowestInactiveBranchToReParentFromDetail is not null && lowestInactiveBranchToReParentFromDetail.Exists; + var couldRebaseOntoParent = lowestInactiveBranchToReParentFromDetail is not null && lowestInactiveBranchToReParentFromDetail.Exists; + string? parentCommitToRebaseFrom = null; - if (shouldRebaseOntoParent) + if (couldRebaseOntoParent) { var gitClient = GetDefaultGitClient(); - shouldRebaseOntoParent = gitClient.IsAncestor(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!); + + // Get the common base between the branch we're rebasing and + // the branch we could potentially re-parent from. + var commonBase = gitClient.GetMergeBase(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!); + + if (commonBase is null) + { + // This should never happen, but if it does, we can't re-parent + // so we'll just rebase from the source branch instead. + couldRebaseOntoParent = false; + } + else + { + logger.LogDebug("Common base between {BranchToRebase} and {LowestInactiveBranchToReParentFrom} is {CommonBase}", + branchToRebaseFrom, + lowestInactiveBranchToReParentFrom, + commonBase); + + // Now check if the common base exists in the branch we're rebasing onto + // If it does, then we know that it got merged in. If it doesn't, + // then we know that it got squash merged in, so we should re-parent + // onto the new parent branch instead. + var commonBaseExistsInBranchBeingRebasedOnto = gitClient.IsCommitReachableFromBranch(commonBase, branchToRebaseOnto.Name); + + if (!commonBaseExistsInBranchBeingRebasedOnto) + { + logger.LogDebug("Common base {CommonBase} exists in branch {BranchToRebaseOnto}", + commonBase, + branchToRebaseOnto.Name); + + // Common base doesn't exist in the branch we're rebasing onto, + // so we know that it got squash merged in. We can re-parent. + parentCommitToRebaseFrom = commonBase; + } + else + { + logger.LogDebug("Common base {CommonBase} does not exist in branch {BranchToRebaseOnto}", + commonBase, + branchToRebaseOnto.Name); + } + } } - if (shouldRebaseOntoParent) + if (parentCommitToRebaseFrom is not null) { - await RebaseOntoNewParent(branchToRebaseFrom, branchToRebaseOnto.Name, lowestInactiveBranchToReParentFrom!, branchStatuses, cancellationToken); + await RebaseOntoNewParent(branchToRebaseFrom, branchToRebaseOnto.Name, parentCommitToRebaseFrom, branchStatuses, cancellationToken); } else { @@ -381,7 +422,7 @@ await displayProvider.DisplayStatusWithSuccess($"Rebasing {branch} onto {sourceB private async Task RebaseOntoNewParent( string branch, string newParentBranchName, - string oldParentBranchName, + string oldParentCommitSha, Dictionary branchStatuses, CancellationToken cancellationToken) { @@ -392,7 +433,7 @@ private async Task RebaseOntoNewParent( try { - branchGitClient.RebaseOntoNewParent(newParentBranchName, oldParentBranchName); + branchGitClient.RebaseOntoNewParent(newParentBranchName, oldParentCommitSha); } catch (ConflictException) { diff --git a/src/Stack/Git/GitClient.cs b/src/Stack/Git/GitClient.cs index be607eea..9c8b765c 100644 --- a/src/Stack/Git/GitClient.cs +++ b/src/Stack/Git/GitClient.cs @@ -32,6 +32,8 @@ public interface IGitClient string GetRootOfRepository(); string? GetConfigValue(string key); bool IsAncestor(string ancestor, string descendant); + string? GetMergeBase(string branch1, string branch2); + bool IsCommitReachableFromBranch(string commitSha, string branchName); bool IsMergeInProgress(); bool IsRebaseInProgress(); @@ -336,6 +338,40 @@ private void ExecuteGitCommand( { ExecuteGitCommandAndReturnOutput(command, captureStandardError, exceptionHandler); } + + public string? GetMergeBase(string branch1, string branch2) + { + var mergeBase = ExecuteGitCommandAndReturnOutput($"merge-base {branch1} {branch2}", false, exitCode => + { + if (exitCode == 0) + { + return null; // success + } + if (exitCode == 1) + { + return null; // no common ancestor + } + return new Exception("Failed to get merge base"); + })?.Trim(); + + return string.IsNullOrWhiteSpace(mergeBase) ? null : mergeBase; + } + + public bool IsCommitReachableFromBranch(string commitSha, string branchName) + { + var branchesThatContainTheCommit = ExecuteGitCommandAndReturnOutput($"branch --contains {commitSha}", false, exitCode => + { + if (exitCode == 1) + { + return null; // no branches contain the commit + } + return new Exception("Failed to check if commit is reachable from branch"); + })?.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) + .Select(b => b.TrimStart('*', ' ').Trim()) + .ToArray() ?? Array.Empty(); + + return branchesThatContainTheCommit.Contains(branchName); + } } internal static partial class LoggerExtensionMethods From e14bcec5c3de2749f982dd7ed1a0fc44d8f87a34 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Mon, 22 Sep 2025 22:20:53 +1000 Subject: [PATCH 02/11] Fix logging and improve test --- src/Stack.Tests/Integration/StackActionsTests.cs | 13 ++++++++++++- src/Stack/Commands/Helpers/StackActions.cs | 4 ++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Stack.Tests/Integration/StackActionsTests.cs b/src/Stack.Tests/Integration/StackActionsTests.cs index 9fe26bee..e88fa65a 100644 --- a/src/Stack.Tests/Integration/StackActionsTests.cs +++ b/src/Stack.Tests/Integration/StackActionsTests.cs @@ -415,7 +415,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndBranchCheckedOutInWorkt } [Fact] - public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMerged_RemovesOriginalCommitsAndKeepsSquashCommit() + public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMerged_ReparentsOntoSourceBranchToAvoidConflicts() { // Arrange var sourceBranch = Some.BranchName(); @@ -465,6 +465,17 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer thirdBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Third branch should contain squash commit from source"); secondBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Second branch should not contain tip commit from first branch"); thirdBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Third branch should not contain tip commit from first branch"); + + repo.CreateCommitOnRemoteTrackingBranch(sourceBranch, "New commit on source after squash merge"); + var tipOfSourceAfterNewCommit = repo.GetTipOfBranch(sourceBranch); + + // Act again to verify subsequent rebase still works + await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); + + secondBranchCommits = repo.GetCommitsReachableFromBranch(secondBranch); + thirdBranchCommits = repo.GetCommitsReachableFromBranch(thirdBranch); + secondBranchCommits.Should().Contain(c => c.Sha == tipOfSourceAfterNewCommit.Sha, "Second branch should contain latest commit from source after subsequent rebase"); + thirdBranchCommits.Should().Contain(c => c.Sha == tipOfSourceAfterNewCommit.Sha, "Third branch should contain latest commit from source after subsequent rebase"); } [Fact] diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index 2c075aba..70546a25 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -349,7 +349,7 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List Date: Tue, 23 Sep 2025 07:58:28 +1000 Subject: [PATCH 03/11] Add more rebase tests --- .../Commands/Helpers/StackActionsTests.cs | 84 ----------- .../Helpers/TestGitRepositoryBuilder.cs | 16 +++ .../Integration/StackActionsTests.cs | 135 +++++++++++------- 3 files changed, 97 insertions(+), 138 deletions(-) diff --git a/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs b/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs index 7ad5636e..8ed24d04 100644 --- a/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs +++ b/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs @@ -162,90 +162,6 @@ public async Task UpdateStack_UsingRebase_WhenConflictResolved_CompletesSuccessf gitClient.Received().ChangeBranch(feature); } - // [Fact] - // public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_RebasesOntoTheParentBranchToAvoidConflicts() - // { - // // Arrange - // var sourceBranch = Some.BranchName(); - // var branch1 = Some.BranchName(); - // var branch2 = Some.BranchName(); - // var gitClient = Substitute.For(); - // var gitHubClient = Substitute.For(); - // var logger = XUnitLogger.CreateLogger(testOutputHelper); - // var console = new TestDisplayProvider(testOutputHelper); - - // // Setup branch statuses to simulate the scenario - // gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - // { - // { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - // { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", false, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, // remote branch deleted - // { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - // }); - // gitClient.IsAncestor(branch2, branch1).Returns(true); - - // var stack = new Config.Stack( - // "Stack1", - // Some.HttpsUri().ToString(), - // sourceBranch, - // new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } - // ); - - // var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - // var factory = Substitute.For(); - // factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - // var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - // // Act - // await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); - - // // Assert - // gitClient.Received().ChangeBranch(branch2); - // gitClient.Received().RebaseOntoNewParent(sourceBranch, branch1); - // } - - // [Fact] - // public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_ButTheTargetBranchHasAlreadyHadAdditionalCommitsMergedInto_DoesNotRebaseOntoTheParentBranch() - // { - // // Arrange - // var sourceBranch = Some.BranchName(); - // var branch1 = Some.BranchName(); - // var branch2 = Some.BranchName(); - // var changedFilePath = Some.Name(); - - // var gitClient = Substitute.For(); - // var gitHubClient = Substitute.For(); - // var logger = XUnitLogger.CreateLogger(testOutputHelper); - // var console = new TestDisplayProvider(testOutputHelper); - - // // Setup branch statuses to simulate the scenario - // gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - // { - // { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - // { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", false, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, // remote branch deleted - // { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - // }); - // gitClient.IsAncestor(branch2, branch1).Returns(false); - - // var stack = new Config.Stack( - // "Stack1", - // Some.HttpsUri().ToString(), - // sourceBranch, - // new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } - // ); - - // var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - // var factory = Substitute.For(); - // factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - // var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - // // Act - // await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); - - // // Assert - // gitClient.Received().ChangeBranch(branch2); - // gitClient.Received().RebaseFromLocalSourceBranch(sourceBranch); - // } - [Fact] public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_AndLocalBranchIsDeleted_DoesNotRebaseOntoTheParentBranch() { diff --git a/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs b/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs index 538f1bc9..025a399f 100644 --- a/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs +++ b/src/Stack.Tests/Helpers/TestGitRepositoryBuilder.cs @@ -372,12 +372,28 @@ public LibGit2Sharp.Commit GetTipOfRemoteBranch(string branchName) return [.. LocalRepository.Branches]; } + public LibGit2Sharp.Branch ChangeBranch(string branchName) + { + return LibGit2Sharp.Commands.Checkout(LocalRepository, branchName); + } + public LibGit2Sharp.Commit Commit(string? message = null) { var signature = new Signature(Some.Name(), Some.Name(), DateTimeOffset.Now); return LocalRepository.Commit(message ?? Some.Name(), signature, signature); } + public LibGit2Sharp.Commit Commit(string relativePath, string contents, string? message = null) + { + var fullPath = Path.Combine(LocalDirectoryPath, relativePath); + Directory.CreateDirectory(Path.GetDirectoryName(fullPath)!); + File.WriteAllText(fullPath, contents); + LibGit2Sharp.Commands.Stage(LocalRepository, relativePath); + + var signature = new Signature(Some.Name(), Some.Name(), DateTimeOffset.Now); + return LocalRepository.Commit(message ?? Some.Name(), signature, signature); + } + public void Stage(string path) { LibGit2Sharp.Commands.Stage(LocalRepository, path); diff --git a/src/Stack.Tests/Integration/StackActionsTests.cs b/src/Stack.Tests/Integration/StackActionsTests.cs index e88fa65a..74c79ce9 100644 --- a/src/Stack.Tests/Integration/StackActionsTests.cs +++ b/src/Stack.Tests/Integration/StackActionsTests.cs @@ -24,9 +24,7 @@ public void PullChanges_WhenChangesExistOnSourceAndBranchInStack_PullsChangesCor .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); @@ -36,7 +34,7 @@ public void PullChanges_WhenChangesExistOnSourceAndBranchInStack_PullsChangesCor repo.CreateCommitOnRemoteTrackingBranch(otherBranch, "Remote change on other"); // Make source branch current - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var stack = new TestStackBuilder() .WithSourceBranch(sourceBranch) @@ -71,9 +69,7 @@ public void PullChanges_WhenChangesExistOnWorktreeBranch_PullsChangesCorrectly() .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); @@ -82,7 +78,7 @@ public void PullChanges_WhenChangesExistOnWorktreeBranch_PullsChangesCorrectly() repo.CreateCommitOnRemoteTrackingBranch(worktreeBranch, "Remote change on worktree branch"); // Switch to source branch first, then create worktree (can't create worktree for current branch) - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); // Create a worktree for the branch var worktreePath = repo.CreateWorktree(worktreeBranch); @@ -116,14 +112,12 @@ public void PullChanges_WhenLocalBranchHasNoRemoteTrackingBranch_DoesNotPullChan .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var initialLocalTip = repo.GetTipOfBranch(localOnlyBranch); var stack = new TestStackBuilder() @@ -154,9 +148,7 @@ public void PullChanges_WhenRemoteTrackingBranchIsDeleted_DoesNotPullChanges() .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); @@ -164,7 +156,7 @@ public void PullChanges_WhenRemoteTrackingBranchIsDeleted_DoesNotPullChanges() // Delete the remote tracking branch to simulate deleted remote repo.DeleteRemoteTrackingBranch(deletedRemoteBranch); - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var initialLocalTip = repo.GetTipOfBranch(deletedRemoteBranch); var stack = new TestStackBuilder() @@ -199,22 +191,20 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndChangesExistOnMultipleBr .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Add changes to source branch (simulating changes to merge) - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "source change"); repo.Stage(Path.GetFileName(filePath)); var sourceCommit = repo.Commit("Source branch change"); // Add changes to line1Branch1 (simulating changes at multiple levels) - gitClient.ChangeBranch(line1Branch1); + repo.ChangeBranch(line1Branch1); var filePath2 = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath2, "line1 change"); repo.Stage(Path.GetFileName(filePath2)); @@ -257,22 +247,20 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndChangesExistOnMultipleB .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Add changes to source branch (simulating changes to rebase onto) - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "source change"); repo.Stage(Path.GetFileName(filePath)); var sourceCommit = repo.Commit("Source branch change"); // Add changes to line1Branch1 (simulating changes at multiple levels) - gitClient.ChangeBranch(line1Branch1); + repo.ChangeBranch(line1Branch1); var filePath2 = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath2, "line1 change"); repo.Stage(Path.GetFileName(filePath2)); @@ -317,22 +305,20 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndBranchCheckedOutInWorktr .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Add change to source - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var sourceFile = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(sourceFile, "source change"); repo.Stage(Path.GetFileName(sourceFile)); var sourceCommit = repo.Commit("Source branch change"); // Add change to parent branch - gitClient.ChangeBranch(parentBranch); + repo.ChangeBranch(parentBranch); var parentFile = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(parentFile, "parent change"); repo.Stage(Path.GetFileName(parentFile)); @@ -343,7 +329,7 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndBranchCheckedOutInWorktr .WithBranch(b => b.WithName(parentBranch).WithChildBranch(c => c.WithName(childBranch))) .Build(); - gitClient.ChangeBranch(sourceBranch); // ensure not on child + repo.ChangeBranch(sourceBranch); // ensure not on child var worktree = repo.CreateWorktree(childBranch); worktree.Should().NotBeNull(); @@ -373,22 +359,20 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndBranchCheckedOutInWorkt .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Add change to source - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var sourceFile = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(sourceFile, "source change"); repo.Stage(Path.GetFileName(sourceFile)); var sourceCommit = repo.Commit("Source branch change"); // Add change to parent - gitClient.ChangeBranch(parentBranch); + repo.ChangeBranch(parentBranch); var parentFile = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(parentFile, "parent change"); repo.Stage(Path.GetFileName(parentFile)); @@ -399,7 +383,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndBranchCheckedOutInWorkt .WithBranch(b => b.WithName(parentBranch).WithChildBranch(c => c.WithName(childBranch))) .Build(); - gitClient.ChangeBranch(sourceBranch); // ensure not on child + repo.ChangeBranch(sourceBranch); // ensure not on child var worktreePath = repo.CreateWorktree(childBranch); worktreePath.Should().NotBeNull(); @@ -431,9 +415,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); @@ -441,7 +423,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var tipOfFirstBranch = repo.GetTipOfBranch(firstBranch); // Simulate squash merge of the first branch into the source branch on the remote (keeping first branch locally untouched) - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var squashCommit = repo.CreateSquashCommitFromBranchOntoBranch(firstBranch, sourceBranch, "Squash merge first branch"); // Delete the remote tracking branch for firstBranch to simulate PR being closed/merged & branch deleted @@ -478,6 +460,62 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer thirdBranchCommits.Should().Contain(c => c.Sha == tipOfSourceAfterNewCommit.Sha, "Third branch should contain latest commit from source after subsequent rebase"); } + [Fact] + public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMergedWithAdditionalCommitsNotMergedIntoChildren_ReparentsOntoSourceBranchToAvoidConflicts() + { + // Arrange + var sourceBranch = Some.BranchName(); + var firstBranch = Some.BranchName(); + var secondBranch = Some.BranchName(); + var thirdBranch = Some.BranchName(); + + using var repo = new TestGitRepositoryBuilder() + .WithBranch(builder => builder.WithName(sourceBranch).PushToRemote().WithNumberOfEmptyCommits(1)) + .WithBranch(builder => builder.WithName(firstBranch).FromSourceBranch(sourceBranch).WithNumberOfEmptyCommits(3).PushToRemote()) + .WithBranch(builder => builder.WithName(secondBranch).FromSourceBranch(firstBranch).WithNumberOfEmptyCommits(1).PushToRemote()) + .WithBranch(builder => builder.WithName(thirdBranch).FromSourceBranch(secondBranch).WithNumberOfEmptyCommits(1).PushToRemote()) + .Build(); + + var logger = XUnitLogger.CreateLogger(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var gitHubClient = Substitute.For(); + var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; + var gitClientFactory = new TestGitClientFactory(testOutputHelper); + + // Add another commit to the first branch that isn't in the children + repo.ChangeBranch(firstBranch); + repo.Commit("file.txt", Some.Name(), "Additional commit on first branch not in children"); + repo.Push(firstBranch); + + var tipOfFirstBranch = repo.GetTipOfBranch(firstBranch); + + // Simulate squash merge of the first branch into the source branch on the remote (keeping first branch locally untouched) + repo.ChangeBranch(sourceBranch); + var squashCommit = repo.CreateSquashCommitFromBranchOntoBranch(firstBranch, sourceBranch, "Squash merge first branch"); + + // Delete the remote tracking branch for firstBranch to simulate PR being closed/merged & branch deleted + repo.DeleteRemoteTrackingBranch(firstBranch); + + var stack = new TestStackBuilder() + .WithSourceBranch(sourceBranch) + .WithBranch(b => b.WithName(firstBranch).WithChildBranch(c => c.WithName(secondBranch).WithChildBranch(d => d.WithName(thirdBranch)))) + .Build(); + + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + + // Act + await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); + + // Assert + var secondBranchCommits = repo.GetCommitsReachableFromBranch(secondBranch); + var thirdBranchCommits = repo.GetCommitsReachableFromBranch(thirdBranch); + + secondBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Second branch should contain squash commit from source"); + thirdBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Third branch should contain squash commit from source"); + secondBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Second branch should not contain tip commit from first branch"); + thirdBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Third branch should not contain tip commit from first branch"); + } + [Fact] public void PushChanges_WhenChangesExistOnCurrentBranch_PushesChangesCorrectly() { @@ -491,15 +529,13 @@ public void PushChanges_WhenChangesExistOnCurrentBranch_PushesChangesCorrectly() .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Make changes to the current branch - gitClient.ChangeBranch(currentBranch); + repo.ChangeBranch(currentBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "local change"); repo.Stage(Path.GetFileName(filePath)); @@ -536,22 +572,20 @@ public void PushChanges_WhenChangesExistOnNonCurrentBranch_PushesChangesCorrectl .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Make changes to the non-current branch - gitClient.ChangeBranch(nonCurrentBranch); + repo.ChangeBranch(nonCurrentBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "local change"); repo.Stage(Path.GetFileName(filePath)); var localCommit = repo.Commit("Local change on non-current branch"); // Switch to source branch so nonCurrentBranch is non-current - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var initialRemoteCommitCount = repo.GetCommitsReachableFromRemoteBranch(nonCurrentBranch).Count; var stack = new TestStackBuilder() @@ -583,22 +617,20 @@ public void PushChanges_WhenChangesExistOnWorktreeBranch_PushesChangesCorrectly( .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Make changes to the worktree branch, then switch away to create worktree - gitClient.ChangeBranch(worktreeBranch); + repo.ChangeBranch(worktreeBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "worktree branch change"); repo.Stage(Path.GetFileName(filePath)); var branchCommit = repo.Commit("Change in worktree branch"); // Switch to source branch first, then create worktree (can't create worktree for current branch) - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); // Create a worktree for the branch var worktreePath = repo.CreateWorktree(worktreeBranch); @@ -632,21 +664,19 @@ public void PushChanges_WhenLocalOnlyBranchExists_CreatesRemoteTrackingBranch() .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Make changes to the local-only branch - gitClient.ChangeBranch(localOnlyBranch); + repo.ChangeBranch(localOnlyBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "local change"); repo.Stage(Path.GetFileName(filePath)); var localCommit = repo.Commit("Local change on local-only branch"); - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); // Verify initially no remote tracking branch var initialHasRemoteTracking = repo.DoesRemoteBranchExist(localOnlyBranch); @@ -663,10 +693,9 @@ public void PushChanges_WhenLocalOnlyBranchExists_CreatesRemoteTrackingBranch() stackActions.PushChanges(stack, 5, false); // Assert - verify branch exists locally and now HAS a remote tracking branch (because PushChanges creates it) - var branchExists = gitClient.DoesLocalBranchExist(localOnlyBranch); + var tipOfBranch = repo.GetTipOfBranch(localOnlyBranch); // Throws if branch doesn't exist var finalHasRemoteTracking = repo.DoesRemoteBranchExist(localOnlyBranch); - branchExists.Should().BeTrue("local branch should still exist"); finalHasRemoteTracking.Should().BeTrue("PushChanges should create remote tracking branch for branches without one"); // Also assert that the remote branch has the correct SHA from the local branch @@ -688,15 +717,13 @@ public void PushChanges_WhenRemoteTrackingBranchHasBeenDeleted_DoesNotPushChange .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var gitClientLogger = XUnitLogger.CreateLogger(testOutputHelper); var displayProvider = new TestDisplayProvider(testOutputHelper); - var gitClient = new GitClient(gitClientLogger, repo.LocalDirectoryPath); var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); // Make changes to the branch - gitClient.ChangeBranch(deletedRemoteBranch); + repo.ChangeBranch(deletedRemoteBranch); var filePath = Path.Join(repo.LocalDirectoryPath, Some.Name()); File.WriteAllText(filePath, "local change"); repo.Stage(Path.GetFileName(filePath)); @@ -705,7 +732,7 @@ public void PushChanges_WhenRemoteTrackingBranchHasBeenDeleted_DoesNotPushChange // Delete the remote tracking branch to simulate deleted remote repo.DeleteRemoteTrackingBranch(deletedRemoteBranch); - gitClient.ChangeBranch(sourceBranch); + repo.ChangeBranch(sourceBranch); var initialLocalCommitCount = repo.GetCommitsReachableFromBranch(deletedRemoteBranch).Count; var stack = new TestStackBuilder() From 1ce0bf430ec0d37de735f66f89fb5e840551df06 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Tue, 23 Sep 2025 08:16:14 +1000 Subject: [PATCH 04/11] Cleanup --- src/Stack/Commands/Helpers/StackActions.cs | 105 +++++++++++---------- 1 file changed, 53 insertions(+), 52 deletions(-) diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index 70546a25..b55bf317 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -287,27 +287,27 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List ", branchLine.Select(b => b.Name))); - BranchDetail? lowestActionBranch = null; + BranchDetail? lowestActiveBranchInLine = null; foreach (var branch in branchLine) { if (branch.IsActive) { - lowestActionBranch = branch; + lowestActiveBranchInLine = branch; } } - if (lowestActionBranch is null) + if (lowestActiveBranchInLine is null) { logger.NoActiveBranchesFound(); return; } - string? branchToRebaseFrom = lowestActionBranch.Name; + string? branchToRebaseFrom = lowestActiveBranchInLine.Name; string? lowestInactiveBranchToReParentFrom = null; List branchesToRebaseOnto = [.. branchLine]; branchesToRebaseOnto.Reverse(); - branchesToRebaseOnto.Remove(lowestActionBranch); + branchesToRebaseOnto.Remove(lowestActiveBranchInLine); branchesToRebaseOnto.Add(status.SourceBranch); List allBranchesInStack = [status.SourceBranch, .. branchLine]; @@ -318,53 +318,7 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List b.Name == lowestInactiveBranchToReParentFrom) : null; var couldRebaseOntoParent = lowestInactiveBranchToReParentFromDetail is not null && lowestInactiveBranchToReParentFromDetail.Exists; - string? parentCommitToRebaseFrom = null; - - if (couldRebaseOntoParent) - { - var gitClient = GetDefaultGitClient(); - - // Get the common base between the branch we're rebasing and - // the branch we could potentially re-parent from. - var commonBase = gitClient.GetMergeBase(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!); - - if (commonBase is null) - { - // This should never happen, but if it does, we can't re-parent - // so we'll just rebase from the source branch instead. - couldRebaseOntoParent = false; - } - else - { - logger.LogDebug("Common base between {BranchToRebase} and {LowestInactiveBranchToReParentFrom} is {CommonBase}", - branchToRebaseFrom, - lowestInactiveBranchToReParentFrom, - commonBase); - - // Now check if the common base exists in the branch we're rebasing onto - // If it does, then we know that it got merged in. If it doesn't, - // then we know that it got squash merged in, so we should re-parent - // onto the new parent branch instead. - var commonBaseExistsInBranchBeingRebasedOnto = gitClient.IsCommitReachableFromBranch(commonBase, branchToRebaseOnto.Name); - - if (!commonBaseExistsInBranchBeingRebasedOnto) - { - logger.LogDebug("Common base {CommonBase} does not exist in branch {BranchToRebaseOnto}, treating previous parent as being squash merged and re-parenting.", - commonBase, - branchToRebaseOnto.Name); - - // Common base doesn't exist in the branch we're rebasing onto, - // so we know that it got squash merged in. We can re-parent. - parentCommitToRebaseFrom = commonBase; - } - else - { - logger.LogDebug("Common base {CommonBase} exists in branch {BranchToRebaseOnto}, no need to re-parent", - commonBase, - branchToRebaseOnto.Name); - } - } - } + var parentCommitToRebaseFrom = couldRebaseOntoParent ? GetCommitShaToReParentFrom(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!) : null; if (parentCommitToRebaseFrom is not null) { @@ -382,6 +336,44 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List branchStatuses, CancellationToken cancellationToken) { await displayProvider.DisplayStatusWithSuccess($"Rebasing {branch} onto {sourceBranchName}", async ct => @@ -503,4 +495,13 @@ internal static partial class LoggerExtensionMethods [LoggerMessage(Level = LogLevel.Debug, Message = "Rebasing {Branch} onto new parent {NewParentBranch}")] public static partial void RebasingBranchOntoNewParent(this ILogger logger, string branch, string newParentBranch); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Common base between {SourceBranch} and {TargetBranch}: {CommitSha}")] + public static partial void CommonBaseBetweenBranches(this ILogger logger, string sourceBranch, string targetBranch, string commitSha); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Commit {CommitSha} does not exist in branch {BranchToRebaseOnto}, treating previous parent as being squash merged and re-parenting.")] + public static partial void CommitDoesNotExistInNewParent(this ILogger logger, string commitSha, string branchToRebaseOnto); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Commit {CommitSha} exists in branch {BranchToRebaseOnto}, no need to re-parent")] + public static partial void CommitExistsInNewParent(this ILogger logger, string commitSha, string branchToRebaseOnto); } From 79f5e7f6b792fd0665c05744d69c767b146e55ad Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Tue, 23 Sep 2025 08:36:05 +1000 Subject: [PATCH 05/11] Add interface for conflict detection to help with tests --- .../Commands/Helpers/StackActionsTests.cs | 275 +++++------------- .../Git/ConflictResolutionDetectorTests.cs | 28 +- .../Integration/StackActionsTests.cs | 45 ++- src/Stack/Commands/Helpers/StackActions.cs | 9 +- src/Stack/Git/ConflictResolutionDetector.cs | 15 +- .../HostApplicationBuilderExtensions.cs | 1 + 6 files changed, 143 insertions(+), 230 deletions(-) diff --git a/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs b/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs index 8ed24d04..ba9a5e0c 100644 --- a/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs +++ b/src/Stack.Tests/Commands/Helpers/StackActionsTests.cs @@ -12,16 +12,17 @@ namespace Stack.Tests.Helpers; public class StackActionsTests(ITestOutputHelper testOutputHelper) { [Fact] - public async Task UpdateStack_UsingMerge_WhenConflictAbortedBeforeProgressRecorded_ThrowsAbortException() + public async Task UpdateStack_UsingMerge_WhenConflictResolutionAborted_ThrowsAbortException() { // Arrange var sourceBranch = Some.BranchName(); var feature = Some.BranchName(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); + var conflictResolutionDetector = Substitute.For(); var stack = new Config.Stack("Stack1", Some.HttpsUri().ToString(), sourceBranch, new List { new(feature, []) }); gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary @@ -32,15 +33,14 @@ public async Task UpdateStack_UsingMerge_WhenConflictAbortedBeforeProgressRecord // Trigger conflict gitClient.When(g => g.MergeFromLocalSourceBranch(sourceBranch)).Throws(new ConflictException()); - // Simulate: initial check says merge in progress, then still in progress once, then not in progress with HEAD unchanged => aborted - gitClient.IsMergeInProgress().Returns(true, true, false); + conflictResolutionDetector + .WaitForConflictResolution(gitClient, logger, ConflictOperationType.Merge, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ConflictResolutionResult.Aborted); - var head = Some.Sha(); - gitClient.GetHeadSha().Returns(head, head, head); // unchanged var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var actions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var actions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act var act = async () => await actions.UpdateStack(stack, UpdateStrategy.Merge, CancellationToken.None); @@ -50,16 +50,17 @@ public async Task UpdateStack_UsingMerge_WhenConflictAbortedBeforeProgressRecord } [Fact] - public async Task UpdateStack_UsingMerge_WhenConflictResolved_CompletesSuccessfully() + public async Task UpdateStack_UsingMerge_WhenConflictsResolved_CompletesSuccessfully() { // Arrange var sourceBranch = Some.BranchName(); var feature = Some.BranchName(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); + var conflictResolutionDetector = Substitute.For(); var stack = new Config.Stack("Stack1", Some.HttpsUri().ToString(), sourceBranch, new List { new(feature, []) }); gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary @@ -70,13 +71,13 @@ public async Task UpdateStack_UsingMerge_WhenConflictResolved_CompletesSuccessfu gitClient.When(g => g.MergeFromLocalSourceBranch(sourceBranch)).Throws(new ConflictException()); - // Merge progress -> then resolved (not in progress) with different HEAD - gitClient.IsMergeInProgress().Returns(true, false); - gitClient.GetHeadSha().Returns(Some.Sha(), Some.Sha()); // changed + conflictResolutionDetector + .WaitForConflictResolution(gitClient, logger, ConflictOperationType.Merge, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ConflictResolutionResult.Completed); var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var actions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var actions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await actions.UpdateStack(stack, UpdateStrategy.Merge, CancellationToken.None); @@ -86,16 +87,17 @@ public async Task UpdateStack_UsingMerge_WhenConflictResolved_CompletesSuccessfu } [Fact] - public async Task UpdateStack_UsingRebase_WhenConflictAbortedBeforeProgressRecorded_ThrowsAbortException() + public async Task UpdateStack_UsingRebase_WhenConflictResolutionAborted_ThrowsAbortException() { // Arrange var source = Some.BranchName(); var feature = Some.BranchName(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); + var conflictResolutionDetector = Substitute.For(); var stack = new Config.Stack("Stack1", Some.HttpsUri().ToString(), source, new List { new(feature, []) }); gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary @@ -105,16 +107,14 @@ public async Task UpdateStack_UsingRebase_WhenConflictAbortedBeforeProgressRecor }); gitClient.When(g => g.RebaseFromLocalSourceBranch(source)).Throws(new ConflictException()); - gitClient.IsRebaseInProgress().Returns(true, true, false); - var origHead = Some.Sha(); + conflictResolutionDetector + .WaitForConflictResolution(gitClient, logger, ConflictOperationType.Rebase, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ConflictResolutionResult.Aborted); - // During rebase conflict HEAD may move; ensure orig head stored and final head equals orig to simulate abort - gitClient.GetOriginalHeadSha().Returns(origHead); - gitClient.GetHeadSha().Returns(origHead, origHead, origHead); var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var actions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var actions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act var act = async () => await actions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); @@ -124,16 +124,17 @@ public async Task UpdateStack_UsingRebase_WhenConflictAbortedBeforeProgressRecor } [Fact] - public async Task UpdateStack_UsingRebase_WhenConflictResolved_CompletesSuccessfully() + public async Task UpdateStack_UsingRebase_WhenConflictsResolved_CompletesSuccessfully() { // Arrange var source = Some.BranchName(); var feature = Some.BranchName(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); + var conflictResolutionDetector = Substitute.For(); var stack = new Config.Stack("Stack1", Some.HttpsUri().ToString(), source, new List { new(feature, []) }); gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary @@ -143,17 +144,14 @@ public async Task UpdateStack_UsingRebase_WhenConflictResolved_CompletesSuccessf }); gitClient.When(g => g.RebaseFromLocalSourceBranch(source)).Throws(new ConflictException()); - gitClient.IsRebaseInProgress().Returns(true, false); - - var origHead = Some.Sha(); - var newHead = Some.Sha(); - gitClient.GetOriginalHeadSha().Returns(origHead); - gitClient.GetHeadSha().Returns(newHead, newHead); // changed from original + conflictResolutionDetector + .WaitForConflictResolution(gitClient, logger, ConflictOperationType.Rebase, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ConflictResolutionResult.Completed); var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var actions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var actions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await actions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); @@ -162,148 +160,6 @@ public async Task UpdateStack_UsingRebase_WhenConflictResolved_CompletesSuccessf gitClient.Received().ChangeBranch(feature); } - [Fact] - public async Task UpdateStack_UsingRebase_WhenARemoteBranchIsDeleted_AndLocalBranchIsDeleted_DoesNotRebaseOntoTheParentBranch() - { - // Arrange - var sourceBranch = Some.BranchName(); - var branch1 = Some.BranchName(); - var branch2 = Some.BranchName(); - var gitClient = Substitute.For(); - var gitHubClient = Substitute.For(); - var logger = XUnitLogger.CreateLogger(testOutputHelper); - - gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - { - { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - }); - - var stack = new Config.Stack( - "Stack1", - Some.HttpsUri().ToString(), - sourceBranch, - new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()) }) } - ); - - var console = new TestDisplayProvider(testOutputHelper); - var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - var factory = Substitute.For(); - factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - gitClient.Fetch(true); - - // Act - await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); - - // Assert - gitClient.Received().ChangeBranch(branch2); - } - - [Fact] - public async Task UpdateStack_UsingRebase_WhenStackHasATreeStructure_RebasesAllBranchesCorrectly() - { - // Arrange - var sourceBranch = "source-branch"; - var branch1 = "branch-1"; - var branch2 = "branch-2"; - var branch3 = "branch-3"; - - var gitClient = Substitute.For(); - var gitHubClient = Substitute.For(); - var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); - - gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - { - { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch3, new GitBranchStatus(branch3, $"origin/{branch3}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - }); - - var stack = new Config.Stack( - "Stack1", - Some.HttpsUri().ToString(), - sourceBranch, - new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()), new Config.Branch(branch3, new List()) }) } - ); - - var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - var factory = Substitute.For(); - factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - // Act - await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); - - // Assert - Received.InOrder(() => - { - gitClient.ChangeBranch(branch2); - gitClient.RebaseFromLocalSourceBranch(branch1); - gitClient.ChangeBranch(branch2); - gitClient.RebaseFromLocalSourceBranch(sourceBranch); - gitClient.ChangeBranch(branch3); - gitClient.RebaseFromLocalSourceBranch(branch1); - gitClient.ChangeBranch(branch3); - gitClient.RebaseFromLocalSourceBranch(sourceBranch); - }); - } - - [Fact] - public async Task UpdateStack_UsingMerge_WhenStackHasATreeStructure_MergesAllBranchesCorrectly() - { - // Arrange - var sourceBranch = "source-branch"; - var branch1 = "branch-1"; - var branch2 = "branch-2"; - var branch3 = "branch-3"; - - var gitClient = Substitute.For(); - var gitHubClient = Substitute.For(); - var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); - - gitClient.GetBranchStatuses(Arg.Any()).Returns(new Dictionary - { - { sourceBranch, new GitBranchStatus(sourceBranch, $"origin/{sourceBranch}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch1, new GitBranchStatus(branch1, $"origin/{branch1}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch2, new GitBranchStatus(branch2, $"origin/{branch2}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) }, - { branch3, new GitBranchStatus(branch3, $"origin/{branch3}", true, false, 0, 0, new Commit(Some.Sha(), Some.Name())) } - }); - - var stack = new Config.Stack( - "Stack1", - Some.HttpsUri().ToString(), - sourceBranch, - new List { new Config.Branch(branch1, new List { new Config.Branch(branch2, new List()), new Config.Branch(branch3, new List()) }) } - ); - - var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; - var factory = Substitute.For(); - factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); - - // Act - await stackActions.UpdateStack(stack, UpdateStrategy.Merge, CancellationToken.None); - - // Assert that merges were attempted - Received.InOrder(() => - { - gitClient.ChangeBranch(branch1); - gitClient.MergeFromLocalSourceBranch(sourceBranch); - gitClient.ChangeBranch(branch2); - gitClient.MergeFromLocalSourceBranch(branch1); - gitClient.ChangeBranch(branch1); - gitClient.MergeFromLocalSourceBranch(sourceBranch); - gitClient.ChangeBranch(branch3); - gitClient.MergeFromLocalSourceBranch(branch1); - }); - } - [Fact] public void PullChanges_WhenSomeBranchesHaveChanges_AndOthersDoNot_OnlyPullsChangesForBranchesThatNeedIt() { @@ -315,7 +171,8 @@ public void PullChanges_WhenSomeBranchesHaveChanges_AndOthersDoNot_OnlyPullsChan var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -335,7 +192,7 @@ public void PullChanges_WhenSomeBranchesHaveChanges_AndOthersDoNot_OnlyPullsChan var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -356,7 +213,8 @@ public void PullChanges_WhenSomeBranchesDoNotExistInRemote_OnlyPullsBranchesThat var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -376,7 +234,7 @@ public void PullChanges_WhenSomeBranchesDoNotExistInRemote_OnlyPullsBranchesThat var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(executionContext.WorkingDirectory).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -398,7 +256,8 @@ public void PullChanges_WhenOnlyNonCurrentBranchesBehind_FetchesThem() var gitHubClient = Substitute.For(); gitClient.GetCurrentBranch().Returns(sourceBranch); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var statuses = new Dictionary { @@ -420,7 +279,7 @@ public void PullChanges_WhenOnlyNonCurrentBranchesBehind_FetchesThem() var worktreePath = "/worktree"; factory.Create(executionContext.WorkingDirectory).Returns(gitClient); factory.Create(worktreePath).Returns(worktreeGitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -439,7 +298,8 @@ public void PullChanges_WhenOnlyCurrentBranchBehind_PullsIt() var gitHubClient = Substitute.For(); gitClient.GetCurrentBranch().Returns(sourceBranch); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var statuses = new Dictionary { @@ -452,7 +312,7 @@ public void PullChanges_WhenOnlyCurrentBranchBehind_PullsIt() var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -471,7 +331,8 @@ public void PullChanges_WhenCurrentAndOtherBranchesBehind_PullsCurrentAndFetches var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); gitClient.GetCurrentBranch().Returns(sourceBranch); var statuses = new Dictionary @@ -489,7 +350,7 @@ public void PullChanges_WhenCurrentAndOtherBranchesBehind_PullsCurrentAndFetches var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -508,7 +369,8 @@ public void PullChanges_WhenNoBranchesBehind_DoesNothing() var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); gitClient.GetCurrentBranch().Returns(sourceBranch); var statuses = new Dictionary @@ -522,7 +384,7 @@ public void PullChanges_WhenNoBranchesBehind_DoesNothing() var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -544,7 +406,8 @@ public void PullChanges_WhenBranchIsBehind_AndCheckedOutInAnotherWorktree_PullsI var worktreeGitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); defaultGitClient.GetCurrentBranch().Returns(sourceBranch); var statuses = new Dictionary @@ -564,7 +427,7 @@ public void PullChanges_WhenBranchIsBehind_AndCheckedOutInAnotherWorktree_PullsI factory.Create(executionContext.WorkingDirectory).Returns(defaultGitClient); factory.Create(worktreePath).Returns(worktreeGitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -585,7 +448,8 @@ public void PushChanges_WhenSomeLocalBranchesAreAhead_OnlyPushesChangesForBranch var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -611,7 +475,7 @@ public void PushChanges_WhenSomeLocalBranchesAreAhead_OnlyPushesChangesForBranch var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, 5, false); @@ -631,7 +495,8 @@ public void PushChanges_WhenSomeBranchesDoNotExistInRemote_OnlyPushesBranchesTha var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -657,7 +522,7 @@ public void PushChanges_WhenSomeBranchesDoNotExistInRemote_OnlyPushesBranchesTha var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, 5, false); @@ -677,7 +542,8 @@ public void PushChanges_WhenSomeBranchesHaveNoRemoteTrackingBranch_PushesThemAsN var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -708,7 +574,7 @@ public void PushChanges_WhenSomeBranchesHaveNoRemoteTrackingBranch_PushesThemAsN var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, 5, false); @@ -730,7 +596,8 @@ public void PushChanges_WhenMaxBatchSizeIsSmaller_PushesBranchesInMultipleBatche var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -758,7 +625,7 @@ public void PushChanges_WhenMaxBatchSizeIsSmaller_PushesBranchesInMultipleBatche var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, maxBatchSize: 2, forceWithLease: false); @@ -780,7 +647,8 @@ public void PushChanges_WhenForceWithLeaseIsTrue_PassesForceWithLeaseParameterTo var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -798,7 +666,7 @@ public void PushChanges_WhenForceWithLeaseIsTrue_PassesForceWithLeaseParameterTo var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, maxBatchSize: 5, forceWithLease: true); @@ -818,7 +686,8 @@ public void PushChanges_WhenNoBranchesNeedToBePushed_DoesNotCallPushMethods() var gitClient = Substitute.For(); var gitHubClient = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); + var conflictResolutionDetector = Substitute.For(); var branchStatus = new Dictionary { @@ -838,7 +707,7 @@ public void PushChanges_WhenNoBranchesNeedToBePushed_DoesNotCallPushMethods() var executionContext = new CliExecutionContext { WorkingDirectory = "/repo" }; var factory = Substitute.For(); factory.Create(Arg.Any()).Returns(gitClient); - var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(factory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, maxBatchSize: 5, forceWithLease: false); @@ -860,9 +729,10 @@ public async Task UpdateStack_UsingMerge_WhenBranchIsInWorktree_UsesWorktreeGitC var gitHubClient = Substitute.For(); var inputProvider = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); var gitClientFactory = Substitute.For(); var worktreeGitClient = Substitute.For(); + var conflictResolutionDetector = Substitute.For(); gitClient.GetCurrentBranch().Returns(sourceBranch); gitClientFactory.Create(worktreePath).Returns(worktreeGitClient); @@ -882,7 +752,7 @@ public async Task UpdateStack_UsingMerge_WhenBranchIsInWorktree_UsesWorktreeGitC ); var executionContext = new CliExecutionContext { WorkingDirectory = "/some/path" }; - var stackActions = new StackActions(gitClientFactory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(gitClientFactory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); gitClientFactory.Create(executionContext.WorkingDirectory).Returns(gitClient); @@ -907,9 +777,10 @@ public async Task UpdateStack_UsingRebase_WhenBranchIsInWorktree_UsesWorktreeGit var gitHubClient = Substitute.For(); var inputProvider = Substitute.For(); var logger = XUnitLogger.CreateLogger(testOutputHelper); - var console = new TestDisplayProvider(testOutputHelper); + var displayProvider = new TestDisplayProvider(testOutputHelper); var gitClientFactory = Substitute.For(); var worktreeGitClient = Substitute.For(); + var conflictResolutionDetector = Substitute.For(); gitClient.GetCurrentBranch().Returns(sourceBranch); gitClientFactory.Create(worktreePath).Returns(worktreeGitClient); @@ -929,7 +800,7 @@ public async Task UpdateStack_UsingRebase_WhenBranchIsInWorktree_UsesWorktreeGit ); var executionContext = new CliExecutionContext { WorkingDirectory = "/some/path" }; - var stackActions = new StackActions(gitClientFactory, executionContext, gitHubClient, logger, console); + var stackActions = new StackActions(gitClientFactory, executionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); gitClientFactory.Create(executionContext.WorkingDirectory).Returns(gitClient); diff --git a/src/Stack.Tests/Git/ConflictResolutionDetectorTests.cs b/src/Stack.Tests/Git/ConflictResolutionDetectorTests.cs index 0bfd6427..4bfaa843 100644 --- a/src/Stack.Tests/Git/ConflictResolutionDetectorTests.cs +++ b/src/Stack.Tests/Git/ConflictResolutionDetectorTests.cs @@ -40,7 +40,9 @@ public async Task WaitForConflictResolution_WhenNotStarted_ReturnsNotStarted() var logger = CreateLogger(); var git = new GitClient(XUnitLogger.CreateLogger(testOutputHelper), repo.LocalDirectoryPath); - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var conflictResolutionDetector = new ConflictResolutionDetector(); + + var result = await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Merge, @@ -64,6 +66,8 @@ public async Task WaitForConflictResolution_WhenMergeCompletes_ReturnsCompleted( var logger = CreateLogger(); var git = new GitClient(XUnitLogger.CreateLogger(testOutputHelper), repo.LocalDirectoryPath); + var conflictResolutionDetector = new ConflictResolutionDetector(); + var relFile = Some.Name(); var filePath = Path.Join(repo.LocalDirectoryPath, relFile); @@ -99,7 +103,7 @@ public async Task WaitForConflictResolution_WhenMergeCompletes_ReturnsCompleted( RunGit(repo.LocalDirectoryPath, "commit -m resolved-merge"); }); - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Merge, @@ -144,7 +148,9 @@ public async Task WaitForConflictResolution_WhenMergeAborted_ReturnsAborted() // Abort after delay var aborter = Task.Run(async () => { await Task.Delay(60); git.AbortMerge(); }); - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var conflictResolutionDetector = new ConflictResolutionDetector(); + + var result = await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Merge, @@ -186,8 +192,10 @@ public async Task WaitForConflictResolution_WhenTimeoutReached_ReturnsTimeout() git.ChangeBranch(branchBase); try { git.MergeFromLocalSourceBranch(branchOther); } catch (ConflictException) { } + var conflictResolutionDetector = new ConflictResolutionDetector(); + // Do not resolve or abort; should timeout - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Merge, @@ -225,12 +233,14 @@ public async Task WaitForConflictResolution_WhenCancelled_Throws() repo.Stage(relFile); repo.Commit(); + var conflictResolutionDetector = new ConflictResolutionDetector(); + // start rebase that will conflict try { git.RebaseFromLocalSourceBranch(branchBase); } catch (ConflictException) { } using var cts = new CancellationTokenSource(); cts.CancelAfter(50); - var act = async () => await ConflictResolutionDetector.WaitForConflictResolution( + var act = async () => await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Rebase, @@ -268,6 +278,8 @@ public async Task WaitForConflictResolution_WhenRebaseCompletes_ReturnsCompleted repo.Stage(relFile); repo.Commit(); + var conflictResolutionDetector = new ConflictResolutionDetector(); + git.ChangeBranch(featureBranch); try { git.RebaseFromLocalSourceBranch(baseBranch); } catch (ConflictException) { } @@ -299,7 +311,7 @@ public async Task WaitForConflictResolution_WhenRebaseCompletes_ReturnsCompleted } }); - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Rebase, @@ -338,12 +350,14 @@ public async Task WaitForConflictResolution_WhenRebaseAborted_ReturnsAborted() repo.Stage(relFile); repo.Commit(); + var conflictResolutionDetector = new ConflictResolutionDetector(); + git.ChangeBranch(featureBranch); try { git.RebaseFromLocalSourceBranch(baseBranch); } catch (ConflictException) { } var aborter = Task.Run(async () => { await Task.Delay(60); git.AbortRebase(); }); - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( git, logger, ConflictOperationType.Rebase, diff --git a/src/Stack.Tests/Integration/StackActionsTests.cs b/src/Stack.Tests/Integration/StackActionsTests.cs index 74c79ce9..e01b5709 100644 --- a/src/Stack.Tests/Integration/StackActionsTests.cs +++ b/src/Stack.Tests/Integration/StackActionsTests.cs @@ -28,6 +28,7 @@ public void PullChanges_WhenChangesExistOnSourceAndBranchInStack_PullsChangesCor var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Create commits on remote tracking branches to simulate changes to pull repo.CreateCommitOnRemoteTrackingBranch(sourceBranch, "Remote change on source"); @@ -41,7 +42,7 @@ public void PullChanges_WhenChangesExistOnSourceAndBranchInStack_PullsChangesCor .WithBranch(b => b.WithName(otherBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -73,6 +74,7 @@ public void PullChanges_WhenChangesExistOnWorktreeBranch_PullsChangesCorrectly() var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Create commits on remote tracking branch to simulate changes to pull repo.CreateCommitOnRemoteTrackingBranch(worktreeBranch, "Remote change on worktree branch"); @@ -88,7 +90,7 @@ public void PullChanges_WhenChangesExistOnWorktreeBranch_PullsChangesCorrectly() .WithBranch(b => b.WithName(worktreeBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -116,6 +118,7 @@ public void PullChanges_WhenLocalBranchHasNoRemoteTrackingBranch_DoesNotPullChan var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); repo.ChangeBranch(sourceBranch); var initialLocalTip = repo.GetTipOfBranch(localOnlyBranch); @@ -125,7 +128,7 @@ public void PullChanges_WhenLocalBranchHasNoRemoteTrackingBranch_DoesNotPullChan .WithBranch(b => b.WithName(localOnlyBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -152,6 +155,7 @@ public void PullChanges_WhenRemoteTrackingBranchIsDeleted_DoesNotPullChanges() var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Delete the remote tracking branch to simulate deleted remote repo.DeleteRemoteTrackingBranch(deletedRemoteBranch); @@ -164,7 +168,7 @@ public void PullChanges_WhenRemoteTrackingBranchIsDeleted_DoesNotPullChanges() .WithBranch(b => b.WithName(deletedRemoteBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PullChanges(stack); @@ -195,6 +199,7 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndChangesExistOnMultipleBr var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Add changes to source branch (simulating changes to merge) repo.ChangeBranch(sourceBranch); @@ -216,7 +221,7 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndChangesExistOnMultipleBr .WithBranch(b => b.WithName(line2Branch1)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await stackActions.UpdateStack(stack, UpdateStrategy.Merge, CancellationToken.None); @@ -251,6 +256,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndChangesExistOnMultipleB var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Add changes to source branch (simulating changes to rebase onto) repo.ChangeBranch(sourceBranch); @@ -272,7 +278,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndChangesExistOnMultipleB .WithBranch(b => b.WithName(line2Branch1)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); @@ -309,6 +315,7 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndBranchCheckedOutInWorktr var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Add change to source repo.ChangeBranch(sourceBranch); @@ -333,7 +340,7 @@ public async Task UpdateStack_WhenUpdatingUsingMerge_AndBranchCheckedOutInWorktr var worktree = repo.CreateWorktree(childBranch); worktree.Should().NotBeNull(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await stackActions.UpdateStack(stack, UpdateStrategy.Merge, CancellationToken.None); @@ -363,6 +370,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndBranchCheckedOutInWorkt var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Add change to source repo.ChangeBranch(sourceBranch); @@ -387,7 +395,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndBranchCheckedOutInWorkt var worktreePath = repo.CreateWorktree(childBranch); worktreePath.Should().NotBeNull(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); @@ -419,6 +427,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); var tipOfFirstBranch = repo.GetTipOfBranch(firstBranch); @@ -434,7 +443,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer .WithBranch(b => b.WithName(firstBranch).WithChildBranch(c => c.WithName(secondBranch).WithChildBranch(d => d.WithName(thirdBranch)))) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); @@ -481,6 +490,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Add another commit to the first branch that isn't in the children repo.ChangeBranch(firstBranch); @@ -501,7 +511,7 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer .WithBranch(b => b.WithName(firstBranch).WithChildBranch(c => c.WithName(secondBranch).WithChildBranch(d => d.WithName(thirdBranch)))) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act await stackActions.UpdateStack(stack, UpdateStrategy.Rebase, CancellationToken.None); @@ -533,6 +543,7 @@ public void PushChanges_WhenChangesExistOnCurrentBranch_PushesChangesCorrectly() var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Make changes to the current branch repo.ChangeBranch(currentBranch); @@ -548,7 +559,7 @@ public void PushChanges_WhenChangesExistOnCurrentBranch_PushesChangesCorrectly() .WithBranch(b => b.WithName(currentBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, 5, false); @@ -576,6 +587,7 @@ public void PushChanges_WhenChangesExistOnNonCurrentBranch_PushesChangesCorrectl var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Make changes to the non-current branch repo.ChangeBranch(nonCurrentBranch); @@ -593,7 +605,7 @@ public void PushChanges_WhenChangesExistOnNonCurrentBranch_PushesChangesCorrectl .WithBranch(b => b.WithName(nonCurrentBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, 5, false); @@ -621,6 +633,7 @@ public void PushChanges_WhenChangesExistOnWorktreeBranch_PushesChangesCorrectly( var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Make changes to the worktree branch, then switch away to create worktree repo.ChangeBranch(worktreeBranch); @@ -640,7 +653,7 @@ public void PushChanges_WhenChangesExistOnWorktreeBranch_PushesChangesCorrectly( .WithBranch(b => b.WithName(worktreeBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act stackActions.PushChanges(stack, 5, false); @@ -668,6 +681,7 @@ public void PushChanges_WhenLocalOnlyBranchExists_CreatesRemoteTrackingBranch() var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Make changes to the local-only branch repo.ChangeBranch(localOnlyBranch); @@ -687,7 +701,7 @@ public void PushChanges_WhenLocalOnlyBranchExists_CreatesRemoteTrackingBranch() .WithBranch(b => b.WithName(localOnlyBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act - should complete without errors and should actually create the remote tracking branch stackActions.PushChanges(stack, 5, false); @@ -721,6 +735,7 @@ public void PushChanges_WhenRemoteTrackingBranchHasBeenDeleted_DoesNotPushChange var gitHubClient = Substitute.For(); var cliExecutionContext = new CliExecutionContext() { WorkingDirectory = repo.LocalDirectoryPath }; var gitClientFactory = new TestGitClientFactory(testOutputHelper); + var conflictResolutionDetector = new ConflictResolutionDetector(); // Make changes to the branch repo.ChangeBranch(deletedRemoteBranch); @@ -740,7 +755,7 @@ public void PushChanges_WhenRemoteTrackingBranchHasBeenDeleted_DoesNotPushChange .WithBranch(b => b.WithName(deletedRemoteBranch)) .Build(); - var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider); + var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); // Act - should complete without errors even with deleted remote stackActions.PushChanges(stack, 5, false); diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index b55bf317..5e166aec 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -20,7 +20,8 @@ public class StackActions( CliExecutionContext executionContext, IGitHubClient gitHubClient, ILogger logger, - IDisplayProvider displayProvider) : IStackActions + IDisplayProvider displayProvider, + IConflictResolutionDetector conflictResolutionDetector) : IStackActions { /// /// Gets the default GitClient for the current working directory @@ -216,7 +217,7 @@ private async Task MergeFromSourceBranch(string branch, string sourceBranchName, } catch (ConflictException) { - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( branchGitClient, logger, ConflictOperationType.Merge, @@ -387,7 +388,7 @@ await displayProvider.DisplayStatusWithSuccess($"Rebasing {branch} onto {sourceB } catch (ConflictException) { - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( branchGitClient, logger, ConflictOperationType.Rebase, @@ -429,7 +430,7 @@ private async Task RebaseOntoNewParent( } catch (ConflictException) { - var result = await ConflictResolutionDetector.WaitForConflictResolution( + var result = await conflictResolutionDetector.WaitForConflictResolution( branchGitClient, logger, ConflictOperationType.Rebase, diff --git a/src/Stack/Git/ConflictResolutionDetector.cs b/src/Stack/Git/ConflictResolutionDetector.cs index a994d211..797f0d5f 100644 --- a/src/Stack/Git/ConflictResolutionDetector.cs +++ b/src/Stack/Git/ConflictResolutionDetector.cs @@ -5,9 +5,20 @@ namespace Stack.Git; public enum ConflictOperationType { Merge, Rebase } public enum ConflictResolutionResult { Completed, Aborted, NotStarted, Timeout } -public static class ConflictResolutionDetector +public interface IConflictResolutionDetector { - public static async Task WaitForConflictResolution( + Task WaitForConflictResolution( + IGitClient gitClient, + ILogger logger, + ConflictOperationType operationType, + TimeSpan pollInterval, + TimeSpan? timeout, + CancellationToken cancellationToken); +} + +public class ConflictResolutionDetector : IConflictResolutionDetector +{ + public async Task WaitForConflictResolution( IGitClient gitClient, ILogger logger, ConflictOperationType operationType, diff --git a/src/Stack/Infrastructure/HostApplicationBuilderExtensions.cs b/src/Stack/Infrastructure/HostApplicationBuilderExtensions.cs index 6f81fca0..d40e493e 100644 --- a/src/Stack/Infrastructure/HostApplicationBuilderExtensions.cs +++ b/src/Stack/Infrastructure/HostApplicationBuilderExtensions.cs @@ -91,6 +91,7 @@ private static void ConfigureServices(this IServiceCollection services, string[] var cache = provider.GetRequiredService(); return new CachingGitHubClient(safe, cache); }); + services.AddSingleton(); services.AddSingleton(); RegisterCommandHandlers(services); From 9b26edb568a2eb885206ed721a6862b349666d44 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Wed, 24 Sep 2025 07:54:50 +1000 Subject: [PATCH 06/11] Fix re-parent logic --- src/Stack/Commands/Helpers/StackActions.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index 5e166aec..7a7b210d 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -319,7 +319,7 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List b.Name == lowestInactiveBranchToReParentFrom) : null; var couldRebaseOntoParent = lowestInactiveBranchToReParentFromDetail is not null && lowestInactiveBranchToReParentFromDetail.Exists; - var parentCommitToRebaseFrom = couldRebaseOntoParent ? GetCommitShaToReParentFrom(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!) : null; + var parentCommitToRebaseFrom = couldRebaseOntoParent ? GetCommitShaToReParentFrom(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!, branchToRebaseOnto.Name) : null; if (parentCommitToRebaseFrom is not null) { @@ -337,7 +337,7 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List Date: Wed, 24 Sep 2025 09:26:30 +1000 Subject: [PATCH 07/11] Improve conflict detection and process exception handling --- src/Stack/Commands/Helpers/StackActions.cs | 9 +- src/Stack/Git/GitClient.cs | 98 +++++++++++----------- src/Stack/Git/GitHubClient.cs | 4 +- src/Stack/Git/ProcessHelpers.cs | 29 ++++--- 4 files changed, 71 insertions(+), 69 deletions(-) diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index 7a7b210d..6b079272 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -234,8 +234,7 @@ private async Task MergeFromSourceBranch(string branch, string sourceBranchName, case ConflictResolutionResult.Timeout: throw new TimeoutException("Timed out waiting for merge conflict resolution."); case ConflictResolutionResult.NotStarted: - logger.LogWarning("Expected merge to be in progress but marker not found. Proceeding cautiously."); - break; + throw new Exception("Expected merge to be in progress but it is not. Use --verbose output for more details."); } } } @@ -405,8 +404,7 @@ await displayProvider.DisplayStatusWithSuccess($"Rebasing {branch} onto {sourceB case ConflictResolutionResult.Timeout: throw new TimeoutException("Timed out waiting for rebase conflict resolution."); case ConflictResolutionResult.NotStarted: - logger.LogWarning("Expected rebase to be in progress but marker not found. Proceeding cautiously."); - break; + throw new Exception("Expected rebase to be in progress but it is not. Use --verbose output for more details."); } } }, cancellationToken); @@ -447,8 +445,7 @@ private async Task RebaseOntoNewParent( case ConflictResolutionResult.Timeout: throw new TimeoutException("Timed out waiting for rebase conflict resolution."); case ConflictResolutionResult.NotStarted: - logger.LogWarning("Expected rebase to be in progress but marker not found. Proceeding cautiously."); - break; + throw new Exception("Expected rebase to be in progress but it is not. Use --verbose output for more details."); } } }, cancellationToken); diff --git a/src/Stack/Git/GitClient.cs b/src/Stack/Git/GitClient.cs index 9c8b765c..8954fdb8 100644 --- a/src/Stack/Git/GitClient.cs +++ b/src/Stack/Git/GitClient.cs @@ -120,14 +120,14 @@ public string GetRootOfRepository() public string? GetConfigValue(string key) { - var configValue = ExecuteGitCommandAndReturnOutput($"config --get {key}", false, exitCode => + var configValue = ExecuteGitCommandAndReturnOutput($"config --get {key}", false, (info, result) => { - if (exitCode == 1) + if (result.ExitCode == 1) { - return null; + return; } - return new Exception("Failed to get config value."); + throw new Exception("Failed to get config value."); })?.Trim(); return string.IsNullOrEmpty(configValue) ? null : configValue; @@ -137,10 +137,9 @@ public bool IsAncestor(string ancestor, string descendant) { var isAncestor = false; - ExecuteGitCommand($"merge-base --is-ancestor {ancestor} {descendant}", false, exitCode => + ExecuteGitCommand($"merge-base --is-ancestor {ancestor} {descendant}", false, (info, result) => { - isAncestor = exitCode == 0; - return null; + isAncestor = result.ExitCode == 0; }); return isAncestor; @@ -150,10 +149,9 @@ public bool IsMergeInProgress() { var inProgress = true; // "git rev-parse -q --verify MERGE_HEAD" returns 0 when a merge is in progress - ExecuteGitCommand("rev-parse -q --verify MERGE_HEAD", false, exitCode => + ExecuteGitCommand("rev-parse -q --verify MERGE_HEAD", false, (info, result) => { - inProgress = exitCode == 0; - return null; // suppress exception + inProgress = result.ExitCode == 0; }); return inProgress; } @@ -186,17 +184,12 @@ public string GetHeadSha() { // ORIG_HEAD is updated by Git before dangerous operations (merge, rebase, reset). // Use quiet verify; exit code 0 when ref exists, 1 otherwise. - string? orig = ExecuteGitCommandAndReturnOutput("rev-parse -q --verify ORIG_HEAD", false, exitCode => + string? orig = ExecuteGitCommandAndReturnOutput("rev-parse -q --verify ORIG_HEAD", false, (info, result) => { - if (exitCode == 0) + if (result.ExitCode > 1) { - return null; // success + throw new Exception("Failed to read ORIG_HEAD"); } - if (exitCode == 1) - { - return null; // ref not found; treat as null without throwing - } - return new Exception("Failed to read ORIG_HEAD"); })?.Trim(); return string.IsNullOrWhiteSpace(orig) ? null : orig; @@ -256,40 +249,43 @@ public void DeleteLocalBranch(string branchName) public void MergeFromLocalSourceBranch(string sourceBranchName) { - ExecuteGitCommand($"merge {sourceBranchName}", false, exitCode => + ExecuteGitCommand($"merge {sourceBranchName}", false, (info, result) => { - if (exitCode > 0) + if (result.StandardOutput.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase) || + result.StandardError.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase)) { - return new ConflictException(); + throw new ConflictException(); } - return null; + throw new ProcessException(result.StandardError, info.FileName, info.Arguments, result.ExitCode); }); } public void RebaseFromLocalSourceBranch(string sourceBranchName) { - ExecuteGitCommand($"rebase {sourceBranchName} --update-refs", false, exitCode => + ExecuteGitCommand($"rebase {sourceBranchName} --update-refs", false, (info, result) => { - if (exitCode > 0) + if (result.StandardOutput.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase) || + result.StandardError.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase)) { - return new ConflictException(); + throw new ConflictException(); } - return null; + throw new ProcessException(result.StandardError, info.FileName, info.Arguments, result.ExitCode); }); } public void RebaseOntoNewParent(string newParentBranchName, string oldParentBranchName) { - ExecuteGitCommand($"rebase --onto {newParentBranchName} {oldParentBranchName} --update-refs", false, exitCode => + ExecuteGitCommand($"rebase --onto {newParentBranchName} {oldParentBranchName} --update-refs", false, (info, result) => { - if (exitCode > 0) + if (result.StandardOutput.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase) || + result.StandardError.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase)) { - return new ConflictException(); + throw new ConflictException(); } - return null; + throw new ProcessException(result.StandardError, info.FileName, info.Arguments, result.ExitCode); }); } @@ -305,23 +301,24 @@ public void AbortRebase() public void ContinueRebase() { - ExecuteGitCommand($"rebase --continue", false, exitCode => + ExecuteGitCommand($"rebase --continue", false, (info, result) => { - if (exitCode > 0) + if (result.StandardOutput.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase) || + result.StandardError.Contains("CONFLICT", StringComparison.OrdinalIgnoreCase)) { - return new ConflictException(); + throw new ConflictException(); } - return null; + throw new ProcessException(result.StandardError, info.FileName, info.Arguments, result.ExitCode); }); } private string ExecuteGitCommandAndReturnOutput( string command, bool captureStandardError = false, - Func? exceptionHandler = null) + Action? exceptionHandler = null) { - return ProcessHelpers.ExecuteProcessAndReturnOutput( + var result = ProcessHelpers.ExecuteProcessAndReturnOutput( "git", command, workingDirectory, @@ -329,29 +326,33 @@ private string ExecuteGitCommandAndReturnOutput( captureStandardError, exceptionHandler ); + + var output = result.StandardOutput; + + if (captureStandardError && !string.IsNullOrWhiteSpace(result.StandardError)) + { + output += $"{Environment.NewLine}{result.StandardError}"; + } + + return output; } private void ExecuteGitCommand( string command, bool captureStandardError = false, - Func? exceptionHandler = null) + Action? exceptionHandler = null) { ExecuteGitCommandAndReturnOutput(command, captureStandardError, exceptionHandler); } public string? GetMergeBase(string branch1, string branch2) { - var mergeBase = ExecuteGitCommandAndReturnOutput($"merge-base {branch1} {branch2}", false, exitCode => + var mergeBase = ExecuteGitCommandAndReturnOutput($"merge-base {branch1} {branch2}", false, (info, result) => { - if (exitCode == 0) - { - return null; // success - } - if (exitCode == 1) + if (result.ExitCode > 1) { - return null; // no common ancestor + throw new Exception("Failed to get merge base"); } - return new Exception("Failed to get merge base"); })?.Trim(); return string.IsNullOrWhiteSpace(mergeBase) ? null : mergeBase; @@ -359,13 +360,12 @@ private void ExecuteGitCommand( public bool IsCommitReachableFromBranch(string commitSha, string branchName) { - var branchesThatContainTheCommit = ExecuteGitCommandAndReturnOutput($"branch --contains {commitSha}", false, exitCode => + var branchesThatContainTheCommit = ExecuteGitCommandAndReturnOutput($"branch --contains {commitSha}", false, (info, result) => { - if (exitCode == 1) + if (result.ExitCode > 1) { - return null; // no branches contain the commit + throw new Exception("Failed to check if commit is reachable from branch"); } - return new Exception("Failed to check if commit is reachable from branch"); })?.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) .Select(b => b.TrimStart('*', ' ').Trim()) .ToArray() ?? Array.Empty(); diff --git a/src/Stack/Git/GitHubClient.cs b/src/Stack/Git/GitHubClient.cs index 0ae9ee8e..8c24a020 100644 --- a/src/Stack/Git/GitHubClient.cs +++ b/src/Stack/Git/GitHubClient.cs @@ -111,7 +111,7 @@ public void OpenPullRequest(GitHubPullRequest pullRequest) private string ExecuteGitHubCommandAndReturnOutput(string command) { - return ProcessHelpers.ExecuteProcessAndReturnOutput( + var result = ProcessHelpers.ExecuteProcessAndReturnOutput( "gh", command, context.WorkingDirectory, @@ -119,6 +119,8 @@ private string ExecuteGitHubCommandAndReturnOutput(string command) false, null ); + + return result.StandardOutput.Trim(); } private void ExecuteGitHubCommand(string command) diff --git a/src/Stack/Git/ProcessHelpers.cs b/src/Stack/Git/ProcessHelpers.cs index 1e409d3b..4e74388f 100644 --- a/src/Stack/Git/ProcessHelpers.cs +++ b/src/Stack/Git/ProcessHelpers.cs @@ -6,15 +6,18 @@ namespace Stack.Git; +public record ProcessExecutionResult(int ExitCode, string StandardOutput, string StandardError); +public record ProcessExecutionInfo(string FileName, string Arguments, string? WorkingDirectory); + public static class ProcessHelpers { - public static string ExecuteProcessAndReturnOutput( + public static ProcessExecutionResult ExecuteProcessAndReturnOutput( string fileName, string command, string? workingDirectory, ILogger logger, bool captureStandardError = false, - Func? exceptionHandler = null) + Action? exceptionHandler = null) { logger.ExecutingCommand(fileName, command); @@ -33,7 +36,7 @@ public static string ExecuteProcessAndReturnOutput( }; using var process = Process.Start(psi); - if (process is null) return string.Empty; + if (process is null) return new ProcessExecutionResult(-1, string.Empty, string.Empty); process.OutputDataReceived += (_, e) => { @@ -49,23 +52,23 @@ public static string ExecuteProcessAndReturnOutput( process.BeginOutputReadLine(); process.WaitForExit(); - int result = process.ExitCode; + int exitCode = process.ExitCode; + var standardOutput = infoBuilder.ToString(); + var standardError = errorBuilder.ToString(); + var result = new ProcessExecutionResult(exitCode, standardOutput, standardError); + var info = new ProcessExecutionInfo(fileName, command, workingDirectory); - if (result != 0) + if (exitCode != 0) { - logger.CommandFailed(fileName, command, result, errorBuilder.ToString()); + logger.CommandFailed(fileName, command, exitCode, errorBuilder.ToString()); if (exceptionHandler != null) { - var exception = exceptionHandler(result); - if (exception != null) - { - throw exception; - } + exceptionHandler(info, result); } else { - throw new ProcessException(errorBuilder.ToString(), fileName, command, result); + throw new ProcessException(errorBuilder.ToString(), fileName, command, exitCode); } } @@ -81,7 +84,7 @@ public static string ExecuteProcessAndReturnOutput( output += $"{Environment.NewLine}{errorBuilder}"; } - return output; + return result; } } From 87b307dfbc8ce77a63b783142b82616219ec8172 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Wed, 24 Sep 2025 22:31:46 +1000 Subject: [PATCH 08/11] Refactor rebase algorithm --- src/Stack/Commands/Helpers/StackActions.cs | 59 ++++++++++++---------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index 6b079272..8de082bd 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -285,54 +285,57 @@ private async Task UpdateBranchLineUsingRebase(StackStatus status, List ", branchLine.Select(b => b.Name))); + List allBranchesInLine = [status.SourceBranch, .. branchLine]; - BranchDetail? lowestActiveBranchInLine = null; foreach (var branch in branchLine) { - if (branch.IsActive) + if (!branch.IsActive) { - lowestActiveBranchInLine = branch; + logger.TraceSkippingInactiveBranch(branch.Name); + continue; } - } - if (lowestActiveBranchInLine is null) - { - logger.NoActiveBranchesFound(); - return; - } - - string? branchToRebaseFrom = lowestActiveBranchInLine.Name; - string? lowestInactiveBranchToReParentFrom = null; + string? lowestInactiveBranchToReParentFrom = null; + List branchesToRebaseOnto = []; - List branchesToRebaseOnto = [.. branchLine]; - branchesToRebaseOnto.Reverse(); - branchesToRebaseOnto.Remove(lowestActiveBranchInLine); - branchesToRebaseOnto.Add(status.SourceBranch); + // Find all active branches above this one to + // rebase onto. Also work out if there is any that + // are inactive that we need to re-parent from in + // order to handle squash merges. + foreach (var branchToRebaseOnto in allBranchesInLine) + { + if (branchToRebaseOnto.Name == branch.Name) + { + break; + } - List allBranchesInStack = [status.SourceBranch, .. branchLine]; + if (branchToRebaseOnto.IsActive) + { + branchesToRebaseOnto.Add(branchToRebaseOnto); + } + else if (lowestInactiveBranchToReParentFrom is null) + { + lowestInactiveBranchToReParentFrom = branchToRebaseOnto.Name; + } + } - foreach (var branchToRebaseOnto in branchesToRebaseOnto) - { - if (branchToRebaseOnto.IsActive) + foreach (var branchToRebaseOnto in branchesToRebaseOnto) { - var lowestInactiveBranchToReParentFromDetail = lowestInactiveBranchToReParentFrom is not null ? allBranchesInStack.First(b => b.Name == lowestInactiveBranchToReParentFrom) : null; + var lowestInactiveBranchToReParentFromDetail = lowestInactiveBranchToReParentFrom is not null ? allBranchesInLine.First(b => b.Name == lowestInactiveBranchToReParentFrom) : null; var couldRebaseOntoParent = lowestInactiveBranchToReParentFromDetail is not null && lowestInactiveBranchToReParentFromDetail.Exists; - var parentCommitToRebaseFrom = couldRebaseOntoParent ? GetCommitShaToReParentFrom(branchToRebaseFrom, lowestInactiveBranchToReParentFrom!, branchToRebaseOnto.Name) : null; + var parentCommitToRebaseFrom = couldRebaseOntoParent ? GetCommitShaToReParentFrom(branch.Name, lowestInactiveBranchToReParentFrom!, branchToRebaseOnto.Name) : null; if (parentCommitToRebaseFrom is not null) { - await RebaseOntoNewParent(branchToRebaseFrom, branchToRebaseOnto.Name, parentCommitToRebaseFrom, branchStatuses, cancellationToken); + await RebaseOntoNewParent(branch.Name, branchToRebaseOnto.Name, parentCommitToRebaseFrom, branchStatuses, cancellationToken); } else { - await RebaseFromSourceBranch(branchToRebaseFrom, branchToRebaseOnto.Name, branchStatuses, cancellationToken); + await RebaseFromSourceBranch(branch.Name, branchToRebaseOnto.Name, branchStatuses, cancellationToken); } } - else if (lowestInactiveBranchToReParentFrom is null) - { - lowestInactiveBranchToReParentFrom = branchToRebaseOnto.Name; - } } } From ce2d178eef02f54fd115f8e1bd92492e2cafabbb Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Thu, 25 Sep 2025 08:04:54 +1000 Subject: [PATCH 09/11] Improve test coverage --- .../Integration/StackActionsTests.cs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Stack.Tests/Integration/StackActionsTests.cs b/src/Stack.Tests/Integration/StackActionsTests.cs index e01b5709..735802df 100644 --- a/src/Stack.Tests/Integration/StackActionsTests.cs +++ b/src/Stack.Tests/Integration/StackActionsTests.cs @@ -414,12 +414,14 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var firstBranch = Some.BranchName(); var secondBranch = Some.BranchName(); var thirdBranch = Some.BranchName(); + var fourthBranch = Some.BranchName(); using var repo = new TestGitRepositoryBuilder() .WithBranch(builder => builder.WithName(sourceBranch).PushToRemote().WithNumberOfEmptyCommits(1)) .WithBranch(builder => builder.WithName(firstBranch).FromSourceBranch(sourceBranch).WithNumberOfEmptyCommits(3).PushToRemote()) .WithBranch(builder => builder.WithName(secondBranch).FromSourceBranch(firstBranch).WithNumberOfEmptyCommits(1).PushToRemote()) .WithBranch(builder => builder.WithName(thirdBranch).FromSourceBranch(secondBranch).WithNumberOfEmptyCommits(1).PushToRemote()) + .WithBranch(builder => builder.WithName(fourthBranch).FromSourceBranch(secondBranch).WithNumberOfEmptyCommits(1).PushToRemote()) .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); @@ -440,7 +442,10 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var stack = new TestStackBuilder() .WithSourceBranch(sourceBranch) - .WithBranch(b => b.WithName(firstBranch).WithChildBranch(c => c.WithName(secondBranch).WithChildBranch(d => d.WithName(thirdBranch)))) + .WithBranch(b => b.WithName(firstBranch) + .WithChildBranch(c => c.WithName(secondBranch) + .WithChildBranch(d => d.WithName(thirdBranch)) + .WithChildBranch(e => e.WithName(fourthBranch)))) .Build(); var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); @@ -451,11 +456,14 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer // Assert var secondBranchCommits = repo.GetCommitsReachableFromBranch(secondBranch); var thirdBranchCommits = repo.GetCommitsReachableFromBranch(thirdBranch); + var fourthBranchCommits = repo.GetCommitsReachableFromBranch(fourthBranch); secondBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Second branch should contain squash commit from source"); thirdBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Third branch should contain squash commit from source"); + fourthBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Fourth branch should contain squash commit from source"); secondBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Second branch should not contain tip commit from first branch"); thirdBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Third branch should not contain tip commit from first branch"); + fourthBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Fourth branch should not contain tip commit from first branch"); repo.CreateCommitOnRemoteTrackingBranch(sourceBranch, "New commit on source after squash merge"); var tipOfSourceAfterNewCommit = repo.GetTipOfBranch(sourceBranch); @@ -465,8 +473,10 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer secondBranchCommits = repo.GetCommitsReachableFromBranch(secondBranch); thirdBranchCommits = repo.GetCommitsReachableFromBranch(thirdBranch); + fourthBranchCommits = repo.GetCommitsReachableFromBranch(fourthBranch); secondBranchCommits.Should().Contain(c => c.Sha == tipOfSourceAfterNewCommit.Sha, "Second branch should contain latest commit from source after subsequent rebase"); thirdBranchCommits.Should().Contain(c => c.Sha == tipOfSourceAfterNewCommit.Sha, "Third branch should contain latest commit from source after subsequent rebase"); + fourthBranchCommits.Should().Contain(c => c.Sha == tipOfSourceAfterNewCommit.Sha, "Fourth branch should contain latest commit from source after subsequent rebase"); } [Fact] @@ -477,12 +487,14 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var firstBranch = Some.BranchName(); var secondBranch = Some.BranchName(); var thirdBranch = Some.BranchName(); + var fourthBranch = Some.BranchName(); using var repo = new TestGitRepositoryBuilder() .WithBranch(builder => builder.WithName(sourceBranch).PushToRemote().WithNumberOfEmptyCommits(1)) .WithBranch(builder => builder.WithName(firstBranch).FromSourceBranch(sourceBranch).WithNumberOfEmptyCommits(3).PushToRemote()) .WithBranch(builder => builder.WithName(secondBranch).FromSourceBranch(firstBranch).WithNumberOfEmptyCommits(1).PushToRemote()) .WithBranch(builder => builder.WithName(thirdBranch).FromSourceBranch(secondBranch).WithNumberOfEmptyCommits(1).PushToRemote()) + .WithBranch(builder => builder.WithName(fourthBranch).FromSourceBranch(secondBranch).WithNumberOfEmptyCommits(1).PushToRemote()) .Build(); var logger = XUnitLogger.CreateLogger(testOutputHelper); @@ -508,7 +520,10 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer var stack = new TestStackBuilder() .WithSourceBranch(sourceBranch) - .WithBranch(b => b.WithName(firstBranch).WithChildBranch(c => c.WithName(secondBranch).WithChildBranch(d => d.WithName(thirdBranch)))) + .WithBranch(b => b.WithName(firstBranch) + .WithChildBranch(c => c.WithName(secondBranch) + .WithChildBranch(d => d.WithName(thirdBranch)) + .WithChildBranch(e => e.WithName(fourthBranch)))) .Build(); var stackActions = new StackActions(gitClientFactory, cliExecutionContext, gitHubClient, logger, displayProvider, conflictResolutionDetector); @@ -519,11 +534,14 @@ public async Task UpdateStack_WhenUpdatingUsingRebase_AndFirstBranchWasSquashMer // Assert var secondBranchCommits = repo.GetCommitsReachableFromBranch(secondBranch); var thirdBranchCommits = repo.GetCommitsReachableFromBranch(thirdBranch); + var fourthBranchCommits = repo.GetCommitsReachableFromBranch(fourthBranch); secondBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Second branch should contain squash commit from source"); thirdBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Third branch should contain squash commit from source"); + fourthBranchCommits.Should().Contain(c => c.Sha == squashCommit.Sha, "Fourth branch should contain squash commit from source"); secondBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Second branch should not contain tip commit from first branch"); thirdBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Third branch should not contain tip commit from first branch"); + fourthBranchCommits.Should().NotContain(c => c.Sha == tipOfFirstBranch.Sha, "Fourth branch should not contain tip commit from first branch"); } [Fact] From 71d4027b2fe288aeaedd9d32a25a91e681137455 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Thu, 25 Sep 2025 08:23:41 +1000 Subject: [PATCH 10/11] Re-write comment --- src/Stack/Commands/Helpers/StackActions.cs | 67 ++++++++++++++-------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/src/Stack/Commands/Helpers/StackActions.cs b/src/Stack/Commands/Helpers/StackActions.cs index 8de082bd..b3a9994f 100644 --- a/src/Stack/Commands/Helpers/StackActions.cs +++ b/src/Stack/Commands/Helpers/StackActions.cs @@ -258,34 +258,55 @@ private async Task UpdateStackUsingRebase( private async Task UpdateBranchLineUsingRebase(StackStatus status, List branchLine, Dictionary branchStatuses, CancellationToken cancellationToken) { // - // When rebasing the stack, we'll use `git rebase --update-refs` from the - // lowest branch in the stack to pick up changes throughout all branches in the stack. - // Because there could be changes in any branch in the stack that aren't in the ones - // below it, we'll repeat this all the way from the bottom to the top of the stack to - // ensure that all changes are applied in the correct order. + // When rebasing the stack, we need to be able to pick up changes at each level of the stack. + // + // We need to handle a few specific scenarios: + // - When one of the branches has been squash merged into the source branch. + // - When a branch has additional commits that weren't rebased into children before merging into the source branch. + // + // The approach we'll take it is to rebase each branch at each level of the stack against + // all active branches above it in the stack. + // + // # Squash merges + // + // Squash merges are tricky: + // - The branch that was squash merged will have one or more commits that are going to squashed into the source branch. + // - Child branches will also have these commits. + // - When the parent branch is squash merged, the child branches will still have the set of commits that + // are equal to the contents of the squashed commit on the source branch. + // - When we try and rebase the child branch we hit conflicts as we try and re-apply all + // the individual commits. If the commit happens to exactly match the final squashed commit it + // might work, but this is unlikely in practice, especially if the branch had multiple commits. + // + // We can detect if a branch has been squash merged into the source branch by: + // - Finding the common base between the branch we're rebasing and it's parent branch that was squash merged. + // - Finding the common base handles the case when additional commits were made to the branch before it was squash merged. + // - Checking if that common base exists in the source branch. If it doesn't, then we know that it was squash merged. + // + // If we find that a branch was squash merged, we rebase directly onto the source branch, telling Git to start + // from the common base to ignore commits up to that point: + // `git rebase --onto {sourceBranch} {commonBaseBetweenChildAndOldParentBranch}` + // + // # Example // - // For example if we have a stack like this: - // main -> feature1 -> feature2 -> feature3 + // With the following stack: // - // We'll rebase feature3 onto feature2, then feature3 onto feature1, and finally feature3 onto main. + // main + // |-feature1 (deleted in remote - squash merged into main) + // |- feature2 + // |- feature3 + // |- feature4 + // |- feature5 // - // In addition to this, if the stack is in a state where one of the branches has been squash merged - // into the source branch, we'll want to rebase onto that branch directly using - // `git rebase --onto {sourceBranch} {oldParentBranch}` to ensure that the changes are - // applied correctly and to try and avoid merge conflicts during the rebase. + // We'll rebase in the following order: // - // For example if we have a stack like this: - // main - // -> feature1 (deleted in remote): Squash merged into main - // -> feature2 - // -> feature3 - // - // We'll rebase feature3 onto feature2 using a normal `git rebase feature2 --update-refs`, - // then feature3 onto main using `git rebase --onto main feature1 --update-refs` to replay - // all commits from feature3 (and therefore from feature2) on top of the latest commits of main - // which will include the squashed commit. + // - feature2 onto main (re-parenting as feature1 was squash merged) + // - feature3 onto main (re-parenting as feature1 was squash merged) + // - feature3 onto feature2 + // - feature4 onto main (re-parenting as feature1 was squash merged) + // - feature4 onto feature2 + // - feature5 onto main (re-parenting as feature1 was squash merged) // - // TODO: Rewrite the above comment to be correct. logger.RebasingStackForBranchLine(status.Name, status.SourceBranch.Name, string.Join(" -> ", branchLine.Select(b => b.Name))); List allBranchesInLine = [status.SourceBranch, .. branchLine]; From 8410699b67d4b291aef06d674dc74af6bdcd2ce8 Mon Sep 17 00:00:00 2001 From: Geoff Lamrock Date: Thu, 25 Sep 2025 08:28:28 +1000 Subject: [PATCH 11/11] Update readme --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 36802432..60690443 100644 --- a/README.md +++ b/README.md @@ -136,9 +136,13 @@ To use the rebase strategy, either: To push changes to the remote after rebasing you'll need to use the `--force-with-lease` option. -**Rough edges** +**_Squash merges_** + +A common pattern when using pull requests is to Squash Merge the pull request when merging into the target branch, squashing all the commits in the PR branch into a single commit. This causes issues when rebasing the rest of the child branches in the stack. + +Stack has handling to detect when a squash merge happens during updating a stack using rebase as the update strategy. It will skip the commits that were squash merged, avoiding conflicts. -If you merge a pull request using "Squash and merge" then you might find that the first update to a stack after that results in merge conflicts that you need to resolve. This can be a bit of a pain, however for each commit that existed on the branch that was merged if you select to take the new single commit that now exists generally it isn't too bad. +The remote tracking branch for the branch that was squash merged needs to be deleted for this handling to be enabled. ### Creating pull requests