From 4c0f0985337386e0c0a3aad09251d5ee7f2b145e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 9 Sep 2024 11:04:13 -0700 Subject: [PATCH] internal/testenv: add MustHaveSource, rm HasSrc All the users of HasSrc call t.Skip anyway, so let's move it to testenv. Fix go/build to use MustHaveSource rather than MustHaveGoBuild where appropriate. Change-Id: I052bf96fd5a5780c1930da5b3a52b7a8dbebea46 Reviewed-on: https://go-review.googlesource.com/c/go/+/612057 Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Auto-Submit: Ian Lance Taylor Reviewed-by: Tim King Auto-Submit: Tim King --- src/compress/gzip/issue14937_test.go | 4 +--- src/go/build/build_test.go | 10 +++++----- src/go/build/deps_test.go | 11 ++-------- .../internal/srcimporter/srcimporter_test.go | 20 +++++-------------- src/internal/testenv/testenv.go | 9 +++++---- src/net/http/http_test.go | 4 +--- 6 files changed, 19 insertions(+), 39 deletions(-) diff --git a/src/compress/gzip/issue14937_test.go b/src/compress/gzip/issue14937_test.go index fe0b264f8a..e8f39ed080 100644 --- a/src/compress/gzip/issue14937_test.go +++ b/src/compress/gzip/issue14937_test.go @@ -30,9 +30,7 @@ func TestGZIPFilesHaveZeroMTimes(t *testing.T) { if testenv.Builder() == "" { t.Skip("skipping test on non-builder") } - if !testenv.HasSrc() { - t.Skip("skipping; no GOROOT available") - } + testenv.MustHaveSource(t) goroot, err := filepath.EvalSymlinks(runtime.GOROOT()) if err != nil { diff --git a/src/go/build/build_test.go b/src/go/build/build_test.go index cb2941d097..605fa365dc 100644 --- a/src/go/build/build_test.go +++ b/src/go/build/build_test.go @@ -496,7 +496,7 @@ func TestShellSafety(t *testing.T) { // Want to get a "cannot find package" error when directory for package does not exist. // There should be valid partial information in the returned non-nil *Package. func TestImportDirNotExist(t *testing.T) { - testenv.MustHaveGoBuild(t) // really must just have source + testenv.MustHaveGoBuild(t) // Need 'go list' internally. ctxt := Default emptyDir := t.TempDir() @@ -550,7 +550,7 @@ func TestImportDirNotExist(t *testing.T) { } func TestImportVendor(t *testing.T) { - testenv.MustHaveGoBuild(t) // really must just have source + testenv.MustHaveSource(t) t.Setenv("GO111MODULE", "off") @@ -571,7 +571,7 @@ func TestImportVendor(t *testing.T) { } func BenchmarkImportVendor(b *testing.B) { - testenv.MustHaveGoBuild(b) // really must just have source + testenv.MustHaveSource(b) b.Setenv("GO111MODULE", "off") @@ -592,7 +592,7 @@ func BenchmarkImportVendor(b *testing.B) { } func TestImportVendorFailure(t *testing.T) { - testenv.MustHaveGoBuild(t) // really must just have source + testenv.MustHaveSource(t) t.Setenv("GO111MODULE", "off") @@ -614,7 +614,7 @@ func TestImportVendorFailure(t *testing.T) { } func TestImportVendorParentFailure(t *testing.T) { - testenv.MustHaveGoBuild(t) // really must just have source + testenv.MustHaveSource(t) t.Setenv("GO111MODULE", "off") diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index b6d956596c..40034263cc 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -16,7 +16,6 @@ import ( "io/fs" "os" "path/filepath" - "runtime" "slices" "strings" "testing" @@ -751,11 +750,7 @@ func listStdPkgs(goroot string) ([]string, error) { } func TestDependencies(t *testing.T) { - if !testenv.HasSrc() { - // Tests run in a limited file system and we do not - // provide access to every source file. - t.Skipf("skipping on %s/%s, missing full GOROOT", runtime.GOOS, runtime.GOARCH) - } + testenv.MustHaveSource(t) ctxt := Default all, err := listStdPkgs(ctxt.GOROOT) @@ -859,9 +854,7 @@ func depsPolicy(t *testing.T) *dag.Graph { // TestStdlibLowercase tests that all standard library package names are // lowercase. See Issue 40065. func TestStdlibLowercase(t *testing.T) { - if !testenv.HasSrc() { - t.Skipf("skipping on %s/%s, missing full GOROOT", runtime.GOOS, runtime.GOARCH) - } + testenv.MustHaveSource(t) ctxt := Default all, err := listStdPkgs(ctxt.GOROOT) diff --git a/src/go/internal/srcimporter/srcimporter_test.go b/src/go/internal/srcimporter/srcimporter_test.go index 61ae0c1453..87dfdc75bb 100644 --- a/src/go/internal/srcimporter/srcimporter_test.go +++ b/src/go/internal/srcimporter/srcimporter_test.go @@ -83,9 +83,7 @@ func walkDir(t *testing.T, path string, endTime time.Time) (int, bool) { } func TestImportStdLib(t *testing.T) { - if !testenv.HasSrc() { - t.Skip("no source code available") - } + testenv.MustHaveSource(t) if testing.Short() && testenv.Builder() == "" { t.Skip("skipping in -short mode") @@ -109,9 +107,7 @@ var importedObjectTests = []struct { } func TestImportedTypes(t *testing.T) { - if !testenv.HasSrc() { - t.Skip("no source code available") - } + testenv.MustHaveSource(t) for _, test := range importedObjectTests { i := strings.LastIndex(test.name, ".") @@ -179,9 +175,7 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) { } func TestReimport(t *testing.T) { - if !testenv.HasSrc() { - t.Skip("no source code available") - } + testenv.MustHaveSource(t) // Reimporting a partially imported (incomplete) package is not supported (see issue #19337). // Make sure we recognize the situation and report an error. @@ -195,9 +189,7 @@ func TestReimport(t *testing.T) { } func TestIssue20855(t *testing.T) { - if !testenv.HasSrc() { - t.Skip("no source code available") - } + testenv.MustHaveSource(t) pkg, err := importer.ImportFrom("go/internal/srcimporter/testdata/issue20855", ".", 0) if err == nil || !strings.Contains(err.Error(), "missing function body") { @@ -209,9 +201,7 @@ func TestIssue20855(t *testing.T) { } func testImportPath(t *testing.T, pkgPath string) { - if !testenv.HasSrc() { - t.Skip("no source code available") - } + testenv.MustHaveSource(t) pkgName := path.Base(pkgPath) diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go index e07e71a9b2..9aecfaa695 100644 --- a/src/internal/testenv/testenv.go +++ b/src/internal/testenv/testenv.go @@ -268,13 +268,14 @@ var goTool = sync.OnceValues(func() (string, error) { return exec.LookPath("go") }) -// HasSrc reports whether the entire source tree is available under GOROOT. -func HasSrc() bool { +// MustHaveSource checks that the entire source tree is available under GOROOT. +// If not, it calls t.Skip with an explanation. +func MustHaveSource(t testing.TB) { switch runtime.GOOS { case "ios": - return false + t.Helper() + t.Skip("skipping test: no source tree on " + runtime.GOOS) } - return true } // HasExternalNetwork reports whether the current system can use diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go index df9812fc94..777634bbb2 100644 --- a/src/net/http/http_test.go +++ b/src/net/http/http_test.go @@ -151,9 +151,7 @@ var forbiddenStringsFunctions = map[string]bool{ // strings and bytes package functions. HTTP is mostly ASCII based, and doing // Unicode-aware case folding or space stripping can introduce vulnerabilities. func TestNoUnicodeStrings(t *testing.T) { - if !testenv.HasSrc() { - t.Skip("source code not available") - } + testenv.MustHaveSource(t) re := regexp.MustCompile(`(strings|bytes).([A-Za-z]+)`) if err := fs.WalkDir(os.DirFS("."), ".", func(path string, d fs.DirEntry, err error) error { -- 2.48.1