git-codereview: add a few more tests
Change-Id: I17ad065e8b4c1eb00cdd2f17af82be76bae4fc09
Reviewed-on: https://go-review.googlesource.com/c/review/+/279717
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/git-codereview/change.go b/git-codereview/change.go
index ccc7836..991707d 100644
--- a/git-codereview/change.go
+++ b/git-codereview/change.go
@@ -6,7 +6,6 @@
import (
"fmt"
- "os"
"regexp"
"strconv"
"strings"
@@ -24,7 +23,7 @@
flags.Parse(args)
if len(flags.Args()) > 1 {
fmt.Fprintf(stderr(), "Usage: %s change %s [-a] [-m msg] [-q] [branch]\n", progName, globalFlags)
- os.Exit(2)
+ exit(2)
}
// Checkout or create branch, if specified.
@@ -56,13 +55,11 @@
}
func (b *Branch) check() {
- // TODO(rsc): Test
staged, unstaged, _ := LocalChanges()
if len(staged) == 0 && len(unstaged) == 0 {
// No staged changes, no unstaged changes.
// If the branch is behind upstream, now is a good time to point that out.
// This applies to both local work branches and tracking branches.
- // TODO(rsc): Test.
b.loadPending()
if n := b.CommitsBehind(); n > 0 {
printf("warning: %d commit%s behind %s; run 'git codereview sync' to update.", n, suffix(n, "s"), b.OriginBranch())
diff --git a/git-codereview/change_test.go b/git-codereview/change_test.go
index 206ed82..8a3960d 100644
--- a/git-codereview/change_test.go
+++ b/git-codereview/change_test.go
@@ -38,6 +38,20 @@
t.Logf("main -> dev.branch")
testMain(t, "change", "dev.branch")
testRan(t, "git checkout -q -t -b dev.branch origin/dev.branch")
+
+ testMain(t, "pending", "-c")
+ testPrintedStdout(t, "tracking dev.branch")
+ testMain(t, "change", "main")
+ testMain(t, "change", "work2")
+
+ t.Logf("server work × 2")
+ gt.serverWork(t)
+ gt.serverWork(t)
+ testMain(t, "sync")
+
+ t.Logf("-> work with server ahead")
+ testMain(t, "change", "work")
+ testPrintedStderr(t, "warning: 2 commits behind origin/main; run 'git codereview sync' to update")
}
func TestChangeHEAD(t *testing.T) {
diff --git a/git-codereview/gofmt.go b/git-codereview/gofmt.go
index bfba123..7986d07 100644
--- a/git-codereview/gofmt.go
+++ b/git-codereview/gofmt.go
@@ -23,7 +23,7 @@
flags.Parse(args)
if len(flag.Args()) > 0 {
fmt.Fprintf(stderr(), "Usage: %s gofmt %s [-l]\n", progName, globalFlags)
- os.Exit(2)
+ exit(2)
}
f := gofmtCommand
diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index e6cdb90..55ab8ed 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -6,7 +6,6 @@
import (
"fmt"
- "os"
"regexp"
"sort"
"strings"
@@ -38,11 +37,12 @@
"Usage: %s mail %s [-r reviewer,...] [-cc mail,...]\n"+
"\t[-f] [-diff] [-hashtag tag,...] [-nokeycheck] [-topic topic]\n"+
"\t[-trust] [-trybot] [-wip] [commit]\n", progName, globalFlags)
+ exit(2)
}
flags.Parse(args)
if len(flags.Args()) > 1 {
flags.Usage()
- os.Exit(2)
+ exit(2)
}
b := CurrentBranch()
@@ -221,7 +221,7 @@
verbosef("expanded %s to %s", short[1:], long[1:])
}
if errors {
- die()
+ exit(1)
}
return spec
}
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index e17a905..5808764 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -9,7 +9,6 @@
"fmt"
"io"
"net/http"
- "os"
"sort"
"strings"
"time"
@@ -87,7 +86,7 @@
flags.Parse(args)
if len(flags.Args()) > 0 {
fmt.Fprintf(stderr(), "Usage: %s pending %s [-c] [-l] [-s]\n", progName, globalFlags)
- os.Exit(2)
+ exit(2)
}
// Fetch info about remote changes, so that we can say which branches need sync.
diff --git a/git-codereview/pending_test.go b/git-codereview/pending_test.go
index ccb2f76..1124e4d 100644
--- a/git-codereview/pending_test.go
+++ b/git-codereview/pending_test.go
@@ -323,6 +323,7 @@
file
`)
+
testPendingArgs(t, []string{"-l", "-s"}, `
work REVHASH..REVHASH (current branch, 1 behind)
+ REVHASH msg
diff --git a/git-codereview/review.go b/git-codereview/review.go
index 0242ec7..0deb039 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -34,6 +34,7 @@
flags.Usage = func() {
fmt.Fprintf(stderr(), usage, progName, progName)
}
+ flags.SetOutput(stderr())
flags.BoolVar(noRun, "n", false, "print but do not run commands")
flags.Var(verbose, "v", "report commands")
}
@@ -76,10 +77,7 @@
if len(os.Args) < 2 {
flags.Usage()
- if dieTrap != nil {
- dieTrap()
- }
- os.Exit(2)
+ exit(2)
}
command, args := os.Args[1], os.Args[2:]
@@ -88,7 +86,7 @@
switch command {
default:
flags.Usage()
- return // avoid installing hooks.
+ exit(2) // avoid installing hooks.
case "help":
fmt.Fprintf(stdout(), help, progName)
return // avoid installing hooks.
@@ -140,7 +138,7 @@
flags.Parse(args)
if len(flags.Args()) > 0 {
fmt.Fprintf(stderr(), "Usage: %s %s %s\n", progName, command, globalFlags)
- os.Exit(2)
+ exit(2)
}
}
@@ -292,18 +290,18 @@
return strings.Join(append([]string{command}, args...), " ")
}
-var dieTrap func()
-
func dief(format string, args ...interface{}) {
printf(format, args...)
- die()
+ exit(1)
}
-func die() {
- if dieTrap != nil {
- dieTrap()
+var exitTrap func()
+
+func exit(code int) {
+ if exitTrap != nil {
+ exitTrap()
}
- os.Exit(1)
+ os.Exit(code)
}
func verbosef(format string, args ...interface{}) {
diff --git a/git-codereview/submit.go b/git-codereview/submit.go
index 9045ea4..5f684ca 100644
--- a/git-codereview/submit.go
+++ b/git-codereview/submit.go
@@ -7,7 +7,6 @@
import (
"bytes"
"fmt"
- "os"
"strings"
"time"
)
@@ -22,7 +21,7 @@
flags.Parse(args)
if interactive && flags.NArg() > 0 {
flags.Usage()
- os.Exit(2)
+ exit(2)
}
b := CurrentBranch()
diff --git a/git-codereview/submit_test.go b/git-codereview/submit_test.go
index fbe2cb3..29f9c02 100644
--- a/git-codereview/submit_test.go
+++ b/git-codereview/submit_test.go
@@ -80,7 +80,7 @@
testPrintedStderr(t, "cannot submit: change abandoned")
t.Logf("> missing approval")
- srv.setJSON(id, `{"status": "NEW", "labels": {"Code-Review": {}}}`)
+ srv.setJSON(id, `{"status": "NEW", "labels": {"Code-Review": {}, "Color": {"Optional": true}}}`)
testMainDied(t, "submit")
testRan(t) // nothing
testPrintedStderr(t, "cannot submit: change missing Code-Review approval")
@@ -160,6 +160,15 @@
submitted = true
return gerritReply{body: ")]}'\n" + submittedJSON}
}})
+
+ testMain(t, "submit", "-n")
+ testPrintedStderr(t, "submitting")
+ testPrintedStderr(t, "stopped before submit")
+
+ testMain(t, "pending", "-c", "-l")
+ testPrintedStdout(t, "(current branch)")
+ testPrintedStdout(t, "Files in this change:")
+
testMain(t, "submit")
testRan(t,
"git fetch -q",
@@ -196,17 +205,31 @@
gt := newGitTest(t)
defer gt.done()
+ os.Setenv("GIT_EDITOR", ":")
+ testMain(t, "submit", "-i")
+ testPrintedStderr(t, "nothing to submit")
+
srv := newGerritServer(t)
defer srv.done()
cl1, cl2 := testSubmitMultiple(t, gt, srv)
- os.Setenv("GIT_EDITOR", "echo "+cl1.CurrentRevision+" > ")
+
+ os.Setenv("GIT_EDITOR", "echo > ")
+ testMain(t, "submit", "-i")
+ if cl1.Status != "NEW" {
+ t.Fatalf("want cl1.Status == NEW; got %v", cl1.Status)
+ }
+ if cl2.Status != "NEW" {
+ t.Fatalf("want cl2.Status == NEW; got %v", cl2.Status)
+ }
+
+ os.Setenv("GIT_EDITOR", "( echo "+cl1.CurrentRevision+"; echo; echo '# comment' ) > ")
testMain(t, "submit", "-i")
if cl1.Status != "MERGED" {
t.Fatalf("want cl1.Status == MERGED; got %v", cl1.Status)
}
if cl2.Status != "NEW" {
- t.Fatalf("want cl2.Status == NEW; got %v", cl1.Status)
+ t.Fatalf("want cl2.Status == NEW; got %v", cl2.Status)
}
}
diff --git a/git-codereview/sync.go b/git-codereview/sync.go
index dc03c61..f518a3f 100644
--- a/git-codereview/sync.go
+++ b/git-codereview/sync.go
@@ -33,11 +33,11 @@
run("git", "pull", "-q", "-r", "origin", strings.TrimPrefix(b.OriginBranch(), "origin/"))
}
- // If the change commit has been submitted,
- // roll back change leaving any changes unstaged.
- // Pull should have done this for us, but check just in case.
b = CurrentBranch() // discard any cached information
if len(b.Pending()) == 1 && b.Submitted(id) {
+ // If the change commit has been submitted,
+ // roll back change leaving any changes unstaged.
+ // Pull should have done this for us, but check just in case.
run("git", "reset", b.Branchpoint())
}
}
diff --git a/git-codereview/sync_test.go b/git-codereview/sync_test.go
index a4210ab..72cbdf8 100644
--- a/git-codereview/sync_test.go
+++ b/git-codereview/sync_test.go
@@ -104,4 +104,22 @@
if len(b.Pending()) != 0 {
t.Fatalf("have %d pending CLs after final sync, want 0", len(b.Pending()))
}
+
+ // sync -v prints git output.
+ // also exercising -v parsing.
+ testMain(t, "sync", "-v=true")
+ testNoStdout(t)
+ testPrintedStderr(t, "git pull -q -r origin main")
+
+ testMain(t, "sync", "-v=1")
+ testNoStdout(t)
+ testPrintedStderr(t, "git pull -q -r origin main")
+
+ testMain(t, "sync", "-v")
+ testNoStdout(t)
+ testPrintedStderr(t, "git pull -q -r origin main")
+
+ testMain(t, "sync", "-v=false")
+ testNoStdout(t)
+ testNoStderr(t)
}
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 90a5b93..b1b5534 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -290,10 +290,9 @@
testStderr *bytes.Buffer
testStdout *bytes.Buffer
died bool
+ mainCanDie bool
)
-var mainCanDie bool
-
func testMainDied(t *testing.T, args ...string) {
t.Helper()
mainCanDie = true
@@ -319,7 +318,7 @@
testStdout = stdoutTrap
testStderr = stderrTrap
- dieTrap = nil
+ exitTrap = nil
runLogTrap = nil
stdoutTrap = nil
stderrTrap = nil
@@ -337,7 +336,7 @@
}
}()
- dieTrap = func() {
+ exitTrap = func() {
died = true
panic("died")
}
@@ -526,3 +525,19 @@
fmt.Fprintf(&buf, "%s", end)
w.Write(buf.Bytes())
}
+
+func TestUsage(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ testMainDied(t)
+ testPrintedStderr(t, "Usage: git-codereview <command>")
+
+ testMainDied(t, "not-a-command")
+ testPrintedStderr(t, "Usage: git-codereview <command>")
+
+ // During tests we configure the flag package to panic on error
+ // instead of
+ testMainDied(t, "mail", "-not-a-flag")
+ testPrintedStderr(t, "flag provided but not defined")
+}
