git-codereview: write hooks with execute permission in tests

The 'write' helper function previously assumed file permission 0666.
We don't actually need these files to be world-writable, but we do
need specific ones — scripts executed as Git commit hooks — to be
executable, at least on certain platforms.

Also use t.Helper() to produce more useful log lines for failures.
(That was added in order to diagnose TryBot failures.)

Fixes golang/go#32836

Change-Id: I2d670563f42778ef8cf645445420756599c37ac6
Reviewed-on: https://go-review.googlesource.com/c/review/+/211477
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>

10 files changed, 122 insertions, 100 deletions

diff --git a/git-codereview/api_test.go b/git-codereview/api_test.go
index 997a347..28ccce9 100644
--- a/git-codereview/api_test.go
+++ b/git-codereview/api_test.go
@@ -104,11 +104,11 @@ func TestLoadAuth(t *testing.T) {
 		remove(t, netrc)
 		remove(t, gt.client+"/.cookies")
 		if tt.netrc != "" {
-			write(t, netrc, tt.netrc)
+			write(t, netrc, tt.netrc, 0644)
 		}
 		if tt.cookiefile != "" {
 			if tt.cookiefile != "MISSING" {
-				write(t, gt.client+"/.cookies", tt.cookiefile)
+				write(t, gt.client+"/.cookies", tt.cookiefile, 0644)
 			}
 			trun(t, gt.client, "git", "config", "http.cookiefile", "~/.cookies")
 		}
diff --git a/git-codereview/branch_test.go b/git-codereview/branch_test.go
index 8d1b2fd..d5b8a43 100644
--- a/git-codereview/branch_test.go
+++ b/git-codereview/branch_test.go
@@ -22,7 +22,7 @@ func TestCurrentBranch(t *testing.T) {
 	checkCurrentBranch(t, "newbranch", "origin/master", true, false, "", "")
 
 	t.Logf("making change")
-	write(t, gt.client+"/file", "i made a change")
+	write(t, gt.client+"/file", "i made a change", 0644)
 	trun(t, gt.client, "git", "commit", "-a", "-m", "My change line.\n\nChange-Id: I0123456789abcdef0123456789abcdef\n")
 	checkCurrentBranch(t, "newbranch", "origin/master", true, true, "I0123456789abcdef0123456789abcdef", "My change line.")
 
@@ -35,7 +35,7 @@ func TestCurrentBranch(t *testing.T) {
 	checkCurrentBranch(t, "newdev", "origin/dev.branch", true, false, "", "")
 
 	t.Logf("making change")
-	write(t, gt.client+"/file", "i made another change")
+	write(t, gt.client+"/file", "i made another change", 0644)
 	trun(t, gt.client, "git", "commit", "-a", "-m", "My other change line.\n\nChange-Id: I1123456789abcdef0123456789abcdef\n")
 	checkCurrentBranch(t, "newdev", "origin/dev.branch", true, true, "I1123456789abcdef0123456789abcdef", "My other change line.")
 
@@ -103,9 +103,9 @@ func TestAmbiguousRevision(t *testing.T) {
 
 	t.Logf("creating file paths that conflict with revision parameters")
 	mkdir(t, gt.client+"/origin")
-	write(t, gt.client+"/origin/master..work", "Uh-Oh! SpaghettiOs")
+	write(t, gt.client+"/origin/master..work", "Uh-Oh! SpaghettiOs", 0644)
 	mkdir(t, gt.client+"/work..origin")
-	write(t, gt.client+"/work..origin/master", "Be sure to drink your Ovaltine")
+	write(t, gt.client+"/work..origin/master", "Be sure to drink your Ovaltine", 0644)
 
 	b := CurrentBranch()
 	b.Submitted("I123456789")
@@ -140,7 +140,7 @@ func TestRebaseWork(t *testing.T) {
 	testMainDied(t, "rebase-work", "-n")
 	testPrintedStderr(t, "no pending work")
 
-	write(t, gt.client+"/file", "uncommitted")
+	write(t, gt.client+"/file", "uncommitted", 0644)
 	testMainDied(t, "rebase-work", "-n")
 	testPrintedStderr(t, "cannot rebase with uncommitted work")
 
@@ -160,7 +160,7 @@ func TestBranchpointMerge(t *testing.T) {
 	defer gt.done()
 
 	// commit more work on master
-	write(t, gt.server+"/file", "more work")
+	write(t, gt.server+"/file", "more work", 0644)
 	trun(t, gt.server, "git", "commit", "-m", "work", "file")
 
 	// update client
diff --git a/git-codereview/change_test.go b/git-codereview/change_test.go
index 8e111be..e9337dc 100644
--- a/git-codereview/change_test.go
+++ b/git-codereview/change_test.go
@@ -29,7 +29,7 @@ func TestChange(t *testing.T) {
 	testRan(t, "git checkout -q master")
 
 	t.Logf("master -> work with staged changes")
-	write(t, gt.client+"/file", "new content")
+	write(t, gt.client+"/file", "new content", 0644)
 	trun(t, gt.client, "git", "add", "file")
 	testMain(t, "change", "work")
 	testRan(t, "git checkout -q work",
@@ -53,7 +53,7 @@ func TestChangeAhead(t *testing.T) {
 	defer gt.done()
 
 	// commit to master (mistake)
-	write(t, gt.client+"/file", "new content")
+	write(t, gt.client+"/file", "new content", 0644)
 	trun(t, gt.client, "git", "add", "file")
 	trun(t, gt.client, "git", "commit", "-m", "msg")
 
@@ -88,7 +88,7 @@ func TestChangeAmendCommit(t *testing.T) {
 	testCommitMsg = "foo: amended commit message"
 	gt.work(t)
 
-	write(t, gt.client+"/file", "new content in work to be amend")
+	write(t, gt.client+"/file", "new content in work to be amend", 0644)
 	trun(t, gt.client, "git", "add", "file")
 	testMain(t, "change")
 }
@@ -101,7 +101,7 @@ func TestChangeFailAmendWithMultiplePending(t *testing.T) {
 	gt.work(t)
 	gt.work(t)
 
-	write(t, gt.client+"/file", "new content in work to be amend")
+	write(t, gt.client+"/file", "new content in work to be amend", 0644)
 	trun(t, gt.client, "git", "add", "file")
 	testMainDied(t, "change")
 	testPrintedStderr(t, "multiple changes pending")
@@ -115,7 +115,7 @@ func TestChangeCL(t *testing.T) {
 	defer srv.done()
 
 	// Ensure that 'change' with a CL accepts we have gerrit. Test address is injected by newGerritServer.
-	write(t, gt.server+"/codereview.cfg", "gerrit: on")
+	write(t, gt.server+"/codereview.cfg", "gerrit: on", 0644)
 	trun(t, gt.server, "git", "add", "codereview.cfg")
 	trun(t, gt.server, "git", "commit", "-m", "codereview.cfg on master")
 	trun(t, gt.client, "git", "pull")
diff --git a/git-codereview/gofmt_test.go b/git-codereview/gofmt_test.go
index bdd0620..0bd6aa9 100644
--- a/git-codereview/gofmt_test.go
+++ b/git-codereview/gofmt_test.go
@@ -34,13 +34,13 @@ func TestGofmt(t *testing.T) {
 	if err := os.MkdirAll(gt.client+"/vendor", 0755); err != nil {
 		t.Fatal(err)
 	}
-	write(t, gt.client+"/bad.go", badGo)
-	write(t, gt.client+"/good.go", goodGo)
-	write(t, gt.client+"/vendor/bad.go", badGo)
-	write(t, gt.client+"/test/bad.go", badGo)
-	write(t, gt.client+"/test/good.go", goodGo)
-	write(t, gt.client+"/test/bench/bad.go", badGo)
-	write(t, gt.client+"/test/bench/good.go", goodGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
+	write(t, gt.client+"/good.go", goodGo, 0644)
+	write(t, gt.client+"/vendor/bad.go", badGo, 0644)
+	write(t, gt.client+"/test/bad.go", badGo, 0644)
+	write(t, gt.client+"/test/good.go", goodGo, 0644)
+	write(t, gt.client+"/test/bench/bad.go", badGo, 0644)
+	write(t, gt.client+"/test/bench/good.go", goodGo, 0644)
 	trun(t, gt.client, "git", "add", ".") // make files tracked
 
 	testMain(t, "gofmt", "-l")
@@ -54,8 +54,8 @@ func TestGofmt(t *testing.T) {
 	testMain(t, "gofmt", "-l")
 	testNoStdout(t)
 
-	write(t, gt.client+"/bad.go", badGo)
-	write(t, gt.client+"/broken.go", brokenGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
+	write(t, gt.client+"/broken.go", brokenGo, 0644)
 	trun(t, gt.client, "git", "add", ".")
 	testMainDied(t, "gofmt", "-l")
 	testPrintedStdout(t, "bad.go")
@@ -71,8 +71,8 @@ func TestGofmtSubdir(t *testing.T) {
 
 	mkdir(t, gt.client+"/dir1")
 	mkdir(t, gt.client+"/longnamedir2")
-	write(t, gt.client+"/dir1/bad1.go", badGo)
-	write(t, gt.client+"/longnamedir2/bad2.go", badGo)
+	write(t, gt.client+"/dir1/bad1.go", badGo, 0644)
+	write(t, gt.client+"/longnamedir2/bad2.go", badGo, 0644)
 	trun(t, gt.client, "git", "add", ".") // make files tracked
 
 	chdir(t, gt.client)
@@ -106,11 +106,11 @@ func TestGofmtSubdirIndexCheckout(t *testing.T) {
 
 	mkdir(t, gt.client+"/dir1")
 	mkdir(t, gt.client+"/longnamedir2")
-	write(t, gt.client+"/dir1/bad1.go", badGo)
-	write(t, gt.client+"/longnamedir2/bad2.go", badGo)
+	write(t, gt.client+"/dir1/bad1.go", badGo, 0644)
+	write(t, gt.client+"/longnamedir2/bad2.go", badGo, 0644)
 	trun(t, gt.client, "git", "add", ".") // put files in index
-	write(t, gt.client+"/dir1/bad1.go", goodGo)
-	write(t, gt.client+"/longnamedir2/bad2.go", goodGo)
+	write(t, gt.client+"/dir1/bad1.go", goodGo, 0644)
+	write(t, gt.client+"/longnamedir2/bad2.go", goodGo, 0644)
 
 	chdir(t, gt.client)
 	testMain(t, "gofmt", "-l")
@@ -160,7 +160,7 @@ func TestGofmtUnstaged(t *testing.T) {
 			text := orig[j%N]
 			file := fmt.Sprintf("%s-%s-%s.go", name[i/N/N], name[(i/N)%N], name[i%N])
 			allFiles = append(allFiles, file)
-			write(t, gt.client+"/"+file, text)
+			write(t, gt.client+"/"+file, text, 0644)
 
 			if (i/N)%N != i%N {
 				staged := file + " (staged)"
@@ -246,7 +246,7 @@ func TestGofmtAmbiguousRevision(t *testing.T) {
 	defer gt.done()
 
 	t.Logf("creating file that conflicts with revision parameter")
-	write(t, gt.client+"/HEAD", "foo")
+	write(t, gt.client+"/HEAD", "foo", 0644)
 
 	testMain(t, "gofmt")
 }
@@ -256,12 +256,12 @@ func TestGofmtFastForwardMerge(t *testing.T) {
 	defer gt.done()
 
 	// merge dev.branch into master
-	write(t, gt.server+"/file", "more work")
+	write(t, gt.server+"/file", "more work", 0644)
 	trun(t, gt.server, "git", "commit", "-m", "work", "file")
 	trun(t, gt.server, "git", "merge", "-m", "merge", "dev.branch")
 
 	// add bad go file on master
-	write(t, gt.server+"/bad.go", "package {\n")
+	write(t, gt.server+"/bad.go", "package {\n", 0644)
 	trun(t, gt.server, "git", "add", "bad.go")
 	trun(t, gt.server, "git", "commit", "-m", "bad go")
 
diff --git a/git-codereview/hook_test.go b/git-codereview/hook_test.go
index d1cc0c6..e9dc1c3 100644
--- a/git-codereview/hook_test.go
+++ b/git-codereview/hook_test.go
@@ -21,7 +21,7 @@ func TestHookCommitMsgGerrit(t *testing.T) {
 	defer gt.done()
 
 	// Check that hook adds Change-Id.
-	write(t, gt.client+"/msg.txt", "Test message.\n")
+	write(t, gt.client+"/msg.txt", "Test message.\n", 0644)
 	testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
 	data := read(t, gt.client+"/msg.txt")
 	if !bytes.Contains(data, []byte("\n\nChange-Id: ")) {
@@ -36,7 +36,7 @@ func TestHookCommitMsgGerrit(t *testing.T) {
 	}
 
 	// Check that hook rejects multiple Change-Ids.
-	write(t, gt.client+"/msgdouble.txt", string(data)+string(data))
+	write(t, gt.client+"/msgdouble.txt", string(data)+string(data), 0644)
 	testMainDied(t, "hook-invoke", "commit-msg", gt.client+"/msgdouble.txt")
 	const multiple = "git-codereview: multiple Change-Id lines\n"
 	if got := testStderr.String(); got != multiple {
@@ -45,7 +45,7 @@ func TestHookCommitMsgGerrit(t *testing.T) {
 
 	// Check that hook doesn't add two line feeds before Change-Id
 	// if the exsting message ends with a metadata line.
-	write(t, gt.client+"/msg.txt", "Test message.\n\nBug: 1234\n")
+	write(t, gt.client+"/msg.txt", "Test message.\n\nBug: 1234\n", 0644)
 	testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
 	data = read(t, gt.client+"/msg.txt")
 	if !bytes.Contains(data, []byte("Bug: 1234\nChange-Id: ")) {
@@ -59,7 +59,7 @@ func TestHookCommitMsg(t *testing.T) {
 	defer gt.done()
 
 	// Check that hook fails when message is empty.
-	write(t, gt.client+"/empty.txt", "\n\n# just a file with\n# comments\n")
+	write(t, gt.client+"/empty.txt", "\n\n# just a file with\n# comments\n", 0644)
 	testMainDied(t, "hook-invoke", "commit-msg", gt.client+"/empty.txt")
 	const empty = "git-codereview: empty commit message\n"
 	if got := testStderr.String(); got != empty {
@@ -83,9 +83,9 @@ func TestHookCommitMsg(t *testing.T) {
 		},
 	}
 	for _, tt := range rewrites {
-		write(t, gt.client+"/in.txt", tt.in)
+		write(t, gt.client+"/in.txt", tt.in, 0644)
 		testMain(t, "hook-invoke", "commit-msg", gt.client+"/in.txt")
-		write(t, gt.client+"/want.txt", tt.want)
+		write(t, gt.client+"/want.txt", tt.want, 0644)
 		testMain(t, "hook-invoke", "commit-msg", gt.client+"/want.txt")
 		got, err := ioutil.ReadFile(gt.client + "/in.txt")
 		if err != nil {
@@ -115,7 +115,7 @@ func TestHookCommitMsgIssueRepoRewrite(t *testing.T) {
 		// Don't forget to write back if Change-Id already exists
 	}
 	for _, msg := range msgs {
-		write(t, gt.client+"/msg.txt", msg)
+		write(t, gt.client+"/msg.txt", msg, 0644)
 		testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
 		got := read(t, gt.client+"/msg.txt")
 		const want = "math/big: catch all the rats\n\nFixes #99999, at least for now\n"
@@ -125,7 +125,7 @@ func TestHookCommitMsgIssueRepoRewrite(t *testing.T) {
 	}
 
 	// Add issuerepo config, clear any previous config.
-	write(t, gt.client+"/codereview.cfg", "issuerepo: golang/go")
+	write(t, gt.client+"/codereview.cfg", "issuerepo: golang/go", 0644)
 	cachedConfig = nil
 
 	// Check for the rewrite
@@ -136,7 +136,7 @@ func TestHookCommitMsgIssueRepoRewrite(t *testing.T) {
 		"math/big: catch all the rats\n\nFixes issue golang/go#99999, at least for now\n",
 	}
 	for _, msg := range msgs {
-		write(t, gt.client+"/msg.txt", msg)
+		write(t, gt.client+"/msg.txt", msg, 0644)
 		testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
 		got := read(t, gt.client+"/msg.txt")
 		const want = "math/big: catch all the rats\n\nFixes golang/go#99999, at least for now\n"
@@ -164,7 +164,7 @@ func testHookCommitMsgBranchPrefix(t *testing.T, gerrit bool) {
 	defer gt.done()
 
 	checkPrefix := func(prefix string) {
-		write(t, gt.client+"/msg.txt", "Test message.\n")
+		write(t, gt.client+"/msg.txt", "Test message.\n", 0644)
 		testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
 		data, err := ioutil.ReadFile(gt.client + "/msg.txt")
 		if err != nil {
@@ -177,7 +177,7 @@ func testHookCommitMsgBranchPrefix(t *testing.T, gerrit bool) {
 		if i := strings.Index(prefix, "]"); i >= 0 {
 			prefix := prefix[:i+1]
 			for _, magic := range []string{"fixup!", "squash!"} {
-				write(t, gt.client+"/msg.txt", magic+" Test message.\n")
+				write(t, gt.client+"/msg.txt", magic+" Test message.\n", 0644)
 				testMain(t, "hook-invoke", "commit-msg", gt.client+"/msg.txt")
 				data, err := ioutil.ReadFile(gt.client + "/msg.txt")
 				if err != nil {
@@ -224,26 +224,26 @@ func TestHookPreCommit(t *testing.T) {
 
 	// Write out a non-Go file.
 	testMain(t, "change", "mybranch")
-	write(t, gt.client+"/msg.txt", "A test message.")
+	write(t, gt.client+"/msg.txt", "A test message.", 0644)
 	trun(t, gt.client, "git", "add", "msg.txt")
 	testMain(t, "hook-invoke", "pre-commit") // should be no-op
 
 	if err := os.MkdirAll(gt.client+"/test/bench", 0755); err != nil {
 		t.Fatal(err)
 	}
-	write(t, gt.client+"/bad.go", badGo)
-	write(t, gt.client+"/good.go", goodGo)
-	write(t, gt.client+"/test/bad.go", badGo)
-	write(t, gt.client+"/test/good.go", goodGo)
-	write(t, gt.client+"/test/bench/bad.go", badGo)
-	write(t, gt.client+"/test/bench/good.go", goodGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
+	write(t, gt.client+"/good.go", goodGo, 0644)
+	write(t, gt.client+"/test/bad.go", badGo, 0644)
+	write(t, gt.client+"/test/good.go", goodGo, 0644)
+	write(t, gt.client+"/test/bench/bad.go", badGo, 0644)
+	write(t, gt.client+"/test/bench/good.go", goodGo, 0644)
 	trun(t, gt.client, "git", "add", ".")
 
 	testMainDied(t, "hook-invoke", "pre-commit")
 	testPrintedStderr(t, "gofmt needs to format these files (run 'git gofmt'):",
 		"bad.go", "!good.go", fromSlash("!test/bad"), fromSlash("test/bench/bad.go"))
 
-	write(t, gt.client+"/broken.go", brokenGo)
+	write(t, gt.client+"/broken.go", brokenGo, 0644)
 	trun(t, gt.client, "git", "add", "broken.go")
 	testMainDied(t, "hook-invoke", "pre-commit")
 	testPrintedStderr(t, "gofmt needs to format these files (run 'git gofmt'):",
@@ -261,7 +261,7 @@ func TestHookChangeGofmt(t *testing.T) {
 	gt.work(t)
 
 	// Write out a non-Go file.
-	write(t, gt.client+"/bad.go", badGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
 	trun(t, gt.client, "git", "add", ".")
 
 	t.Logf("invoking commit hook explicitly")
@@ -291,7 +291,7 @@ func TestHookPreCommitDetachedHead(t *testing.T) {
 	defer gt.done()
 	gt.work(t)
 
-	write(t, gt.client+"/bad.go", badGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
 	trun(t, gt.client, "git", "add", ".")
 	trun(t, gt.client, "git", "checkout", "HEAD^0")
 
@@ -311,7 +311,7 @@ func TestHookPreCommitDetachedHead(t *testing.T) {
 		defer gt.done()
 		gt.work(t)
 
-		write(t, gt.client+"/bad.go", badGo)
+		write(t, gt.client+"/bad.go", badGo, 0644)
 		trun(t, gt.client, "git", "add", ".")
 		trun(t, gt.client, "git", "checkout", "HEAD^0")
 
@@ -328,7 +328,7 @@ func TestHookPreCommitEnv(t *testing.T) {
 	defer gt.done()
 	gt.work(t)
 
-	write(t, gt.client+"/bad.go", badGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
 	trun(t, gt.client, "git", "add", ".")
 	os.Setenv("GIT_GOFMT_HOOK", "off")
 	defer os.Unsetenv("GIT_GOFMT_HOOK")
@@ -343,8 +343,8 @@ func TestHookPreCommitUnstaged(t *testing.T) {
 	defer gt.done()
 	gt.work(t)
 
-	write(t, gt.client+"/bad.go", badGo)
-	write(t, gt.client+"/good.go", goodGo)
+	write(t, gt.client+"/bad.go", badGo, 0644)
+	write(t, gt.client+"/good.go", goodGo, 0644)
 
 	// The pre-commit hook is being asked about files in the index.
 	// Make sure it is not looking at files in the working tree (current directory) instead.
@@ -373,7 +373,7 @@ func TestHookPreCommitUnstaged(t *testing.T) {
 			}
 			file := fmt.Sprintf("%s-%s-%s.go", name[i/N/N], name[(i/N)%N], name[i%N])
 			allFiles = append(allFiles, file)
-			write(t, gt.client+"/"+file, content[j%N])
+			write(t, gt.client+"/"+file, content[j%N], 0644)
 
 			switch {
 			case strings.Contains(file, "-bad-"):
@@ -475,7 +475,7 @@ func TestHooksOverwriteOldCommitMsg(t *testing.T) {
 	gt := newGitTest(t)
 	defer gt.done()
 
-	write(t, gt.client+"/.git/hooks/commit-msg", oldCommitMsgHook)
+	write(t, gt.client+"/.git/hooks/commit-msg", oldCommitMsgHook, 0755)
 	testMain(t, "hooks") // install hooks
 	data, err := ioutil.ReadFile(gt.client + "/.git/hooks/commit-msg")
 	if err != nil {
@@ -510,7 +510,7 @@ func TestHookCommitMsgFromGit(t *testing.T) {
 	defer restore()
 
 	testMain(t, "change", "mybranch")
-	write(t, gt.client+"/file", "more data")
+	write(t, gt.client+"/file", "more data", 0644)
 	trun(t, gt.client, "git", "add", "file")
 	trun(t, gt.client, "git", "commit", "-m", "mymsg")
 
diff --git a/git-codereview/mail_test.go b/git-codereview/mail_test.go
index 3a63c33..22d4747 100644
--- a/git-codereview/mail_test.go
+++ b/git-codereview/mail_test.go
@@ -130,7 +130,7 @@ func TestMailAmbiguousRevision(t *testing.T) {
 	t.Logf("creating file that conflicts with revision parameter")
 	b := CurrentBranch()
 	mkdir(t, gt.client+"/origin")
-	write(t, gt.client+"/"+b.Branchpoint()+"..HEAD", "foo")
+	write(t, gt.client+"/"+b.Branchpoint()+"..HEAD", "foo", 0644)
 
 	testMain(t, "mail", "-diff")
 }
@@ -191,7 +191,7 @@ func TestMailShort(t *testing.T) {
 
 	// Seed commit history with reviewers.
 	for i, addr := range reviewerLog {
-		write(t, gt.server+"/file", fmt.Sprintf("v%d", i))
+		write(t, gt.server+"/file", fmt.Sprintf("v%d", i), 0644)
 		trun(t, gt.server, "git", "commit", "-a", "-m", "msg\n\nReviewed-by: "+addr+"\n")
 	}
 	trun(t, gt.client, "git", "pull")
diff --git a/git-codereview/pending_test.go b/git-codereview/pending_test.go
index 025fb5b..ee5e947 100644
--- a/git-codereview/pending_test.go
+++ b/git-codereview/pending_test.go
@@ -62,30 +62,30 @@ func TestPendingComplex(t *testing.T) {
 	defer gt.done()
 	gt.work(t)
 
-	write(t, gt.server+"/file", "v2")
+	write(t, gt.server+"/file", "v2", 0644)
 	trun(t, gt.server, "git", "commit", "-a", "-m", "v2")
 
-	write(t, gt.server+"/file", "v3")
+	write(t, gt.server+"/file", "v3", 0644)
 	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")
+	write(t, gt.server+"/file", "v4", 0644)
 	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")
+	write(t, gt.client+"/file", "modify", 0644)
+	write(t, gt.client+"/file1", "new", 0644)
 	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")
+	write(t, gt.client+"/file1", "modify", 0644)
+	write(t, gt.client+"/afile1", "new", 0644)
 	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")
+	write(t, gt.client+"/file1", "modify again", 0644)
+	write(t, gt.client+"/file", "modify again", 0644)
+	write(t, gt.client+"/bfile", "untracked", 0644)
 
 	testPending(t, `
 		work2 REVHASH..REVHASH (current branch)
@@ -160,7 +160,7 @@ func TestPendingErrors(t *testing.T) {
 	defer gt.done()
 
 	trun(t, gt.client, "git", "checkout", "master")
-	write(t, gt.client+"/file", "v3")
+	write(t, gt.client+"/file", "v3", 0644)
 	trun(t, gt.client, "git", "commit", "-a", "-m", "v3")
 
 	testPending(t, `
@@ -190,14 +190,14 @@ func TestPendingMultiChange(t *testing.T) {
 	defer gt.done()
 
 	gt.work(t)
-	write(t, gt.client+"/file", "v2")
+	write(t, gt.client+"/file", "v2", 0644)
 	trun(t, gt.client, "git", "commit", "-a", "-m", "v2")
 
-	write(t, gt.client+"/file", "v4")
+	write(t, gt.client+"/file", "v4", 0644)
 	trun(t, gt.client, "git", "add", "file")
 
-	write(t, gt.client+"/file", "v5")
-	write(t, gt.client+"/file2", "v6")
+	write(t, gt.client+"/file", "v5", 0644)
+	write(t, gt.client+"/file2", "v6", 0644)
 
 	testPending(t, `
 		work REVHASH..REVHASH (current branch)
@@ -266,7 +266,7 @@ func TestPendingGerrit(t *testing.T) {
 	// Test local mode does not talk to any server.
 	// Make client 1 behind server.
 	// The '1 behind' should not show up, nor any Gerrit information.
-	write(t, gt.server+"/file", "v4")
+	write(t, gt.server+"/file", "v4", 0644)
 	trun(t, gt.server, "git", "add", "file")
 	trun(t, gt.server, "git", "commit", "-m", "msg")
 	testPendingArgs(t, []string{"-l"}, `
@@ -337,15 +337,15 @@ func TestPendingGerritMultiChange(t *testing.T) {
 	gt.work(t)
 	hash1 := CurrentBranch().Pending()[0].Hash
 
-	write(t, gt.client+"/file", "v2")
+	write(t, gt.client+"/file", "v2", 0644)
 	trun(t, gt.client, "git", "commit", "-a", "-m", "v2\n\nChange-Id: I2345")
 	hash2 := CurrentBranch().Pending()[0].Hash
 
-	write(t, gt.client+"/file", "v4")
+	write(t, gt.client+"/file", "v4", 0644)
 	trun(t, gt.client, "git", "add", "file")
 
-	write(t, gt.client+"/file", "v5")
-	write(t, gt.client+"/file2", "v6")
+	write(t, gt.client+"/file", "v5", 0644)
+	write(t, gt.client+"/file2", "v6", 0644)
 
 	srv := newGerritServer(t)
 	defer srv.done()
@@ -417,7 +417,7 @@ func TestPendingGerritMultiChange15(t *testing.T) {
 	testPendingReply(srv, "I123456789", hash1, "MERGED")
 
 	for i := 1; i < 15; i++ {
-		write(t, gt.client+"/file", fmt.Sprintf("v%d", i))
+		write(t, gt.client+"/file", fmt.Sprintf("v%d", i), 0644)
 		trun(t, gt.client, "git", "commit", "-a", "-m", fmt.Sprintf("v%d\n\nChange-Id: I%010d", i, i))
 		hash2 := CurrentBranch().Pending()[0].Hash
 		testPendingReply(srv, fmt.Sprintf("I%010d", i), hash2, "NEW")
diff --git a/git-codereview/submit_test.go b/git-codereview/submit_test.go
index db783f8..127d7d3 100644
--- a/git-codereview/submit_test.go
+++ b/git-codereview/submit_test.go
@@ -21,7 +21,7 @@ func TestSubmitErrors(t *testing.T) {
 	t.Logf("> no commit")
 	testMainDied(t, "submit")
 	testPrintedStderr(t, "cannot submit: no changes pending")
-	write(t, gt.client+"/file1", "")
+	write(t, gt.client+"/file1", "", 0644)
 	trun(t, gt.client, "git", "add", "file1")
 	trun(t, gt.client, "git", "commit", "-m", "msg\n\nDO NOT SUBMIT\n\nChange-Id: I123456789\n")
 
@@ -32,7 +32,7 @@ func TestSubmitErrors(t *testing.T) {
 	trun(t, gt.client, "git", "commit", "--amend", "-m", "msg\n\nChange-Id: I123456789\n")
 
 	t.Logf("> staged changes")
-	write(t, gt.client+"/file1", "asdf")
+	write(t, gt.client+"/file1", "asdf", 0644)
 	trun(t, gt.client, "git", "add", "file1")
 	testMainDied(t, "submit")
 	testPrintedStderr(t, "cannot submit: staged changes exist",
@@ -40,7 +40,7 @@ func TestSubmitErrors(t *testing.T) {
 	testNoStdout(t)
 
 	t.Logf("> unstaged changes")
-	write(t, gt.client+"/file1", "actual content")
+	write(t, gt.client+"/file1", "actual content", 0644)
 	testMainDied(t, "submit")
 	testPrintedStderr(t, "cannot submit: unstaged changes exist",
 		"git status", "git stash", "git add", "git-codereview change")
@@ -137,7 +137,7 @@ func TestSubmit(t *testing.T) {
 	trun(t, gt.client, "git", "tag", "-f", "work.mailed")
 	clientHead := strings.TrimSpace(trun(t, gt.client, "git", "log", "-n", "1", "--format=format:%H"))
 
-	write(t, gt.server+"/file", "another change")
+	write(t, gt.server+"/file", "another change", 0644)
 	trun(t, gt.server, "git", "add", "file")
 	trun(t, gt.server, "git", "commit", "-m", "conflict")
 	serverHead := strings.TrimSpace(trun(t, gt.server, "git", "log", "-n", "1", "--format=format:%H"))
@@ -217,12 +217,12 @@ func TestSubmitInteractive(t *testing.T) {
 }
 
 func testSubmitMultiple(t *testing.T, gt *gitTest, srv *gerritServer) (*GerritChange, *GerritChange) {
-	write(t, gt.client+"/file1", "")
+	write(t, gt.client+"/file1", "", 0644)
 	trun(t, gt.client, "git", "add", "file1")
 	trun(t, gt.client, "git", "commit", "-m", "msg\n\nChange-Id: I0000001\n")
 	hash1 := strings.TrimSpace(trun(t, gt.client, "git", "log", "-n", "1", "--format=format:%H"))
 
-	write(t, gt.client+"/file2", "")
+	write(t, gt.client+"/file2", "", 0644)
 	trun(t, gt.client, "git", "add", "file2")
 	trun(t, gt.client, "git", "commit", "-m", "msg\n\nChange-Id: I0000002\n")
 	hash2 := strings.TrimSpace(trun(t, gt.client, "git", "log", "-n", "1", "--format=format:%H"))
diff --git a/git-codereview/sync_test.go b/git-codereview/sync_test.go
index 88461b3..a4210ab 100644
--- a/git-codereview/sync_test.go
+++ b/git-codereview/sync_test.go
@@ -13,9 +13,9 @@ func TestSync(t *testing.T) {
 	testMain(t, "change", "work")
 
 	// check for error with unstaged changes
-	write(t, gt.client+"/file1", "")
+	write(t, gt.client+"/file1", "", 0644)
 	trun(t, gt.client, "git", "add", "file1")
-	write(t, gt.client+"/file1", "actual content")
+	write(t, gt.client+"/file1", "actual content", 0644)
 	testMainDied(t, "sync")
 	testPrintedStderr(t, "cannot sync: unstaged changes exist",
 		"git status", "git stash", "git add", "git-codereview change")
@@ -35,7 +35,7 @@ func TestSync(t *testing.T) {
 	testNoStderr(t)
 
 	// make server 1 step ahead of client
-	write(t, gt.server+"/file", "new content")
+	write(t, gt.server+"/file", "new content", 0644)
 	trun(t, gt.server, "git", "add", "file")
 	trun(t, gt.server, "git", "commit", "-m", "msg")
 
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 3f4dce6..8acdb66 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -68,7 +68,8 @@ func (gt *gitTest) done() {
 
 // doWork simulates commit 'n' touching 'file' in 'dir'
 func doWork(t *testing.T, n int, dir, file, changeid string) {
-	write(t, dir+"/"+file, fmt.Sprintf("new content %d", n))
+	t.Helper()
+	write(t, dir+"/"+file, fmt.Sprintf("new content %d", n), 0644)
 	trun(t, dir, "git", "add", file)
 	suffix := ""
 	if n > 1 {
@@ -79,6 +80,7 @@ func doWork(t *testing.T, n int, dir, file, changeid string) {
 }
 
 func (gt *gitTest) work(t *testing.T) {
+	t.Helper()
 	if gt.nwork == 0 {
 		trun(t, gt.client, "git", "checkout", "-b", "work")
 		trun(t, gt.client, "git", "branch", "--set-upstream-to", "origin/master")
@@ -91,12 +93,14 @@ func (gt *gitTest) work(t *testing.T) {
 }
 
 func (gt *gitTest) workFile(t *testing.T, file string) {
+	t.Helper()
 	// make local change on client in the specific file
 	gt.nwork++
 	doWork(t, gt.nwork, gt.client, file, "23456789")
 }
 
 func (gt *gitTest) serverWork(t *testing.T) {
+	t.Helper()
 	// make change on server
 	// duplicating the sequence of changes in gt.work to simulate them
 	// having gone through Gerrit and submitted with possibly
@@ -106,6 +110,7 @@ func (gt *gitTest) serverWork(t *testing.T) {
 }
 
 func (gt *gitTest) serverWorkUnrelated(t *testing.T) {
+	t.Helper()
 	// make unrelated change on server
 	// this makes history different on client and server
 	gt.nworkOther++
@@ -113,6 +118,7 @@ func (gt *gitTest) serverWorkUnrelated(t *testing.T) {
 }
 
 func newGitTest(t *testing.T) (gt *gitTest) {
+	t.Helper()
 	// The Linux builders seem not to have git in their paths.
 	// That makes this whole repo a bit useless on such systems,
 	// but make sure the tests don't fail.
@@ -136,8 +142,8 @@ func newGitTest(t *testing.T) (gt *gitTest) {
 	server := tmpdir + "/git-origin"
 
 	mkdir(t, server)
-	write(t, server+"/file", "this is master")
-	write(t, server+"/.gitattributes", "* -text\n")
+	write(t, server+"/file", "this is master", 0644)
+	write(t, server+"/.gitattributes", "* -text\n", 0644)
 	trun(t, server, "git", "init", ".")
 	trun(t, server, "git", "config", "user.name", "gopher")
 	trun(t, server, "git", "config", "user.email", "gopher@example.com")
@@ -147,7 +153,7 @@ func newGitTest(t *testing.T) (gt *gitTest) {
 	for _, name := range []string{"dev.branch", "release.branch"} {
 		trun(t, server, "git", "checkout", "master")
 		trun(t, server, "git", "checkout", "-b", name)
-		write(t, server+"/file."+name, "this is "+name)
+		write(t, server+"/file."+name, "this is "+name, 0644)
 		trun(t, server, "git", "add", "file."+name)
 		trun(t, server, "git", "commit", "-m", "on "+name)
 	}
@@ -169,7 +175,7 @@ func newGitTest(t *testing.T) (gt *gitTest) {
 		mkdir(t, client+"/.git/hooks")
 	}
 	for _, h := range hookFiles {
-		write(t, client+"/.git/hooks/"+h, "#!/bin/bash\nexit 0\n")
+		write(t, client+"/.git/hooks/"+h, "#!/bin/sh\nexit 0\n", 0755)
 	}
 
 	trun(t, client, "git", "config", "core.editor", "false")
@@ -191,7 +197,8 @@ func newGitTest(t *testing.T) (gt *gitTest) {
 }
 
 func (gt *gitTest) enableGerrit(t *testing.T) {
-	write(t, gt.server+"/codereview.cfg", "gerrit: myserver\n")
+	t.Helper()
+	write(t, gt.server+"/codereview.cfg", "gerrit: myserver\n", 0644)
 	trun(t, gt.server, "git", "add", "codereview.cfg")
 	trun(t, gt.server, "git", "commit", "-m", "add gerrit")
 	trun(t, gt.client, "git", "pull", "-r")
@@ -203,18 +210,21 @@ func (gt *gitTest) removeStubHooks() {
 
 func mkdir(t *testing.T, dir string) {
 	if err := os.Mkdir(dir, 0777); err != nil {
+		t.Helper()
 		t.Fatal(err)
 	}
 }
 
 func chdir(t *testing.T, dir string) {
 	if err := os.Chdir(dir); err != nil {
+		t.Helper()
 		t.Fatal(err)
 	}
 }
 
-func write(t *testing.T, file, data string) {
-	if err := ioutil.WriteFile(file, []byte(data), 0666); err != nil {
+func write(t *testing.T, file, data string, perm os.FileMode) {
+	if err := ioutil.WriteFile(file, []byte(data), perm); err != nil {
+		t.Helper()
 		t.Fatal(err)
 	}
 }
@@ -222,6 +232,7 @@ func write(t *testing.T, file, data string) {
 func read(t *testing.T, file string) []byte {
 	b, err := ioutil.ReadFile(file)
 	if err != nil {
+		t.Helper()
 		t.Fatal(err)
 	}
 	return b
@@ -229,6 +240,7 @@ func read(t *testing.T, file string) []byte {
 
 func remove(t *testing.T, file string) {
 	if err := os.RemoveAll(file); err != nil {
+		t.Helper()
 		t.Fatal(err)
 	}
 }
@@ -239,6 +251,7 @@ func trun(t *testing.T, dir string, cmdline ...string) string {
 	setEnglishLocale(cmd)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
+		t.Helper()
 		if cmdline[0] == "git" {
 			t.Fatalf("in %s/, ran %s with %s:\n%v\n%s", filepath.Base(dir), cmdline, gitversion, err, out)
 		}
@@ -269,6 +282,7 @@ var (
 var mainCanDie bool
 
 func testMainDied(t *testing.T, args ...string) {
+	t.Helper()
 	mainCanDie = true
 	testMain(t, args...)
 	if !died {
@@ -277,6 +291,7 @@ func testMainDied(t *testing.T, args ...string) {
 }
 
 func testMain(t *testing.T, args ...string) {
+	t.Helper()
 	*noRun = false
 	*verbose = 0
 	cachedConfig = nil
@@ -327,6 +342,7 @@ func testRan(t *testing.T, cmds ...string) {
 		cmds = []string{}
 	}
 	if !reflect.DeepEqual(runLog, cmds) {
+		t.Helper()
 		t.Errorf("ran:\n%s", strings.Join(runLog, "\n"))
 		t.Errorf("wanted:\n%s", strings.Join(cmds, "\n"))
 	}
@@ -347,26 +363,31 @@ func testPrinted(t *testing.T, buf *bytes.Buffer, name string, messages ...strin
 		}
 	}
 	if errors.Len() > 0 {
+		t.Helper()
 		t.Fatalf("wrong output\n%s%s:\n%s", &errors, name, all)
 	}
 }
 
 func testPrintedStdout(t *testing.T, messages ...string) {
+	t.Helper()
 	testPrinted(t, testStdout, "stdout", messages...)
 }
 
 func testPrintedStderr(t *testing.T, messages ...string) {
+	t.Helper()
 	testPrinted(t, testStderr, "stderr", messages...)
 }
 
 func testNoStdout(t *testing.T) {
 	if testStdout.Len() != 0 {
+		t.Helper()
 		t.Fatalf("unexpected stdout:\n%s", testStdout)
 	}
 }
 
 func testNoStderr(t *testing.T) {
 	if testStderr.Len() != 0 {
+		t.Helper()
 		t.Fatalf("unexpected stderr:\n%s", testStderr)
 	}
 }
@@ -380,6 +401,7 @@ type gerritServer struct {
 func newGerritServer(t *testing.T) *gerritServer {
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
+		t.Helper()
 		t.Fatalf("starting fake gerrit: %v", err)
 	}