cmd/govim: make testscript scripts truly concurrent

_scripts/runDockerRun.sh

@@ -8,8 +8,9 @@ cd "${BASH_SOURCE%/*}/../"
 
 # The ARTEFACTS variable set by .travis.yml cannot expand
 # variables so we do that here
-ARTEFACTS=$(echo $ARTEFACTS)
+ARTEFACTS=$(echo ${ARTEFACTS:-})
 
+artefacts=""
 proxy=""
 
 if [ "${CI:-}" != "true" ]

cmd/govim/cleanup1.14_test.go

@@ -0,0 +1,11 @@
+// +build go1.14
+
+package main
+
+import "testing"
+
+func cleanup(t *testing.T, f func()) {
+	// TODO when there is a 1.14 release which includes CL 214822 we can
+	// uncomment the next line
+	// t.Cleanup(f)
+}

cmd/govim/cleanup_pre1.14_test.go

@@ -0,0 +1,9 @@
+// +build !go1.14
+
+package main
+
+import "testing"
+
+func cleanup(t *testing.T, f func()) {
+	// This is a no-op pre 1.14
+}

cmd/govim/main_test.go

@@ -63,7 +63,13 @@ func TestScripts(t *testing.T) {
 		if err != nil {
 			t.Fatalf("failed to install gopls to temp directory: %v", err)
 		}
-		defer os.RemoveAll(td)
+		// Note for pre 1.14 this is a no-op. This means that for Go versions pre
+		// 1.14 we leave test artefacts lying around in TMPDIR. This is a price
+		// worth paying given the speedup we achieve by making subtests truly
+		// parallel.
+		cleanup(t, func() {
+			os.RemoveAll(td)
+		})
 		goplsPath = filepath.Join(td, "gopls")
 	}
 	t.Logf("using gopls at %q", goplsPath)
@@ -89,6 +95,7 @@ func TestScripts(t *testing.T) {
 			os.MkdirAll(workdir, 0777)
 		}
 		t.Run(entry.Name(), func(t *testing.T) {
+			t.Parallel()
 			params := testscript.Params{
 				WorkdirRoot: workdir,
 				Dir:         filepath.Join("testdata", entry.Name()),
@@ -98,7 +105,6 @@ func TestScripts(t *testing.T) {
 				},
 				Condition: testdriver.Condition,
 				Setup: func(e *testscript.Env) error {
-					// We set a special TMPDIR so the file watcher ignores it
 					tmp := filepath.Join(e.WorkDir, "_tmp")
 					if err := os.MkdirAll(tmp, 0777); err != nil {
 						return fmt.Errorf("failed to create temp dir %v: %v", tmp, err)
@@ -131,14 +137,14 @@ func TestScripts(t *testing.T) {
 					if workdir == "" {
 						tf, err = ioutil.TempFile(tmp, "govim.log*")
 						if err != nil {
-							t.Fatalf("failed to create govim log file: %v", err)
+							return fmt.Errorf("failed to create govim log file: %v", err)
 						}
 					} else {
 						// create a "plain"-named logfile because as above we set
 						// GOVIM_LOGFILE_TMPL=%v
 						tf, err = os.OpenFile(filepath.Join(tmp, "govim.log"), os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0666)
 						if err != nil {
-							t.Fatalf("failed to create non-tmp govim log file: %v", err)
+							return fmt.Errorf("failed to create non-tmp govim log file: %v", err)
 						}
 					}
 					e.Defer(func() {
@@ -151,7 +157,7 @@ func TestScripts(t *testing.T) {
 					defaultsPath := filepath.Join("testdata", entry.Name(), "default_config.json")
 					defaults, err := readConfig(defaultsPath)
 					if err != nil {
-						t.Fatalf("failed to read defaults from %v: %v", defaultsPath, err)
+						return fmt.Errorf("failed to read defaults from %v: %v", defaultsPath, err)
 					}
 					// We now ensure we have a default for at least CompletionBudget because
 					// unless we are specifically testing the behaviour of that config value
@@ -160,7 +166,7 @@ func TestScripts(t *testing.T) {
 					userPath := filepath.Join("testdata", entry.Name(), "user_config.json")
 					user, err := readConfig(userPath)
 					if err != nil {
-						t.Fatalf("failed to read user from %v: %v", userPath, err)
+						return fmt.Errorf("failed to read user from %v: %v", userPath, err)
 					}
 					if user == nil {
 						user = new(config.Config)
@@ -184,10 +190,10 @@ func TestScripts(t *testing.T) {
 					}
 					td, err := testdriver.NewTestDriver(config)
 					if err != nil {
-						t.Fatalf("failed to create new driver: %v", err)
+						return fmt.Errorf("failed to create new driver: %v", err)
 					}
 					if err := td.Run(); err != nil {
-						t.Fatalf("failed to run TestDriver: %v", err)
+						return fmt.Errorf("failed to run TestDriver: %v", err)
 					}
 					waitLock.Lock()
 					waitList = append(waitList, td.Wait)
@@ -248,16 +254,22 @@ func TestInstallScripts(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to install gopls to temp directory: %v", err)
 	}
-	defer os.RemoveAll(td)
+	cleanup(t, func() {
+		os.RemoveAll(td)
+	})
+
+	gopath := strings.TrimSpace(runCmd(t, "go", "env", "GOPATH"))
+	gocache := strings.TrimSpace(runCmd(t, "go", "env", "GOCACHE"))
 
 	t.Run("scripts", func(t *testing.T) {
+		t.Parallel()
 		testscript.Run(t, testscript.Params{
 			Dir: filepath.Join("testdata", "install"),
 			Setup: func(e *testscript.Env) error {
 				e.Vars = append(e.Vars,
 					"PLUGIN_PATH="+govimPath,
-					"GOPATH="+strings.TrimSpace(runCmd(t, "go", "env", "GOPATH")),
-					"GOCACHE="+strings.TrimSpace(runCmd(t, "go", "env", "GOCACHE")),
+					"GOPATH="+gopath,
+					"GOCACHE="+gocache,
 					testsetup.EnvLoadTestAPI+"=true",
 				)
 				return nil
@@ -266,6 +278,7 @@ func TestInstallScripts(t *testing.T) {
 	})
 
 	t.Run("scripts-with-gopls-from-path", func(t *testing.T) {
+		t.Parallel()
 		testscript.Run(t, testscript.Params{
 			Dir: filepath.Join("testdata", "install"),
 			Setup: func(e *testscript.Env) error {
@@ -285,8 +298,8 @@ func TestInstallScripts(t *testing.T) {
 				e.Vars = append(e.Vars,
 					"PATH="+path,
 					"PLUGIN_PATH="+govimPath,
-					"GOPATH="+strings.TrimSpace(runCmd(t, "go", "env", "GOPATH")),
-					"GOCACHE="+strings.TrimSpace(runCmd(t, "go", "env", "GOCACHE")),
+					"GOPATH="+gopath,
+					"GOCACHE="+gocache,
 					string(config.EnvVarUseGoplsFromPath)+"=true",
 					testsetup.EnvLoadTestAPI+"=true",
 				)

go.sum

@@ -43,6 +43,7 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
 golang.org/x/crypto v0.0.0-20181106171534-e4dc69e5b2fd/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 h1:ObdrDkeb4kJdCP557AjRjq69pTHfNouLtWZG7j9rPN8=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e h1:JgcxKXxCjrA2tyDP/aNU9K0Ck5Czfk6C7e2tMw7+bSI=
 golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc=