Repository: remotemobprogramming/mob
Pull request #354 (2023-01-11 23:59:26)

mob start with ci skip option
## Summary

Implement CI skipping function when `mob start`. 
Check out #353 for details.

## Implementation (editted)

- Skip CI when `mob start`
  - CI is skipped by committing empty with message `mob start [ci-skip] [ci skip] [skip ci]`.
- Remove `--push-option ci.skip`
  - It may by effective only for GitLab at the moment.
  - This requires us to fix test: 5810d58
- Drop the initial empty commit when `mob done --squash-wip`
  - It is removed by `markStartCommitForDropping()`.
  - It is squashed when `mob done` or `mob done --squash`.
  - It is not removed when `mob done --no-squash`.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 768d4ba..cf7c13e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,6 @@
+# 4.2.0
+- Feature: mob.sh now starts a mob session with an empty commit to skip CI when creating a new remote branch for the session. The commit is squashed or dropped when `mob done` except for `--no-squash` option.
+
 # 4.1.2
 - Fix: `mob done --squash-wip` won't lose changes when you forget to `mob start`
 
diff --git a/configuration/configuration.go b/configuration/configuration.go
index 8c36da2..c56eef0 100644
--- a/configuration/configuration.go
+++ b/configuration/configuration.go
@@ -14,6 +14,8 @@ const (
 	Squash    = "squash"
 	NoSquash  = "no-squash"
 	SquashWip = "squash-wip"
+
+	StartCISkipCommitMessage = "mob start [ci-skip] [ci skip] [skip ci]"
 )
 
 type Configuration struct {
diff --git a/mob.go b/mob.go
index cfc24e1..cc96c96 100644
--- a/mob.go
+++ b/mob.go
@@ -26,7 +26,7 @@ import (
 )
 
 const (
-	versionNumber     = "4.1.2"
+	versionNumber     = "4.2.0"
 	minimumGitVersion = "2.13.0"
 )
 
@@ -969,18 +969,12 @@ func startNewMobSession(configuration config.Configuration) {
 
 	say.Info("starting new session from " + currentBaseBranch.remote(configuration).String())
 	git("checkout", "-B", currentWipBranch.Name, currentBaseBranch.remote(configuration).Name)
+	git("commit", "--allow-empty", "-m", config.StartCISkipCommitMessage)
 	gitPush(gitHooksOption(configuration), "--set-upstream", configuration.RemoteName, currentWipBranch.Name)
 }
 
 func gitPush(args ...string) {
-	err := gitIgnoreFailure(pushArgsWithCiSkip(args)...)
-	if err != nil {
-		gitWithoutEmptyStrings(pushArgs(args)...)
-	}
-}
-
-func pushArgsWithCiSkip(args []string) []string {
-	return append([]string{"push", "--push-option", "ci.skip"}, deleteEmptyStrings(args)...)
+	gitWithoutEmptyStrings(pushArgs(args)...)
 }
 
 func pushArgs(args []string) []string {
diff --git a/squash_wip.go b/squash_wip.go
index fa20d38..47d4726 100644
--- a/squash_wip.go
+++ b/squash_wip.go
@@ -97,7 +97,8 @@ func squashWipGitEditor(fileName string, configuration config.Configuration) {
 // used for non-interactive rebase to squash post-wip-commits
 func squashWipGitSequenceEditor(fileName string, configuration config.Configuration) {
 	replaceFileContents(fileName, func(input string) string {
-		return markPostWipCommitsForSquashing(input, configuration)
+		result := markPostWipCommitsForSquashing(input, configuration)
+		return markStartCommitForDropping(result, configuration)
 	})
 }
 
@@ -152,6 +153,19 @@ func markPostWipCommitsForSquashing(input string, configuration config.Configura
 	return strings.Join(result, "\n")
 }
 
+func markStartCommitForDropping(input string, configuration config.Configuration) string {
+	inputLines := strings.Split(input, "\n")
+	result := inputLines
+
+	firstLine := inputLines[0]
+	if isStartCISkipCommitLine(firstLine) {
+		markedLine := markDrop(firstLine)
+		result[0] = markedLine
+	}
+
+	return strings.Join(result, "\n")
+}
+
 func markLine(inputLines []string, i int, configuration config.Configuration) string {
 	var resultLine = inputLines[i]
 	previousLine := previousLine(inputLines, i)
@@ -193,18 +207,30 @@ func markFixup(line string) string {
 	return strings.Replace(line, "pick ", "fixup ", 1)
 }
 
+func markDrop(line string) string {
+	return strings.Replace(line, "pick ", "drop ", 1)
+}
+
 func isWipCommitLine(line string, configuration config.Configuration) bool {
 	return isPick(line) && isWipCommit(line, configuration)
 }
 
+func isStartCISkipCommitLine(line string) bool {
+	return isPick(line) && isStartCISkipCommit(line)
+}
+
 func isManualCommit(line string, configuration config.Configuration) bool {
-	return !isWipCommit(line, configuration)
+	return !isWipCommit(line, configuration) && !isStartCISkipCommit(line)
 }
 
 func isWipCommit(line string, configuration config.Configuration) bool {
 	return strings.Contains(line, configuration.WipCommitMessage)
 }
 
+func isStartCISkipCommit(line string) bool {
+	return strings.Contains(line, config.StartCISkipCommitMessage)
+}
+
 func isPick(line string) bool {
 	return strings.HasPrefix(line, "pick ")
 }


Test patch:
diff --git a/mob_test.go b/mob_test.go
index 4ac5c88..d894ac5 100644
--- a/mob_test.go
+++ b/mob_test.go
@@ -21,9 +21,6 @@ var (
 )
 
 type GitStatus = map[string]string
-type TestBedOptions struct {
-	enablePushOptions bool
-}
 
 func TestCurrentCliName(t *testing.T) {
 	equals(t, "mob", currentCliName("mob"))
@@ -183,7 +180,8 @@ func TestStatusWithMoreThan5LinesOfLog(t *testing.T) {
 	}
 
 	status(configuration)
-	assertOutputContains(t, output, "wip branch 'mob-session' contains 6 commits.")
+	// 6 wip commits + 1 start commit
+	assertOutputContains(t, output, "wip branch 'mob-session' contains 7 commits.")
 }
 
 func TestExecuteKicksOffStatus(t *testing.T) {
@@ -221,6 +219,16 @@ func TestStart(t *testing.T) {
 	assertMobSessionBranches(t, configuration, "mob-session")
 }
 
+func TestStartWithCISkip(t *testing.T) {
+	_, configuration := setup(t)
+
+	start(configuration)
+
+	assertOnBranch(t, "mob-session")
+	assertMobSessionBranches(t, configuration, "mob-session")
+	assertCommitLogContainsMessage(t, "mob-session", config.StartCISkipCommitMessage)
+}
+
 func TestStartWithMultipleExistingBranches(t *testing.T) {
 	output, configuration := setup(t)
 
@@ -708,16 +716,14 @@ func TestStartPushOnWIPBranchWithOptions(t *testing.T) {
 
 	start(configuration)
 
-	assertOutputNotContains(t, output, "git push --no-verify --set-upstream origin mob-session")
-	assertOutputContains(t, output, "git push --push-option ci.skip --no-verify --set-upstream origin mob-session")
+	assertOutputContains(t, output, "git push --no-verify --set-upstream origin mob-session")
 }
 
 func TestStartPushOnWIPBranchWithOptionsShouldFailAndRetry(t *testing.T) {
-	output, configuration := setupWithOptions(t, TestBedOptions{enablePushOptions: false})
+	output, configuration := setup(t)
 
 	start(configuration)
 
-	assertOutputContains(t, output, "git push --push-option ci.skip --no-verify --set-upstream origin mob-session")
 	assertOutputContains(t, output, "git push --no-verify --set-upstream origin mob-session")
 	assertOutputContains(t, output, "you are on wip branch 'mob-session' (base branch 'master')")
 }
@@ -975,16 +981,17 @@ func TestStartDonePublishingOneManualCommit(t *testing.T) {
 
 	start(configuration)
 	assertOnBranch(t, "mob-session")
-	// should be 1 commit on mob-session so far
+	// should be 2 commits on mob-session so far
+	// 1 commit from setup() and 1 commit from ci-skip commit when mob start
 
 	createFileAndCommitIt(t, "example.txt", "contentIrrelevant", "[manual-commit-1] publish this commit to master")
-	assertCommits(t, 2)
+	assertCommits(t, 3)
 
 	done(configuration) // without squash (configuration)
 
 	assertOnBranch(t, "master")
 	assertCleanGitStatus(t)
-	assertCommitsOnBranch(t, 2, "master")
+	assertCommitsOnBranch(t, 3, "master")
 	assertCommitLogContainsMessage(t, "master", "[manual-commit-1] publish this commit to master")
 	assertCommitsOnBranch(t, 1, "origin/master")
 	assertNoMobSessionBranches(t, configuration, "mob-session")
@@ -996,10 +1003,11 @@ func TestStartDoneSquashTheOneManualCommit(t *testing.T) {
 
 	start(configuration)
 	assertOnBranch(t, "mob-session")
-	// should be 1 commit on mob-session so far
+	// should be 2 commits on mob-session so far
+	// 1 commit from setup() and 1 commit from ci-skip commit when mob start
 
 	createFileAndCommitIt(t, "example.txt", "contentIrrelevant", "[manual-commit-1] publish this commit to master")
-	assertCommits(t, 2)
+	assertCommits(t, 3)
 
 	done(configuration)
 
@@ -1033,7 +1041,9 @@ func TestStartDoneNoSquashWithUncommittedChanges(t *testing.T) {
 	_, configuration := setup(t)
 	configuration.DoneSquash = config.NoSquash
 
-	start(configuration) // should be 1 commit on mob-session so far
+	start(configuration)
+	// should be 2 commits on mob-session so far
+	// 1 commit from setup() and 1 commit from ci-skip commit when mob start
 	createFile(t, "example.txt", "content")
 
 	done(configuration) // without squash (configuration)
@@ -1042,7 +1052,7 @@ func TestStartDoneNoSquashWithUncommittedChanges(t *testing.T) {
 	assertGitStatus(t, GitStatus{
 		"example.txt": "A",
 	})
-	assertCommitsOnBranch(t, 1, "master")
+	assertCommitsOnBranch(t, 2, "master")
 	assertCommitsOnBranch(t, 1, "origin/master")
 	assertNoMobSessionBranches(t, configuration, "mob-session")
 }
@@ -1306,11 +1316,13 @@ func TestNothingToCommitCreatesNoCommits(t *testing.T) {
 
 	setWorkingDir(tempDir + "/local")
 	start(configuration)
-	assertCommits(t, 1)
+	// should be 2 commits on mob-session so far
+	// 1 commit from setup() and 1 commit from ci-skip commit when mob start
+	assertCommits(t, 2)
 
 	setWorkingDir(tempDir + "/localother")
 	start(configuration)
-	assertCommits(t, 1)
+	assertCommits(t, 2)
 
 	setWorkingDir(tempDir + "/local")
 	next(configuration)
@@ -1320,11 +1332,11 @@ func TestNothingToCommitCreatesNoCommits(t *testing.T) {
 
 	setWorkingDir(tempDir + "/local")
 	start(configuration)
-	assertCommits(t, 1)
+	assertCommits(t, 2)
 
 	setWorkingDir(tempDir + "/localother")
 	start(configuration)
-	assertCommits(t, 1)
+	assertCommits(t, 2)
 }
 
 func TestStartNextPushManualCommits(t *testing.T) {
@@ -1457,6 +1469,32 @@ func TestDoneSquashNoChanges(t *testing.T) {
 	assertOutputContains(t, output, "nothing to commit")
 }
 
+func TestDoneSquashWipStartCommit(t *testing.T) {
+	_, configuration := setup(t)
+	configuration.NextStay = true
+	configuration.DoneSquash = config.SquashWip
+
+	start(configuration)
+	createFile(t, "file1.txt", "contentIrrelevant")
+	next(configuration)
+	assertCommitsOnBranch(t, 3, "mob-session")
+	done(configuration)
+	assertCommitsOnBranch(t, 1, "master")
+}
+
+func TestDoneNoSquashStartCommit(t *testing.T) {
+	_, configuration := setup(t)
+	configuration.NextStay = true
+	configuration.DoneSquash = config.NoSquash
+
+	start(configuration)
+	createFile(t, "file1.txt", "contentIrrelevant")
+	next(configuration)
+	assertCommitsOnBranch(t, 3, "mob-session")
+	done(configuration)
+	assertCommitsOnBranch(t, 3, "master")
+}
+
 func TestStartAndNextInSubdir(t *testing.T) {
 	_, configuration := setup(t)
 
@@ -1650,11 +1688,11 @@ func gitStatus() GitStatus {
 	return statusMap
 }
 
-func setupWithOptions(t *testing.T, options TestBedOptions) (output *string, configuration config.Configuration) {
+func setup(t *testing.T) (output *string, configuration config.Configuration) {
 	configuration = config.GetDefaultConfiguration()
 	configuration.NextStay = false
 	output = captureOutput(t)
-	createTestbed(t, configuration, options)
+	createTestbed(t, configuration)
 	assertOnBranch(t, "master")
 	equals(t, []string{"master"}, gitBranches())
 	equals(t, []string{"origin/master"}, gitRemoteBranches())
@@ -1663,12 +1701,6 @@ func setupWithOptions(t *testing.T, options TestBedOptions) (output *string, con
 	return output, configuration
 }
 
-func setup(t *testing.T) (output *string, configuration config.Configuration) {
-	return setupWithOptions(t, TestBedOptions{
-		enablePushOptions: true,
-	})
-}
-
 func captureOutput(t *testing.T) *string {
 	messages := ""
 	say.PrintToConsole = func(text string) {
@@ -1678,20 +1710,20 @@ func captureOutput(t *testing.T) *string {
 	return &messages
 }
 
-func createTestbed(t *testing.T, configuration config.Configuration, options TestBedOptions) {
+func createTestbed(t *testing.T, configuration config.Configuration) {
 	workingDir = ""
 
 	tempDir = t.TempDir()
 
 	say.Say("Creating testbed in temporary directory " + tempDir)
-	createTestbedIn(t, tempDir, options)
+	createTestbedIn(t, tempDir)
 
 	setWorkingDir(tempDir + "/local")
 	assertOnBranch(t, "master")
 	assertNoMobSessionBranches(t, configuration, "mob-session")
 }
 
-func createTestbedIn(t *testing.T, temporaryDirectory string, options TestBedOptions) {
+func createTestbedIn(t *testing.T, temporaryDirectory string) {
 	say.Debug("Creating temporary test assets in " + temporaryDirectory)
 	err := os.MkdirAll(temporaryDirectory, 0755)
 	if err != nil {
@@ -1702,7 +1734,7 @@ func createTestbedIn(t *testing.T, temporaryDirectory string, options TestBedOpt
 	say.Debug("Create remote repository")
 	remoteDirectory := getRemoteDirectory(temporaryDirectory)
 	cleanRepository(remoteDirectory)
-	createRemoteRepository(remoteDirectory, options)
+	createRemoteRepository(remoteDirectory)
 
 	say.Debug("Create first local repository")
 	localDirectory := getLocalDirectory(temporaryDirectory)
@@ -1922,7 +1954,7 @@ func cleanRepository(path string) {
 	}
 }
 
-func createRemoteRepository(path string, options TestBedOptions) {
+func createRemoteRepository(path string) {
 	branch := "master" // fixed to master for now
 	say.Debug("createremoterepository: Creating remote repository " + path)
 	err := os.MkdirAll(path, 0755)
@@ -1936,7 +1968,6 @@ func createRemoteRepository(path string, options TestBedOptions) {
 	git("--bare", "init")
 	say.Debug("before symbolic-ref")
 	git("symbolic-ref", "HEAD", "refs/heads/"+branch)
-	git("config", "receive.advertisePushOptions", strconv.FormatBool(options.enablePushOptions))
 	say.Debug("finished")
 }
 
diff --git a/squash_wip_test.go b/squash_wip_test.go
index 8d632d6..fce1df8 100644
--- a/squash_wip_test.go
+++ b/squash_wip_test.go
@@ -136,6 +136,42 @@ func TestSquashWipCommits_worksWithEmptyCommits(t *testing.T) {
 	}, commitsOnCurrentBranch(configuration))
 }
 
+func TestSquashWipCommits_acceptanceWithDroppingStartCommit(t *testing.T) {
+	_, configuration := setup(t)
+	wipCommit(t, configuration, "file1.txt")
+	manualCommit(t, configuration, "file2.txt", "first manual commit")
+
+	// manual commit followed by a wip commit
+	start(configuration)
+	createFileAndCommitIt(t, "file3.txt", "contentIrrelevant", "second manual commit")
+	createFile(t, "file4.txt", "contentIrrelevant")
+	next(configuration)
+
+	// final manual commit
+	start(configuration)
+	createFileAndCommitIt(t, "file5.txt", "contentIrrelevant", "third manual commit")
+
+	// Check if the initial commit for ci skip exists
+	equals(t, []string{
+		"third manual commit",
+		configuration.WipCommitMessage,
+		"second manual commit",
+		"first manual commit",
+		configuration.WipCommitMessage,
+		config.StartCISkipCommitMessage,
+	}, commitsOnCurrentBranch(configuration))
+
+	squashWip(configuration)
+
+	assertOnBranch(t, "mob-session")
+	equals(t, []string{
+		"third manual commit",
+		"second manual commit",
+		"first manual commit",
+	}, commitsOnCurrentBranch(configuration))
+	equals(t, commitsOnCurrentBranch(configuration), commitsOnRemoteBranch(configuration))
+}
+
 func TestCommitsOnCurrentBranch(t *testing.T) {
 	_, configuration := setup(t)
 	createFileAndCommitIt(t, "file1.txt", "contentIrrelevant", "not on branch")
@@ -151,6 +187,7 @@ func TestCommitsOnCurrentBranch(t *testing.T) {
 	equals(t, []string{
 		configuration.WipCommitMessage,
 		"on branch",
+		config.StartCISkipCommitMessage,
 	}, commits)
 }
 
@@ -269,6 +306,49 @@ fixup 01a9a33 %[1]s
 	equals(t, expected, result)
 }
 
+func TestMarkDropStartCommit_hasStartCISkipCommitLine(t *testing.T) {
+	configuration := config.GetDefaultConfiguration()
+
+	input := fmt.Sprintf(`pick 01a9a31 %[2]s
+pick c51a56d manual commit
+pick 01a9a32 %[1]s
+pick 01a9a33 %[1]s
+
+# Rebase ...`, configuration.WipCommitMessage, config.StartCISkipCommitMessage)
+	expected := fmt.Sprintf(`drop 01a9a31 %[2]s
+pick c51a56d manual commit
+pick 01a9a32 %[1]s
+pick 01a9a33 %[1]s
+
+# Rebase ...`, configuration.WipCommitMessage, config.StartCISkipCommitMessage)
+
+	result := markStartCommitForDropping(input, configuration)
+
+	equals(t, expected, result)
+}
+
+// Check if the initial commit is not dropped when the commmit line does not contain `InitialCISkipCommitMessage`
+func TestMarkDropStartCommit_notHasStartCISkipCommitLine(t *testing.T) {
+	configuration := config.GetDefaultConfiguration()
+
+	input := fmt.Sprintf(`pick 01a9a31 %[1]s
+pick c51a56d manual commit
+pick 01a9a32 %[1]s
+pick 01a9a33 %[1]s
+
+# Rebase ...`, configuration.WipCommitMessage)
+	expected := fmt.Sprintf(`pick 01a9a31 %[1]s
+pick c51a56d manual commit
+pick 01a9a32 %[1]s
+pick 01a9a33 %[1]s
+
+# Rebase ...`, configuration.WipCommitMessage)
+
+	result := markStartCommitForDropping(input, configuration)
+
+	equals(t, expected, result)
+}
+
 func TestCommentWipCommits_oneWipAndOneManualCommit(t *testing.T) {
 	configuration := config.GetDefaultConfiguration()
 	input := fmt.Sprintf(`# This is a combination of 2 commits.
@@ -298,9 +378,8 @@ manual commit
 }
 
 func TestSquashWipCommitGitEditor(t *testing.T) {
-	testOptions := TestBedOptions{enablePushOptions: true}
 	configuration := config.GetDefaultConfiguration()
-	createTestbed(t, configuration, testOptions)
+	createTestbed(t, configuration)
 	input := createFile(t, "commits", fmt.Sprintf(
 		`# This is a combination of 2 commits.
 # This is the 1st commit message:
@@ -332,7 +411,7 @@ new file
 
 func TestSquashWipCommitGitSequenceEditor(t *testing.T) {
 	configuration := config.GetDefaultConfiguration()
-	createTestbed(t, configuration, TestBedOptions{enablePushOptions: true})
+	createTestbed(t, configuration)
 	input := createFile(t, "rebase", fmt.Sprintf(
 		`pick 01a9a31 %[1]s
 pick 01a9a32 %[1]s