Make more tests parallel and fix dependency on GOPATH and PWD.

- Make checks call each process with specified cwd and GOPATH.
- This permits running tests concurrently with different GOPATH.
- This removes the need to call os.Getwd().
- Enforce wd to be specified to Capture() to make sure there is no regression.

checks/checks.go

@@ -16,7 +16,6 @@ import (
 	"sync"
 
 	"github.com/maruel/pre-commit-go/checks/definitions"
-	"github.com/maruel/pre-commit-go/internal"
 	"github.com/maruel/pre-commit-go/scm"
 )
 
@@ -76,7 +75,7 @@ func (b *build) Run(change scm.Change) error {
 	// building would have to be done in an efficient way by looking at which
 	// package builds what, to not result in a O(n²) algorithm.
 	args := append([]string{"go", "build"}, b.ExtraArgs...)
-	out, _, err := internal.Capture("", nil, append(args, change.Indirect().Packages()...)...)
+	out, _, err := capture(change.Repo(), append(args, change.Indirect().Packages()...)...)
 	if len(out) != 0 {
 		return fmt.Errorf("%s failed: %s", strings.Join(args, " "), out)
 	}
@@ -103,7 +102,7 @@ func (g *gofmt) GetPrerequisites() []definitions.CheckPrerequisite {
 func (g *gofmt) Run(change scm.Change) error {
 	// gofmt doesn't return non-zero even if some files need to be updated.
 	// gofmt accepts files, not packages.
-	out, _, err := internal.Capture("", nil, "gofmt", "-l", "-s", ".")
+	out, _, err := capture(change.Repo(), "gofmt", "-l", "-s", ".")
 	if len(out) != 0 {
 		return fmt.Errorf("these files are improperly formmatted, please run: gofmt -w -s .\n%s", out)
 	}
@@ -137,7 +136,7 @@ func (t *test) Run(change scm.Change) error {
 		go func(testPkg string) {
 			defer wg.Done()
 			args := append([]string{"go", "test"}, t.ExtraArgs...)
-			out, exitCode, _ := internal.Capture("", nil, append(args, testPkg)...)
+			out, exitCode, _ := capture(change.Repo(), append(args, testPkg)...)
 			if exitCode != 0 {
 				errs <- fmt.Errorf("%s failed:\n%s", strings.Join(args, " "), out)
 			}
@@ -171,7 +170,7 @@ func (e *errcheck) GetPrerequisites() []definitions.CheckPrerequisite {
 func (e *errcheck) Run(change scm.Change) error {
 	// errcheck accepts packages, not files.
 	args := []string{"errcheck", "-ignore", e.Ignores}
-	out, _, err := internal.Capture("", nil, append(args, change.Changed().Packages()...)...)
+	out, _, err := capture(change.Repo(), append(args, change.Changed().Packages()...)...)
 	if len(out) != 0 {
 		return fmt.Errorf("%s failed:\n%s", strings.Join(args, " "), out)
 	}
@@ -200,7 +199,7 @@ func (g *goimports) GetPrerequisites() []definitions.CheckPrerequisite {
 func (g *goimports) Run(change scm.Change) error {
 	// goimports accepts files, not packages.
 	// goimports doesn't return non-zero even if some files need to be updated.
-	out, _, err := internal.Capture("", nil, append([]string{"goimports", "-l"}, change.Changed().GoFiles()...)...)
+	out, _, err := capture(change.Repo(), append([]string{"goimports", "-l"}, change.Changed().GoFiles()...)...)
 	if len(out) != 0 {
 		return fmt.Errorf("these files are improperly formmatted, please run: goimports -w <files>\n%s", out)
 	}
@@ -229,7 +228,7 @@ func (g *golint) GetPrerequisites() []definitions.CheckPrerequisite {
 func (g *golint) Run(change scm.Change) error {
 	// golint accepts packages, not files.
 	// golint doesn't return non-zero ever.
-	out, _, _ := internal.Capture("", nil, "golint", "./...")
+	out, _, _ := capture(change.Repo(), "golint", "./...")
 	result := []string{}
 	for _, line := range strings.Split(string(out), "\n") {
 		for _, b := range g.Blacklist {
@@ -264,7 +263,7 @@ func (g *govet) GetPrerequisites() []definitions.CheckPrerequisite {
 func (g *govet) Run(change scm.Change) error {
 	// govet accepts files, not packages.
 	// Ignore the return code since we ignore many errors.
-	out, _, _ := internal.Capture("", nil, "go", "tool", "vet", "-all", ".")
+	out, _, _ := capture(change.Repo(), "go", "tool", "vet", "-all", ".")
 	result := []string{}
 	for _, line := range strings.Split(string(out), "\n") {
 		for _, b := range g.Blacklist {
@@ -302,7 +301,7 @@ func (c *custom) GetPrerequisites() []definitions.CheckPrerequisite {
 func (c *custom) Run(change scm.Change) error {
 	// TODO(maruel): Make what is passed to the command configurable, e.g. one of:
 	// (Changed, Indirect, All) x (GoFiles, Packages, TestPackages)
-	out, exitCode, err := internal.Capture("", nil, c.Command...)
+	out, exitCode, err := capture(change.Repo(), c.Command...)
 	if exitCode != 0 && c.CheckExitCode {
 		return fmt.Errorf("\"%s\" failed with code %d:\n%s", strings.Join(c.Command, " "), exitCode, out)
 	}

checks/checks_test.go

@@ -21,7 +21,7 @@ import (
 
 func TestChecksSuccess(t *testing.T) {
 	// Runs all checks, they should all pass.
-	// Can't run in parallel due to os.Chdir() and os.Setenv().
+	t.Parallel()
 	if testing.Short() {
 		t.SkipNow()
 	}
@@ -32,21 +32,21 @@ func TestChecksSuccess(t *testing.T) {
 			t.Fail()
 		}
 	}()
-	oldGOPATH := os.Getenv("GOPATH")
-	defer func() {
-		ut.ExpectEqual(t, nil, os.Setenv("GOPATH", oldGOPATH))
-	}()
-	ut.AssertEqual(t, nil, os.Setenv("GOPATH", td))
-
-	oldWd, change := setup(t, td, goodFiles)
-	defer func() {
-		ut.ExpectEqual(t, nil, os.Chdir(oldWd))
-	}()
+	change := setup(t, td, goodFiles)
 	for _, name := range getKnownChecks() {
 		c := KnownChecks[name]()
 		if name == "custom" {
-			// Tested separatedly.
-			continue
+			c = &custom{
+				Description: "foo",
+				Command:     []string{"go", "version"},
+				Prerequisites: []definitions.CheckPrerequisite{
+					{
+						HelpCommand:      []string{"go", "version"},
+						ExpectedExitCode: 0,
+						URL:              "example.com.local",
+					},
+				},
+			}
 		}
 		if l, ok := c.(sync.Locker); ok {
 			l.Lock()
@@ -60,7 +60,7 @@ func TestChecksSuccess(t *testing.T) {
 
 func TestChecksFailure(t *testing.T) {
 	// Runs all checks, they should all fail.
-	// Can't run in parallel due to os.Chdir() and os.Setenv().
+	t.Parallel()
 	if testing.Short() {
 		t.SkipNow()
 	}
@@ -71,16 +71,7 @@ func TestChecksFailure(t *testing.T) {
 			t.Fail()
 		}
 	}()
-	oldGOPATH := os.Getenv("GOPATH")
-	defer func() {
-		ut.ExpectEqual(t, nil, os.Setenv("GOPATH", oldGOPATH))
-	}()
-	ut.AssertEqual(t, nil, os.Setenv("GOPATH", td))
-
-	oldWd, change := setup(t, td, badFiles)
-	defer func() {
-		ut.ExpectEqual(t, nil, os.Chdir(oldWd))
-	}()
+	change := setup(t, td, badFiles)
 	for _, name := range getKnownChecks() {
 		c := KnownChecks[name]()
 		// TODO(maruel): Make golint and govet fail. They are not currently working
@@ -119,8 +110,6 @@ func TestCustom(t *testing.T) {
 	}
 	ut.AssertEqual(t, "foo", c.GetDescription())
 	ut.AssertEqual(t, p, c.GetPrerequisites())
-	err := c.Run(nil)
-	ut.AssertEqual(t, nil, err)
 }
 
 // Private stuff.
@@ -195,17 +184,14 @@ func init() {
 	}
 }
 
-func setup(t *testing.T, td string, files map[string]string) (string, scm.Change) {
+func setup(t *testing.T, td string, files map[string]string) scm.Change {
 	fooDir := filepath.Join(td, "src", "foo")
 	ut.AssertEqual(t, nil, os.MkdirAll(fooDir, 0700))
 	for f, c := range files {
 		p := filepath.Join(fooDir, f)
 		ut.AssertEqual(t, nil, os.MkdirAll(filepath.Dir(p), 0700))
 		ut.AssertEqual(t, nil, ioutil.WriteFile(p, []byte(c), 0600))
 	}
-	oldWd, err := os.Getwd()
-	ut.AssertEqual(t, nil, err)
-	ut.AssertEqual(t, nil, os.Chdir(fooDir))
 	_, code, err := internal.Capture(fooDir, nil, "git", "init")
 	ut.AssertEqual(t, 0, code)
 	ut.AssertEqual(t, nil, err)
@@ -215,12 +201,12 @@ func setup(t *testing.T, td string, files map[string]string) (string, scm.Change
 	ut.AssertEqual(t, 0, code)
 	ut.AssertEqual(t, nil, err)
 
-	repo, err := scm.GetRepo(fooDir)
+	repo, err := scm.GetRepo(fooDir, td)
 	ut.AssertEqual(t, nil, err)
 	change, err := repo.Between(scm.Current, scm.GitInitialCommit, nil)
 	ut.AssertEqual(t, nil, err)
 	ut.AssertEqual(t, true, change != nil)
-	return oldWd, change
+	return change
 }
 
 func getKnownChecks() []string {

checks/coverage.go

@@ -131,7 +131,7 @@ func (c *Coverage) RunProfile(change scm.Change) (profile CoverageProfile, err e
 				"-coverprofile", filepath.Join(tmpDir, fmt.Sprintf("test%d.cov", index)),
 				testPkg,
 			}
-			out, exitCode, _ := internal.Capture("", nil, args...)
+			out, exitCode, _ := capture(change.Repo(), args...)
 			if exitCode != 0 {
 				errs <- fmt.Errorf("%s %s failed:\n%s", strings.Join(args, " "), testPkg, out)
 			}
@@ -177,7 +177,7 @@ func (c *Coverage) RunProfile(change scm.Change) (profile CoverageProfile, err e
 	if c.UseCoveralls && IsContinuousIntegration() {
 		// Please send a pull request if the following doesn't work for you on your
 		// favorite CI system.
-		out, _, err2 := internal.Capture("", nil, "goveralls", "-coverprofile", profilePath)
+		out, _, err2 := capture(change.Repo(), "goveralls", "-coverprofile", profilePath)
 		// Don't fail the build.
 		if err2 != nil {
 			fmt.Printf("%s", out)

checks/coverage_test.go

@@ -6,7 +6,6 @@ package checks
 
 import (
 	"io/ioutil"
-	"os"
 	"testing"
 
 	"github.com/maruel/pre-commit-go/checks/definitions"
@@ -15,7 +14,7 @@ import (
 )
 
 func TestCoverage(t *testing.T) {
-	// Can't run in parallel due to os.Chdir() and os.Setenv().
+	t.Parallel()
 	if testing.Short() {
 		t.SkipNow()
 	}
@@ -26,16 +25,7 @@ func TestCoverage(t *testing.T) {
 			t.Fail()
 		}
 	}()
-	oldGOPATH := os.Getenv("GOPATH")
-	defer func() {
-		ut.ExpectEqual(t, nil, os.Setenv("GOPATH", oldGOPATH))
-	}()
-	ut.AssertEqual(t, nil, os.Setenv("GOPATH", td))
-
-	oldWd, change := setup(t, td, coverageFiles)
-	defer func() {
-		ut.ExpectEqual(t, nil, os.Chdir(oldWd))
-	}()
+	change := setup(t, td, coverageFiles)
 
 	c := &Coverage{
 		Global: definitions.CoverageSettings{

checks/go14_test.go

@@ -0,0 +1,18 @@
+// Copyright 2015 Marc-Antoine Ruel. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+// +build !go15
+
+// On pre 1.5, there will be a 2x slow down due to context switches but it will
+// still likely help with our use case.
+// Ref:
+// https://docs.google.com/document/d/1At2Ls5_fhJQ59kDK2DFVhFu3g5mATSXqqV5QrxinasI/preview
+
+package checks
+
+import "runtime"
+
+func init() {
+	runtime.GOMAXPROCS(runtime.NumCPU())
+}

checks/utils.go

@@ -7,6 +7,9 @@ package checks
 import (
 	"os"
 	"strings"
+
+	"github.com/maruel/pre-commit-go/internal"
+	"github.com/maruel/pre-commit-go/scm"
 )
 
 // IsContinuousIntegration returns true if it thinks it's running on a known CI
@@ -45,3 +48,8 @@ func rsplitn(s, sep string, n int) []string {
 	}
 	return items
 }
+
+// capture sets GOPATH.
+func capture(r scm.ReadOnlyRepo, args ...string) (string, int, error) {
+	return internal.Capture(r.Root(), []string{"GOPATH=" + r.GOPATH()}, args...)
+}

cmd/covg/main.go

@@ -29,7 +29,7 @@ func mainImpl() error {
 	if err != nil {
 		return err
 	}
-	repo, err := scm.GetRepo(cwd)
+	repo, err := scm.GetRepo(cwd, "")
 	if err != nil {
 		return err
 	}

internal/utils.go

@@ -31,9 +31,10 @@ func Capture(wd string, env []string, args ...string) (string, int, error) {
 	default:
 		c = exec.Command(args[0], args[1:]...)
 	}
-	if wd != "" {
-		c.Dir = wd
+	if wd == "" {
+		return "", -1, errors.New("wd is required")
 	}
+	c.Dir = wd
 	procEnv := map[string]string{}
 	for _, item := range os.Environ() {
 		items := strings.SplitN(item, "=", 2)

internal/utils_test.go

@@ -15,6 +15,7 @@ import (
 )
 
 func TestCaptureNormal(t *testing.T) {
+	t.Parallel()
 	wd, err := os.Getwd()
 	ut.AssertEqual(t, nil, err)
 	out, code, err := Capture(wd, []string{"FOO=BAR"}, "go", "version")
@@ -24,21 +25,37 @@ func TestCaptureNormal(t *testing.T) {
 }
 
 func TestCaptureEmpty(t *testing.T) {
-	out, code, err := Capture("", nil)
+	t.Parallel()
+	wd, err := os.Getwd()
+	ut.AssertEqual(t, nil, err)
+	out, code, err := Capture(wd, nil)
 	ut.AssertEqual(t, "", out)
 	ut.AssertEqual(t, -1, code)
 	ut.AssertEqual(t, errors.New("no command specified"), err)
 }
 
 func TestCaptureOne(t *testing.T) {
-	_, code, err := Capture("", nil, "go")
+	t.Parallel()
+	wd, err := os.Getwd()
+	ut.AssertEqual(t, nil, err)
+	_, code, err := Capture(wd, nil, "go")
 	ut.AssertEqual(t, 2, code)
 	ut.AssertEqual(t, nil, err)
 }
 
 func TestCaptureMissing(t *testing.T) {
-	out, code, err := Capture("", nil, "program_is_non_existent")
+	t.Parallel()
+	wd, err := os.Getwd()
+	ut.AssertEqual(t, nil, err)
+	out, code, err := Capture(wd, nil, "program_is_non_existent")
 	ut.AssertEqual(t, "", out)
 	ut.AssertEqual(t, -1, code)
 	ut.AssertEqual(t, true, err != nil)
 }
+
+func TestCaptureNoWd(t *testing.T) {
+	t.Parallel()
+	_, code, err := Capture("", nil, "go")
+	ut.AssertEqual(t, -1, code)
+	ut.AssertEqual(t, errors.New("wd is required"), err)
+}

main.go

@@ -659,13 +659,10 @@ func mainImpl() error {
 	if err != nil {
 		return err
 	}
-	repo, err := scm.GetRepo(cwd)
+	repo, err := scm.GetRepo(cwd, "")
 	if err != nil {
 		return err
 	}
-	if err := os.Chdir(repo.Root()); err != nil {
-		return err
-	}
 
 	configPath, config := loadConfig(repo, *configPathFlag)
 	log.Printf("config: %s", configPath)

scm/change.go

@@ -84,7 +84,7 @@ func newChange(r ReadOnlyRepo, files, allFiles, ignorePatterns IgnorePatterns) *
 	root := r.Root()
 	// An error occurs when the repository is not inside GOPATH. Ignore this
 	// error here.
-	pkgName, _ := relToGOPATH(root)
+	pkgName, _ := relToGOPATH(root, r.GOPATH())
 	c := &change{
 		repo:           r,
 		packageName:    pkgName,

scm/change_test.go

@@ -317,6 +317,7 @@ func (d *dummyRepo) Between(recent, old Commit, ignoredPaths IgnorePatterns) (Ch
 	d.t.FailNow()
 	return nil, nil
 }
+func (d *dummyRepo) GOPATH() string { return d.root }
 
 // makeTree creates a temporary directory and creates the files in it.
 //