git-codereview: fix bug in sync for multi-commit client

When deciding whether to discard the commit info for
the final commit, it was using the first commit instead of
the final one.

Change-Id: I2304fba6fa82a1d21600c3caa08b6b119edcdb7f
Reviewed-on: https://go-review.googlesource.com/3628
Reviewed-by: Rob Pike <r@golang.org>

4 files changed, 87 insertions, 9 deletions

diff --git a/git-codereview/review.go b/git-codereview/review.go
index bc13a41..8ebb8d9 100644
--- a/git-codereview/review.go
+++ b/git-codereview/review.go
@@ -141,7 +141,7 @@ func main() {
 	case "pending":
 		cmdPending(args)
 	case "rebase-work":
-		cmdRebasework(args)
+		cmdRebaseWork(args)
 	case "submit":
 		cmdSubmit(args)
 	case "sync":
diff --git a/git-codereview/sync.go b/git-codereview/sync.go
index 59611ac..5cba5bc 100644
--- a/git-codereview/sync.go
+++ b/git-codereview/sync.go
@@ -13,7 +13,7 @@ func cmdSync(args []string) {
 	b := CurrentBranch()
 	var id string
 	if work := b.Pending(); len(work) > 0 {
-		id = work[len(work)-1].ChangeID
+		id = work[0].ChangeID
 	}
 
 	// Don't sync with staged or unstaged changes.
diff --git a/git-codereview/sync_test.go b/git-codereview/sync_test.go
index 0fdcbf8..2f7318d 100644
--- a/git-codereview/sync_test.go
+++ b/git-codereview/sync_test.go
@@ -45,4 +45,61 @@ func TestSync(t *testing.T) {
 	testNoStderr(t)
 }
 
-// TODO: Add TestSyncRebase?
+func TestSyncRebase(t *testing.T) {
+	gt := newGitTest(t)
+	defer gt.done()
+
+	// client 3 ahead
+	gt.work(t)
+	gt.work(t)
+	gt.work(t)
+
+	b := CurrentBranch()
+	if len(b.Pending()) != 3 {
+		t.Fatalf("have %d pending CLs, want 3", len(b.Pending()))
+	}
+	top := b.Pending()[0].Hash
+
+	// check for success for sync no-op
+	testMain(t, "sync")
+	testNoStdout(t)
+	testNoStderr(t)
+
+	b = CurrentBranch()
+	if len(b.Pending()) != 3 {
+		t.Fatalf("have %d pending CLs after no-op sync, want 3", len(b.Pending()))
+	}
+	if b.Pending()[0].Hash != top {
+		t.Fatalf("CL hashes changed during no-op sync")
+	}
+
+	// submit first two CLs - gt.serverWork does same thing gt.work does, but on client
+	gt.serverWork(t)
+	gt.serverWork(t)
+
+	testMain(t, "sync")
+	testNoStdout(t)
+	testNoStderr(t)
+
+	// there should be one left, and it should be a different hash
+	b = CurrentBranch()
+	if len(b.Pending()) != 1 {
+		t.Fatalf("have %d pending CLs after submitting two, want 1", len(b.Pending()))
+	}
+	if b.Pending()[0].Hash == top {
+		t.Fatalf("CL hashes DID NOT change during sync after submit")
+	}
+
+	// submit final change
+	gt.serverWork(t)
+
+	testMain(t, "sync")
+	testNoStdout(t)
+	testNoStderr(t)
+
+	// there should be none left
+	b = CurrentBranch()
+	if len(b.Pending()) != 0 {
+		t.Fatalf("have %d pending CLs after final sync, want 0", len(b.Pending()))
+	}
+}
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 1fd2505..380609f 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -20,11 +20,12 @@ import (
 )
 
 type gitTest struct {
-	pwd    string // current directory before test
-	tmpdir string // temporary directory holding repos
-	server string // server repo root
-	client string // client repo root
-	nwork  int    // number of calls to work method
+	pwd         string // current directory before test
+	tmpdir      string // temporary directory holding repos
+	server      string // server repo root
+	client      string // client repo root
+	nwork       int    // number of calls to work method
+	nworkServer int    // number of calls to serverWork method
 }
 
 // resetReadOnlyFlagAll resets windows read-only flag
@@ -71,7 +72,27 @@ func (gt *gitTest) work(t *testing.T) {
 	gt.nwork++
 	write(t, gt.client+"/file", fmt.Sprintf("new content %d", gt.nwork))
 	trun(t, gt.client, "git", "add", "file")
-	trun(t, gt.client, "git", "commit", "-m", fmt.Sprintf("msg\n\nChange-Id: I%d23456789\n", gt.nwork))
+	suffix := ""
+	if gt.nwork > 1 {
+		suffix = fmt.Sprintf(" #%d", gt.nwork)
+	}
+	trun(t, gt.client, "git", "commit", "-m", fmt.Sprintf("msg%s\n\nChange-Id: I%d23456789\n", suffix, gt.nwork))
+}
+
+func (gt *gitTest) serverWork(t *testing.T) {
+	// make change on server
+	// duplicating the changes of gt.work to simulate them
+	// having gone through Gerrit and submitted with
+	// different times and commit hashes but the same content.
+	gt.nworkServer++
+	write(t, gt.server+"/file", fmt.Sprintf("new content %d", gt.nworkServer))
+	trun(t, gt.server, "git", "add", "file")
+	suffix := ""
+	if gt.nworkServer > 1 {
+		suffix = fmt.Sprintf(" #%d", gt.nworkServer)
+	}
+	trun(t, gt.server, "git", "commit", "-m", fmt.Sprintf("msg%s\n\nChange-Id: I%d23456789\n", suffix, gt.nworkServer))
+
 }
 
 func newGitTest(t *testing.T) (gt *gitTest) {