git-codereview: add test for pending
Fixes #9352
Change-Id: I9714c58ead93d8ec91a09e40a04b1a4e2f075c54
Reviewed-on: https://go-review.googlesource.com/1961
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index ad82a46..9c9d8e7 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -116,9 +116,9 @@
// We want to save the info about the *first* commit
// after the branch point, and the log is ordered
// starting at the most recent and working backward.
- b.commitHash = hash
- b.shortCommitHash = shortHash
- b.parentHash = parent
+ b.commitHash = strings.TrimSpace(hash)
+ b.shortCommitHash = strings.TrimSpace(shortHash)
+ b.parentHash = strings.TrimSpace(parent)
b.subject = subject
b.message = msg
for _, line := range strings.Split(msg, "\n") {
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index 96220d4..e4e2a96 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
-// TODO(rsc): Tests
-
package main
import (
@@ -42,7 +40,9 @@
if b.current {
b.staged, b.unstaged, b.untracked = LocalChanges()
}
- b.committed = getLines("git", "diff", "--name-only", b.parentHash, b.commitHash)
+ if b.parentHash != "" && b.commitHash != "" {
+ b.committed = getLines("git", "diff", "--name-only", b.parentHash, b.commitHash)
+ }
if !pendingLocal {
b.g, b.gerr = b.GerritChange("DETAILED_LABELS", "CURRENT_REVISION", "MESSAGES", "DETAILED_ACCOUNTS")
}
@@ -165,8 +165,7 @@
}
fmt.Fprintf(&buf, "\n")
if text := b.errors(); text != "" {
- // TODO(rsc): Test
- fmt.Fprintf(&buf, "\tERROR: %s", strings.Replace(text, "\n", "\n\t", -1))
+ fmt.Fprintf(&buf, "\tERROR: %s\n\n", strings.Replace(strings.TrimSpace(text), "\n", "\n\t", -1))
}
if b.message != "" {
msg := strings.TrimRight(b.message, "\r\n")
@@ -220,7 +219,11 @@
fmt.Fprintf(&buf, "\n")
}
- os.Stdout.Write(buf.Bytes())
+ if stdoutTrap != nil {
+ stdoutTrap.Write(buf.Bytes())
+ } else {
+ os.Stdout.Write(buf.Bytes())
+ }
}
// errors returns any errors that should be displayed
@@ -233,8 +236,8 @@
fmt.Fprintf(&buf, "\tDo not commit directly to %s branch.\n", b.Name)
} else if b.commitsAhead > 1 {
fmt.Fprintf(&buf, "Branch contains %d commits not on origin/%s.\n", b.commitsAhead, b.OriginBranch())
- fmt.Fprint(&buf, "\tThere should be at most one.\n")
- fmt.Fprint(&buf, "\tUse 'git change', not 'git commit'.\n")
+ fmt.Fprintf(&buf, "\tThere should be at most one.\n")
+ fmt.Fprintf(&buf, "\tUse 'git change', not 'git commit'.\n")
fmt.Fprintf(&buf, "\tRun 'git log %s..%s' to list commits.\n", b.OriginBranch(), b.Name)
}
return buf.String()
diff --git a/git-codereview/pending_test.go b/git-codereview/pending_test.go
new file mode 100644
index 0000000..b1030e2
--- /dev/null
+++ b/git-codereview/pending_test.go
@@ -0,0 +1,283 @@
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "io/ioutil"
+ "os"
+ "os/exec"
+ "regexp"
+ "strings"
+ "testing"
+)
+
+func TestPendingNone(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ testPending(t, `
+ master (current branch)
+
+ `)
+}
+
+func TestPendingNoneBranch(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ trun(t, gt.client, "git", "checkout", "-b", "work")
+
+ testPending(t, `
+ work (current branch)
+
+ `)
+}
+
+func TestPendingBasic(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+ gt.work(t)
+
+ testPending(t, `
+ work REVHASH (current branch)
+ msg
+
+ Change-Id: I123456789
+
+ Files in this change:
+ file
+
+ `)
+}
+
+func TestPendingComplex(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+ gt.work(t)
+
+ write(t, gt.server+"/file", "v2")
+ trun(t, gt.server, "git", "commit", "-a", "-m", "v2")
+
+ write(t, gt.server+"/file", "v3")
+ trun(t, gt.server, "git", "commit", "-a", "-m", "v3")
+
+ trun(t, gt.client, "git", "fetch")
+ trun(t, gt.client, "git", "checkout", "-b", "work3ignored", "-t", "origin/master")
+
+ write(t, gt.server+"/file", "v4")
+ trun(t, gt.server, "git", "commit", "-a", "-m", "v4")
+
+ trun(t, gt.client, "git", "fetch")
+ trun(t, gt.client, "git", "checkout", "-b", "work2", "-t", "origin/master")
+ write(t, gt.client+"/file", "modify")
+ write(t, gt.client+"/file1", "new")
+ trun(t, gt.client, "git", "add", "file", "file1")
+ trun(t, gt.client, "git", "commit", "-m", "some changes")
+ write(t, gt.client+"/file1", "modify")
+ write(t, gt.client+"/afile1", "new")
+ trun(t, gt.client, "git", "add", "file1", "afile1")
+ write(t, gt.client+"/file1", "modify again")
+ write(t, gt.client+"/file", "modify again")
+ write(t, gt.client+"/bfile", "untracked")
+
+ testPending(t, `
+ work REVHASH (5 behind)
+ msg
+
+ Change-Id: I123456789
+
+ Files in this change:
+ file
+
+ work2 REVHASH (current branch)
+ some changes
+
+ Files in this change:
+ file
+ file1
+ Files staged:
+ afile1
+ file1
+ Files unstaged:
+ file
+ file1
+ Files untracked:
+ bfile
+
+ `)
+}
+
+func TestPendingErrors(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ gt.work(t)
+ write(t, gt.client+"/file", "v2")
+ trun(t, gt.client, "git", "commit", "-a", "-m", "v2")
+
+ trun(t, gt.client, "git", "checkout", "master")
+ write(t, gt.client+"/file", "v3")
+ trun(t, gt.client, "git", "commit", "-a", "-m", "v3")
+
+ testPending(t, `
+ master REVHASH (current branch)
+ ERROR: Branch contains 1 commit not on origin/master.
+ Do not commit directly to master branch.
+
+ v3
+
+ Files in this change:
+ file
+
+ work REVHASH
+ ERROR: Branch contains 2 commits not on origin/origin/master.
+ There should be at most one.
+ Use 'git change', not 'git commit'.
+ Run 'git log origin/master..work' to list commits.
+
+ msg
+
+ Change-Id: I123456789
+
+ Files in this change:
+ file
+
+`)
+}
+
+func TestPendingGerrit(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+ gt.work(t)
+
+ srv := newGerritServer(t)
+ defer srv.done()
+
+ // Test error from Gerrit server.
+ testPending(t, `
+ work REVHASH (current branch)
+ msg
+
+ Change-Id: I123456789
+
+ Files in this change:
+ file
+
+ `)
+
+ setJSON := func(json string) {
+ srv.setReply("/a/changes/proj~master~I123456789", gerritReply{body: ")]}'\n" + json})
+ }
+
+ setJSON(`{
+ "current_revision": "` + CurrentBranch().CommitHash() + `",
+ "status": "MERGED",
+ "_number": 1234,
+ "owner": {"_id": 42},
+ "labels": {
+ "Code-Review": {
+ "all": [
+ {
+ "_id": 42,
+ "value": 0
+ },
+ {
+ "_id": 43,
+ "name": "George Opher",
+ "value": -2
+ },
+ {
+ "_id": 44,
+ "name": "Grace Emlin",
+ "value": 1
+ }
+ ]
+ },
+ "Other-Label": {
+ "all": [
+ {
+ "_account_id": 42,
+ "name": "The Owner",
+ "value": 2
+ }
+ ]
+ }
+ }
+ }`)
+
+ testPending(t, `
+ work REVHASH http://127.0.0.1:PORT/1234 (current branch, mailed, submitted)
+ msg
+
+ Change-Id: I123456789
+
+ Code-Review:
+ +1 Grace Emlin
+ -2 George Opher
+ Other-Label:
+ +2 The Owner
+ Files in this change:
+ file
+
+ `)
+
+}
+
+func testPending(t *testing.T, want string) {
+ // fake auth information to avoid Gerrit error
+ if auth.host == "" {
+ auth.host = "gerrit.fake"
+ auth.user = "not-a-user"
+ defer func() {
+ auth.host = ""
+ auth.user = ""
+ }()
+ }
+
+ want = strings.Replace(want, "\n\t", "\n", -1)
+ want = strings.Replace(want, "\n\t", "\n", -1)
+ want = strings.TrimPrefix(want, "\n")
+
+ testMain(t, "pending")
+ out := stdout.Bytes()
+
+ out = regexp.MustCompile(`\b[0-9a-f]{7}\b`).ReplaceAllLiteral(out, []byte("REVHASH"))
+ out = regexp.MustCompile(`\b127\.0\.0\.1:\d+\b`).ReplaceAllLiteral(out, []byte("127.0.0.1:PORT"))
+
+ if string(out) != want {
+ t.Errorf("invalid pending output:\n%s\nwant:\n%s", out, want)
+ if d, err := diff([]byte(want), out); err == nil {
+ t.Errorf("diff want actual:\n%s", d)
+ }
+ }
+}
+
+func diff(b1, b2 []byte) (data []byte, err error) {
+ f1, err := ioutil.TempFile("", "gofmt")
+ if err != nil {
+ return
+ }
+ defer os.Remove(f1.Name())
+ defer f1.Close()
+
+ f2, err := ioutil.TempFile("", "gofmt")
+ if err != nil {
+ return
+ }
+ defer os.Remove(f2.Name())
+ defer f2.Close()
+
+ f1.Write(b1)
+ f2.Write(b2)
+
+ data, err = exec.Command("diff", "-u", f1.Name(), f2.Name()).CombinedOutput()
+ if len(data) > 0 {
+ // diff exits with a non-zero status when the files don't match.
+ // Ignore that failure as long as we get output.
+ err = nil
+ }
+ return
+
+}
