From fc95c034367ed02580d4bcdf320493dad969adbe Mon Sep 17 00:00:00 2001
From: Peter Weinberger <pjw@google.com>
Date: Sun, 29 Dec 2024 08:18:22 -0500
Subject: [PATCH] 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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
---
 internal/imports/sourcex_test.go    |  2 +-
 internal/modindex/dir_test.go       | 18 +++--------
 internal/modindex/gomodindex/cmd.go |  5 +--
 internal/modindex/index.go          | 48 ++++++++++++++++-------------
 4 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/internal/imports/sourcex_test.go b/internal/imports/sourcex_test.go
index e8a4d537f8f..0a2327ca300 100644
--- a/internal/imports/sourcex_test.go
+++ b/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"),
diff --git a/internal/modindex/dir_test.go b/internal/modindex/dir_test.go
index 6e76f825116..e0919e4c4bf 100644
--- a/internal/modindex/dir_test.go
+++ b/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)
 	}
diff --git a/internal/modindex/gomodindex/cmd.go b/internal/modindex/gomodindex/cmd.go
index 06314826422..4fc0caf400e 100644
--- a/internal/modindex/gomodindex/cmd.go
+++ b/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
diff --git a/internal/modindex/index.go b/internal/modindex/index.go
index 9d1d137db3a..9665356c01b 100644
--- a/internal/modindex/index.go
+++ b/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.