Merge pull request cli#3036 from cli/pr-merge-no-commits

mislav · web-flow · commit 823514022de9 · 2021-02-26T13:08:27.000+01:00
Avoid crash in `pr merge` when the pull request has no commits
diff --git a/git/fixtures/.gitignore b/git/fixtures/.gitignore
@@ -0,0 +1 @@
+*.git/COMMIT_EDITMSG
diff --git a/git/fixtures/simple.git/HEAD b/git/fixtures/simple.git/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/main
diff --git a/git/fixtures/simple.git/config b/git/fixtures/simple.git/config
@@ -0,0 +1,9 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	;bare = true
+	ignorecase = true
+	precomposeunicode = true
+[user]
+	name = Mona the Cat
+	email = monalisa@github.com
diff --git a/git/fixtures/simple.git/index b/git/fixtures/simple.git/index
diff --git a/git/fixtures/simple.git/logs/HEAD b/git/fixtures/simple.git/logs/HEAD
@@ -0,0 +1,2 @@
+0000000000000000000000000000000000000000 d1e0abfb7d158ed544a202a6958c62d4fc22e12f Mona the Cat <monalisa@github.com> 1614174263 +0100	commit (initial): Initial commit
+d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat <monalisa@github.com> 1614174275 +0100	commit: Second commit
diff --git a/git/fixtures/simple.git/logs/refs/heads/main b/git/fixtures/simple.git/logs/refs/heads/main
@@ -0,0 +1,2 @@
+0000000000000000000000000000000000000000 d1e0abfb7d158ed544a202a6958c62d4fc22e12f Mona the Cat <monalisa@github.com> 1614174263 +0100	commit (initial): Initial commit
+d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat <monalisa@github.com> 1614174275 +0100	commit: Second commit
diff --git a/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 b/git/fixtures/simple.git/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904
diff --git a/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659 b/git/fixtures/simple.git/objects/6f/1a2405cace1633d89a79c74c65f22fe78f9659
diff --git a/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f b/git/fixtures/simple.git/objects/d1/e0abfb7d158ed544a202a6958c62d4fc22e12f
@@ -0,0 +1,3 @@
+x�NK
+�0t�S�� ILc
+"�+�@���M�뢷7"^@���b�Z�xF��h���b轴;�l��K����r�3<��3��3�Kc#-��"�k8�Z.��2�d�=�^*)ES�&�iq��ɏ��i��ϋP�j��A�y��3*H/
diff --git a/git/fixtures/simple.git/refs/heads/main b/git/fixtures/simple.git/refs/heads/main
@@ -0,0 +1 @@
+6f1a2405cace1633d89a79c74c65f22fe78f9659
diff --git a/git/git.go b/git/git.go
@@ -180,43 +180,30 @@ func Commits(baseRef, headRef string) ([]*Commit, error) {
 	return commits, nil
 }
 
-func LastCommit() (*Commit, error) {
-	logCmd, err := GitCommand("-c", "log.ShowSignature=false", "log", "--pretty=format:%H,%s", "-1")
+func lookupCommit(sha, format string) ([]byte, error) {
+	logCmd, err := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:"+format, sha)
 	if err != nil {
 		return nil, err
 	}
+	return run.PrepareCmd(logCmd).Output()
+}
 
-	output, err := run.PrepareCmd(logCmd).Output()
+func LastCommit() (*Commit, error) {
+	output, err := lookupCommit("HEAD", "%H,%s")
 	if err != nil {
 		return nil, err
 	}
 
-	lines := outputLines(output)
-	if len(lines) != 1 {
-		return nil, ErrNotOnAnyBranch
-	}
-
-	split := strings.SplitN(lines[0], ",", 2)
-	if len(split) != 2 {
-		return nil, ErrNotOnAnyBranch
-	}
-
+	idx := bytes.IndexByte(output, ',')
 	return &Commit{
-		Sha:   split[0],
-		Title: split[1],
+		Sha:   string(output[0:idx]),
+		Title: strings.TrimSpace(string(output[idx+1:])),
 	}, nil
 }
 
 func CommitBody(sha string) (string, error) {
-	showCmd, err := GitCommand("-c", "log.ShowSignature=false", "show", "-s", "--pretty=format:%b", sha)
-	if err != nil {
-		return "", err
-	}
-	output, err := run.PrepareCmd(showCmd).Output()
-	if err != nil {
-		return "", err
-	}
-	return string(output), nil
+	output, err := lookupCommit(sha, "%b")
+	return string(output), err
 }
 
 // Push publishes a git ref to a remote and sets up upstream configuration
diff --git a/git/git_test.go b/git/git_test.go
@@ -1,12 +1,53 @@
 package git
 
 import (
+	"os"
 	"reflect"
 	"testing"
 
 	"github.com/cli/cli/internal/run"
 )
 
+func setGitDir(t *testing.T, dir string) {
+	// TODO: also set XDG_CONFIG_HOME, GIT_CONFIG_NOSYSTEM
+	old_GIT_DIR := os.Getenv("GIT_DIR")
+	os.Setenv("GIT_DIR", dir)
+	t.Cleanup(func() {
+		os.Setenv("GIT_DIR", old_GIT_DIR)
+	})
+}
+
+func TestLastCommit(t *testing.T) {
+	setGitDir(t, "./fixtures/simple.git")
+	c, err := LastCommit()
+	if err != nil {
+		t.Fatalf("LastCommit error: %v", err)
+	}
+	if c.Sha != "6f1a2405cace1633d89a79c74c65f22fe78f9659" {
+		t.Errorf("expected sha %q, got %q", "6f1a2405cace1633d89a79c74c65f22fe78f9659", c.Sha)
+	}
+	if c.Title != "Second commit" {
+		t.Errorf("expected title %q, got %q", "Second commit", c.Title)
+	}
+}
+
+func TestCommitBody(t *testing.T) {
+	setGitDir(t, "./fixtures/simple.git")
+	body, err := CommitBody("6f1a2405cace1633d89a79c74c65f22fe78f9659")
+	if err != nil {
+		t.Fatalf("CommitBody error: %v", err)
+	}
+	if body != "I'm starting to get the hang of things\n" {
+		t.Errorf("expected %q, got %q", "I'm starting to get the hang of things\n", body)
+	}
+}
+
+/*
+	NOTE: below this are stubbed git tests, i.e. those that do not actually invoke `git`. If possible, utilize
+	`setGitDir()` to allow new tests to interact with `git`. For write operations, you can use `t.TempDir()` to
+	host a temporary git repository that is safe to be changed.
+*/
+
 func Test_UncommittedChangeCount(t *testing.T) {
 	type c struct {
 		Label    string
diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go
@@ -157,9 +157,8 @@ func mergeRun(opts *MergeOptions) error {
 		return nil
 	}
 
-	if opts.SelectorArg == "" {
-		localBranchLastCommit, err := git.LastCommit()
-		if err == nil {
+	if opts.SelectorArg == "" && len(pr.Commits.Nodes) > 0 {
+		if localBranchLastCommit, err := git.LastCommit(); err == nil {
 			if localBranchLastCommit.Sha != pr.Commits.Nodes[0].Commit.Oid {
 				fmt.Fprintf(opts.IO.ErrOut,
 					"%s Pull request #%d (%s) has diverged from local branch\n", cs.Yellow("!"), pr.Number, pr.Title)
diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+0000000000000000000000000000000000000000 d1e0abfb7d158ed544a202a6958c62d4fc22e12f Mona the Cat <monalisa@github.com> 1614174263 +0100 commit (initial): Initial commit`
	`2`	`+d1e0abfb7d158ed544a202a6958c62d4fc22e12f 6f1a2405cace1633d89a79c74c65f22fe78f9659 Mona the Cat <monalisa@github.com> 1614174275 +0100 commit: Second commit`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+x�NK`
	`2`	`+�0t�S�� ILc`
	`3`	`+"�+�@��M�뢷7"^@��b�Z�xF��h��b轴;�l��K��r�3<��3��3�Kc#-��"�k8�Z.��2�d�=�^)ES�&�iq��ɏ��i��ϋP�j��A�y��3H/`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+6f1a2405cace1633d89a79c74c65f22fe78f9659`