internal/modindex: fix index directory for tests

The module cache index is written to a subdirectory
of os.UserCacheDir(). In non-test circumstances this
is the right choice, as it makes the index generally
available.

In tests this is the wrong choice, as it may leave test remnants behind.

This change sets the directory to a subdir  os.TempDir for tests.
Tests are free to further change modindex.IndexDir.

Change-Id: I046e0520adf3e3ffa71cfc8a0dfbdd060e41eb1a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/639176
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

internal/imports/sourcex_test.go

@@ -93,7 +93,7 @@ type dirs struct {
 func testDirs(t *testing.T) dirs {
 	t.Helper()
 	dir := t.TempDir()
-	modindex.IndexDir = func() (string, error) { return dir, nil }
+	modindex.IndexDir = dir
 	x := dirs{
 		tmpdir:   dir,
 		cachedir: filepath.Join(dir, "pkg", "mod"),

internal/modindex/dir_test.go

@@ -47,10 +47,8 @@ var idtests = []id{
 }
 
 func testModCache(t *testing.T) string {
-	t.Helper()
-	dir := t.TempDir()
-	IndexDir = func() (string, error) { return dir, nil }
-	return dir
+	IndexDir = t.TempDir()
+	return IndexDir
 }
 
 // add a trivial package to the test module cache
@@ -211,11 +209,7 @@ func TestMissingCachedir(t *testing.T) {
 	if err := Create(dir); err != nil {
 		t.Fatal(err)
 	}
-	ixd, err := IndexDir()
-	if err != nil {
-		t.Fatal(err)
-	}
-	des, err := os.ReadDir(ixd)
+	des, err := os.ReadDir(IndexDir)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -232,11 +226,7 @@ func TestMissingIndex(t *testing.T) {
 	} else if !ok {
 		t.Error("Update returned !ok")
 	}
-	ixd, err := IndexDir()
-	if err != nil {
-		t.Fatal(err)
-	}
-	des, err := os.ReadDir(ixd)
+	des, err := os.ReadDir(IndexDir)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/modindex/gomodindex/cmd.go

@@ -93,10 +93,7 @@ func query(dir string) {
 	panic("implement")
 }
 func clean(_ string) {
-	des, err := modindex.IndexDir()
-	if err != nil {
-		log.Fatal(err)
-	}
+	des := modindex.IndexDir
 	// look at the files starting with 'index'
 	// the current ones of each version are pointed to by
 	// index-name-%d files. Any others more than an hour old

internal/modindex/index.go

@@ -17,6 +17,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"testing"
 	"time"
 )
 
@@ -85,6 +86,28 @@ type Entry struct {
 	Names []string // exported names and information
 }
 
+// IndexDir is where the module index is stored.
+var IndexDir string
+
+// Set IndexDir
+func init() {
+	var dir string
+	var err error
+	if testing.Testing() {
+		dir = os.TempDir()
+	} else {
+		dir, err = os.UserCacheDir()
+		// shouldn't happen, but TempDir is better than
+		// creating ./go/imports
+		if err != nil {
+			dir = os.TempDir()
+		}
+	}
+	dir = filepath.Join(dir, "go", "imports")
+	os.MkdirAll(dir, 0777)
+	IndexDir = dir
+}
+
 // ReadIndex reads the latest version of the on-disk index
 // for the cache directory cd.
 // It returns (nil, nil) if there is no index, but returns
@@ -95,10 +118,7 @@ func ReadIndex(cachedir string) (*Index, error) {
 		return nil, err
 	}
 	cd := Abspath(cachedir)
-	dir, err := IndexDir()
-	if err != nil {
-		return nil, err
-	}
+	dir := IndexDir
 	base := indexNameBase(cd)
 	iname := filepath.Join(dir, base)
 	buf, err := os.ReadFile(iname)
@@ -185,12 +205,8 @@ func readIndexFrom(cd Abspath, bx io.Reader) (*Index, error) {
 
 // write the index as a text file
 func writeIndex(cachedir Abspath, ix *Index) error {
-	dir, err := IndexDir()
-	if err != nil {
-		return err
-	}
 	ipat := fmt.Sprintf("index-%d-*", CurrentVersion)
-	fd, err := os.CreateTemp(dir, ipat)
+	fd, err := os.CreateTemp(IndexDir, ipat)
 	if err != nil {
 		return err // can this happen?
 	}
@@ -201,7 +217,7 @@ func writeIndex(cachedir Abspath, ix *Index) error {
 	content := fd.Name()
 	content = filepath.Base(content)
 	base := indexNameBase(cachedir)
-	nm := filepath.Join(dir, base)
+	nm := filepath.Join(IndexDir, base)
 	err = os.WriteFile(nm, []byte(content), 0666)
 	if err != nil {
 		return err
@@ -241,18 +257,6 @@ func writeIndexToFile(x *Index, fd *os.File) error {
 	return nil
 }
 
-// tests can override this
-var IndexDir = indexDir
-
-// indexDir computes the directory containing the index
-func indexDir() (string, error) {
-	dir, err := os.UserCacheDir()
-	if err != nil {
-		return "", fmt.Errorf("cannot open UserCacheDir, %w", err)
-	}
-	return filepath.Join(dir, "go", "imports"), nil
-}
-
 // return the base name of the file containing the name of the current index
 func indexNameBase(cachedir Abspath) string {
 	// crc64 is a way to convert path names into 16 hex digits.