Repository: remotemobprogramming/mob
Pull request #278 (2022-06-13 22:43:48)

Test behaviour on pre existing commits
Found and addressed a bug where when one of the mobbers has had an unpushed commit thus other mobbers were behind on the base branch. They ran into a problem when they would do `mob done --squash-wip`

simonharrer: Which of the new tests would fail with the old behavior?
simonharrer: Fine for me. Can you add a changelog, please?
gregorriegler: The more I think about it the more I dislike it.
The wip branch shouldn't diverge from the main branch ever. ( I cannot outrule a conflict )
When it diverges, mobbers should be warned and guided to resolve it instead.
gregorriegler: @simonharrer what should happen in your opinion if the base branch diverges from the mob branch?
simonharrer: With squash, it’s not that large of an issue as you normally don’t have conflicts - at least in my experience. And if you do, you have to fix them manually with the staged changes until you make the final commit.

With squash-wip it should behave the same.

With no-squash, it’s more of a problem as it creates a merge commit. But also ok. 
simonharrer: We could add an info to the user that the mob branch diverged from when it was created.
gregorriegler: I prefer that. If users get the info that it diverges, they at least have the choice to fix it on the wip-branch rather than on the main-branch

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c579030..8545147 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,5 @@
 # 3.1.5
+- Fix: `mob done --squash-wip` now successfully auto-merges auto-mergeable diverging changes.
 - Print the help output whenever any kind of help argument (`help`, `--help`, `-h`) is present in the command, e.g. `mob s 10 -h`.
 
 # 3.1.4
diff --git a/mob.go b/mob.go
index 6383d27..4eba383 100644
--- a/mob.go
+++ b/mob.go
@@ -1439,7 +1439,7 @@ func done(configuration Configuration) {
 
 		git("checkout", baseBranch.Name)
 		git("merge", baseBranch.remote(configuration).Name, "--ff-only")
-		mergeFailed := gitignorefailure("merge", squashOrNoCommit(configuration), "--ff", wipBranch.Name)
+		mergeFailed := gitignorefailure("merge", squashOrCommit(configuration), "--ff", wipBranch.Name)
 
 		if mergeFailed != nil {
 			// TODO should this be an error and a fix for that error?
@@ -1487,11 +1487,11 @@ func gitRootDir() string {
 	return strings.TrimSuffix(gitDir(), "/.git")
 }
 
-func squashOrNoCommit(configuration Configuration) string {
+func squashOrCommit(configuration Configuration) string {
 	if configuration.DoneSquash == Squash {
 		return "--squash"
 	} else {
-		return "--no-commit"
+		return "--commit"
 	}
 }
 
diff --git a/squash_wip.go b/squash_wip.go
index 87bac80..792d07b 100644
--- a/squash_wip.go
+++ b/squash_wip.go
@@ -12,7 +12,7 @@ type Replacer func(string) string
 
 func squashWip(configuration Configuration) {
 	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration)
-	mergeBase := silentgit("merge-base", currentWipBranch.String(), currentBaseBranch.String())
+	mergeBase := silentgit("merge-base", currentWipBranch.String(), currentBaseBranch.remote(configuration).String())
 
 	originalGitEditor, originalGitSequenceEditor := getEnvGitEditor()
 	setEnvGitEditor(
@@ -23,7 +23,7 @@ func squashWip(configuration Configuration) {
 	git("rebase", "--interactive", "--keep-empty", mergeBase)
 	setEnvGitEditor(originalGitEditor, originalGitSequenceEditor)
 	sayInfo("resulting history is:")
-	sayLastCommitsWithMessage(currentBaseBranch.String(), currentWipBranch.String())
+	sayLastCommitsWithMessage(currentBaseBranch.remote(configuration).String(), currentWipBranch.String())
 	if lastCommitIsWipCommit(configuration) { // last commit is wip commit
 		sayInfo("undoing the final wip commit and staging its changes:")
 		git("reset", "--soft", "HEAD^")


Test patch:
diff --git a/mob_test.go b/mob_test.go
index 46e1722..45f4ef3 100644
--- a/mob_test.go
+++ b/mob_test.go
@@ -866,19 +866,6 @@ func TestStartNextStay_OpenLastModifiedFile(t *testing.T) {
 	})
 }
 
-func TestStartDoneWithMobDoneSquash(t *testing.T) {
-	_, configuration := setup(t)
-	configuration.DoneSquash = Squash
-
-	start(configuration)
-	assertOnBranch(t, "mob-session")
-
-	done(configuration)
-
-	assertOnBranch(t, "master")
-	assertNoMobSessionBranches(t, configuration, "mob-session")
-}
-
 func TestRunOutput(t *testing.T) {
 	_, configuration := setup(t)
 
@@ -922,6 +909,64 @@ func TestTestbed(t *testing.T) {
 	assertOutputContains(t, &output, "bob")
 }
 
+func TestStartDoneWithMobDoneSquash(t *testing.T) {
+	_, configuration := setup(t)
+	configuration.DoneSquash = Squash
+
+	start(configuration)
+	assertOnBranch(t, "mob-session")
+
+	done(configuration)
+
+	assertOnBranch(t, "master")
+	assertNoMobSessionBranches(t, configuration, "mob-session")
+}
+
+func TestStartDoneSquashWithUnpushedCommit(t *testing.T) {
+	_, configuration := setup(t)
+	configuration.DoneSquash = Squash
+
+	// now in /local
+	createFileAndCommitIt(t, "file1.txt", "owqe", "not a mob session yet")
+
+	setWorkingDir(tempDir + "/alice")
+	start(configuration)
+	createFile(t, "file2.txt", "zcvx")
+	next(configuration)
+
+	setWorkingDir(tempDir + "/local")
+	git("push")
+
+	setWorkingDir(tempDir + "/alice")
+	start(configuration)
+	done(configuration)
+
+	assertFileExist(t, "file1.txt")
+}
+
+func TestStartDoneSquashWipWithUnpushedCommit(t *testing.T) {
+	Debug = true
+	_, configuration := setup(t)
+	configuration.DoneSquash = SquashWip
+
+	// now in /local
+	createFileAndCommitIt(t, "file1.txt", "owqe", "not a mob session yet")
+
+	setWorkingDir(tempDir + "/alice")
+	start(configuration)
+	createFile(t, "file2.txt", "zcvx")
+	next(configuration)
+
+	setWorkingDir(tempDir + "/local")
+	git("push")
+
+	setWorkingDir(tempDir + "/alice")
+	start(configuration)
+	done(configuration)
+
+	assertFileExist(t, "file1.txt")
+}
+
 func TestStartDoneWithMobDoneNoSquash(t *testing.T) {
 	_, configuration := setup(t)
 	configuration.DoneSquash = NoSquash