From 8e7acdbce89da28e6f7d7c27f67be8cd0abaecd7 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Thu, 21 May 2020 15:19:49 -0400 Subject: [PATCH] all: replace build tags in tests with testenv helper Many tool features, particularly modules-related, require particular Go versions. Build tags are unwieldy, requiring one-off test files which break up test organization. Add a suite of testenv functions that check what Go version is in use. Note that this is the logical Go version, as denoted by the release tags; it should be updated at the beginning of the release cycle per issue golang/go#38704. For ease of reviewing, I'll merge/delete files in a followup CL. Change-Id: Id85ce0f83387b3c45d68465161cf88447325d4f2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/234882 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Robert Findley --- go/internal/gcimporter/gcimporter11_test.go | 6 ++-- go/packages/packages110_test.go | 11 ------ go/packages/packages114_test.go | 5 ++- go/packages/packages115_test.go | 13 +++----- go/packages/packagestest/modules_test.go | 4 +-- internal/imports/mod_112_test.go | 5 +-- internal/imports/mod_114_test.go | 5 +-- internal/imports/mod_pre114_test.go | 5 +-- internal/imports/mod_test.go | 8 +++-- internal/imports/proxy_112_test.go | 20 ----------- internal/imports/proxy_113_test.go | 24 -------------- internal/lsp/regtest/diagnostics_114_test.go | 6 ++-- internal/proxydir/proxydir_112.go | 18 ---------- internal/proxydir/proxydir_113.go | 22 ++++++++---- internal/testenv/testenv.go | 35 ++++++++++++++++++++ 15 files changed, 80 insertions(+), 107 deletions(-) delete mode 100644 go/packages/packages110_test.go delete mode 100644 internal/imports/proxy_112_test.go delete mode 100644 internal/imports/proxy_113_test.go delete mode 100644 internal/proxydir/proxydir_112.go diff --git a/go/internal/gcimporter/gcimporter11_test.go b/go/internal/gcimporter/gcimporter11_test.go index 627300d08d..3f2f0a08ff 100644 --- a/go/internal/gcimporter/gcimporter11_test.go +++ b/go/internal/gcimporter/gcimporter11_test.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build go1.11 - package gcimporter import ( @@ -11,6 +9,8 @@ import ( "runtime" "strings" "testing" + + "golang.org/x/tools/internal/testenv" ) var importedObjectTests = []struct { @@ -38,6 +38,7 @@ var importedObjectTests = []struct { } func TestImportedTypes(t *testing.T) { + testenv.NeedsGo1Point(t, 11) skipSpecialPlatforms(t) // This package only handles gc export data. @@ -112,6 +113,7 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) { } } func TestIssue25301(t *testing.T) { + testenv.NeedsGo1Point(t, 11) skipSpecialPlatforms(t) // This package only handles gc export data. diff --git a/go/packages/packages110_test.go b/go/packages/packages110_test.go deleted file mode 100644 index d4005682d1..0000000000 --- a/go/packages/packages110_test.go +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +build !go1.11 - -package packages_test - -func init() { - usesOldGolist = true -} diff --git a/go/packages/packages114_test.go b/go/packages/packages114_test.go index c85fcd7d32..d2d5a904aa 100644 --- a/go/packages/packages114_test.go +++ b/go/packages/packages114_test.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build go1.14 - package packages_test import ( @@ -13,13 +11,14 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/internal/testenv" ) // These tests check fixes that are only available in Go 1.14. -// They can be moved into packages_test.go when we no longer support 1.13. // See golang/go#35973 for more information. func TestInvalidFilesInOverlay(t *testing.T) { packagestest.TestAll(t, testInvalidFilesInOverlay) } func testInvalidFilesInOverlay(t *testing.T, exporter packagestest.Exporter) { + testenv.NeedsGo1Point(t, 14) exported := packagestest.Export(t, exporter, []packagestest.Module{ { Name: "golang.org/fake", diff --git a/go/packages/packages115_test.go b/go/packages/packages115_test.go index 222cd27719..11d8044b54 100644 --- a/go/packages/packages115_test.go +++ b/go/packages/packages115_test.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build go1.15 - package packages_test import ( @@ -16,10 +14,9 @@ import ( ) // TestInvalidFilesInXTest checks the fix for golang/go#37971. -func TestInvalidFilesInXTest(t *testing.T) { - packagestest.TestAll(t, testInvalidFilesInXTest) -} +func TestInvalidFilesInXTest(t *testing.T) { packagestest.TestAll(t, testInvalidFilesInXTest) } func testInvalidFilesInXTest(t *testing.T, exporter packagestest.Exporter) { + testenv.NeedsGo1Point(t, 15) exported := packagestest.Export(t, exporter, []packagestest.Module{ { Name: "golang.org/fake", @@ -44,11 +41,9 @@ func testInvalidFilesInXTest(t *testing.T, exporter packagestest.Exporter) { } } -func TestTypecheckCgo(t *testing.T) { - packagestest.TestAll(t, testTypecheckCgo) -} - +func TestTypecheckCgo(t *testing.T) { packagestest.TestAll(t, testTypecheckCgo) } func testTypecheckCgo(t *testing.T, exporter packagestest.Exporter) { + testenv.NeedsGo1Point(t, 15) testenv.NeedsTool(t, "cgo") const cgo = `package cgo diff --git a/go/packages/packagestest/modules_test.go b/go/packages/packagestest/modules_test.go index 5d13176da1..6f627b1e5b 100644 --- a/go/packages/packagestest/modules_test.go +++ b/go/packages/packagestest/modules_test.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build go1.11 - package packagestest_test import ( @@ -11,9 +9,11 @@ import ( "testing" "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/internal/testenv" ) func TestModulesExport(t *testing.T) { + testenv.NeedsGo1Point(t, 11) exported := packagestest.Export(t, packagestest.Modules, testdata) defer exported.Cleanup() // Check that the cfg contains all the right bits diff --git a/internal/imports/mod_112_test.go b/internal/imports/mod_112_test.go index 106db7c610..23cca56790 100644 --- a/internal/imports/mod_112_test.go +++ b/internal/imports/mod_112_test.go @@ -1,14 +1,15 @@ -// +build go1.12 - package imports import ( "context" "testing" + + "golang.org/x/tools/internal/testenv" ) // Tests that we handle GO111MODULE=on with no go.mod file. See #30855. func TestNoMainModule(t *testing.T) { + testenv.NeedsGo1Point(t, 12) mt := setup(t, ` -- x.go -- package x diff --git a/internal/imports/mod_114_test.go b/internal/imports/mod_114_test.go index 10c1151d1b..2303f47a46 100644 --- a/internal/imports/mod_114_test.go +++ b/internal/imports/mod_114_test.go @@ -1,11 +1,12 @@ -// +build go1.14 - package imports import ( "testing" + + "golang.org/x/tools/internal/testenv" ) func TestModVendorAuto_114(t *testing.T) { + testenv.NeedsGo1Point(t, 14) testModVendorAuto(t, true) } diff --git a/internal/imports/mod_pre114_test.go b/internal/imports/mod_pre114_test.go index 8c307c13ad..4ab7c0f7ec 100644 --- a/internal/imports/mod_pre114_test.go +++ b/internal/imports/mod_pre114_test.go @@ -1,11 +1,12 @@ -// +build !go1.14 - package imports import ( "testing" + + "golang.org/x/tools/internal/testenv" ) func TestModVendorAuto_Pre114(t *testing.T) { + testenv.SkipAfterGo1Point(t, 13) testModVendorAuto(t, false) } diff --git a/internal/imports/mod_test.go b/internal/imports/mod_test.go index 352fd202b5..47868fe0b4 100644 --- a/internal/imports/mod_test.go +++ b/internal/imports/mod_test.go @@ -1,5 +1,3 @@ -// +build go1.11 - package imports import ( @@ -21,6 +19,7 @@ import ( "golang.org/x/mod/module" "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/gopathwalk" + "golang.org/x/tools/internal/proxydir" "golang.org/x/tools/internal/testenv" "golang.org/x/tools/txtar" ) @@ -647,6 +646,7 @@ type modTest struct { // in testdata/mod, along the lines of TestScript in cmd/go. func setup(t *testing.T, main, wd string) *modTest { t.Helper() + testenv.NeedsGo1Point(t, 11) testenv.NeedsTool(t, "go") proxyOnce.Do(func() { @@ -674,7 +674,7 @@ func setup(t *testing.T, main, wd string) *modTest { GOROOT: build.Default.GOROOT, GOPATH: filepath.Join(dir, "gopath"), GO111MODULE: "on", - GOPROXY: proxyDirToURL(proxyDir), + GOPROXY: proxydir.ToURL(proxyDir), GOSUMDB: "off", WorkingDir: filepath.Join(mainDir, wd), GocmdRunner: &gocommand.Runner{}, @@ -834,6 +834,7 @@ import _ "rsc.io/quote" // Tests that crud in the module cache is ignored. func TestInvalidModCache(t *testing.T) { + testenv.NeedsGo1Point(t, 11) dir, err := ioutil.TempDir("", t.Name()) if err != nil { t.Fatal(err) @@ -920,6 +921,7 @@ import _ "rsc.io/quote" } func BenchmarkScanModCache(b *testing.B) { + testenv.NeedsGo1Point(b, 11) env := &ProcessEnv{ GOPATH: build.Default.GOPATH, GOROOT: build.Default.GOROOT, diff --git a/internal/imports/proxy_112_test.go b/internal/imports/proxy_112_test.go deleted file mode 100644 index 732879a157..0000000000 --- a/internal/imports/proxy_112_test.go +++ /dev/null @@ -1,20 +0,0 @@ -// +build !go1.13 - -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package imports - -import "path/filepath" - -// TODO: use proxy functionality in golang.org/x/tools/go/packages/packagestest -// instead of copying it here. - -func proxyDirToURL(dir string) string { - // Prior to go1.13, the Go command on Windows only accepted GOPROXY file URLs - // of the form file://C:/path/to/proxy. This was incorrect: when parsed, "C:" - // is interpreted as the host. See golang.org/issue/6027. This has been - // fixed in go1.13, but we emit the old format for old releases. - return "file://" + filepath.ToSlash(dir) -} diff --git a/internal/imports/proxy_113_test.go b/internal/imports/proxy_113_test.go deleted file mode 100644 index f6f6be5804..0000000000 --- a/internal/imports/proxy_113_test.go +++ /dev/null @@ -1,24 +0,0 @@ -// +build go1.13 - -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package imports - -import ( - "path/filepath" - "strings" -) - -// TODO: use proxy functionality in golang.org/x/tools/go/packages/packagestest -// instead of copying it here. - -func proxyDirToURL(dir string) string { - // file URLs on Windows must start with file:///. See golang.org/issue/6027. - path := filepath.ToSlash(dir) - if !strings.HasPrefix(path, "/") { - path = "/" + path - } - return "file://" + path -} diff --git a/internal/lsp/regtest/diagnostics_114_test.go b/internal/lsp/regtest/diagnostics_114_test.go index 5a8540951b..fbb7cc67dd 100644 --- a/internal/lsp/regtest/diagnostics_114_test.go +++ b/internal/lsp/regtest/diagnostics_114_test.go @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//+build go1.14 - package regtest import ( @@ -12,6 +10,7 @@ import ( "golang.org/x/tools/internal/lsp" "golang.org/x/tools/internal/lsp/fake" "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/testenv" ) const ardanLabsProxy = ` @@ -28,6 +27,7 @@ var ErrHelpWanted error // -modfile flag that is used to provide modfile diagnostics is only available // with 1.14. func Test_Issue38211(t *testing.T) { + testenv.NeedsGo1Point(t, 14) const ardanLabs = ` -- go.mod -- module mod.com @@ -91,6 +91,7 @@ func main() { // Test for golang/go#38207. func TestNewModule_Issue38207(t *testing.T) { + testenv.NeedsGo1Point(t, 14) const emptyFile = ` -- go.mod -- module mod.com @@ -125,6 +126,7 @@ func main() { } func TestNewFileBadImports_Issue36960(t *testing.T) { + testenv.NeedsGo1Point(t, 14) const simplePackage = ` -- go.mod -- module mod.com diff --git a/internal/proxydir/proxydir_112.go b/internal/proxydir/proxydir_112.go deleted file mode 100644 index c87fe1f622..0000000000 --- a/internal/proxydir/proxydir_112.go +++ /dev/null @@ -1,18 +0,0 @@ -// +build !go1.13 - -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package proxydir - -import "path/filepath" - -// ToURL returns the file uri for a proxy directory. -func ToURL(dir string) string { - // Prior to go1.13, the Go command on Windows only accepted GOPROXY file URLs - // of the form file://C:/path/to/proxy. This was incorrect: when parsed, "C:" - // is interpreted as the host. See golang.org/issue/6027. This has been - // fixed in go1.13, but we emit the old format for old releases. - return "file://" + filepath.ToSlash(dir) -} diff --git a/internal/proxydir/proxydir_113.go b/internal/proxydir/proxydir_113.go index ec9925de38..d67544709d 100644 --- a/internal/proxydir/proxydir_113.go +++ b/internal/proxydir/proxydir_113.go @@ -1,5 +1,3 @@ -// +build go1.13 - // Copyright 2020 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. @@ -9,14 +7,24 @@ package proxydir import ( "path/filepath" "strings" + + "golang.org/x/tools/internal/testenv" ) // ToURL returns the file uri for a proxy directory. func ToURL(dir string) string { - // file URLs on Windows must start with file:///. See golang.org/issue/6027. - path := filepath.ToSlash(dir) - if !strings.HasPrefix(path, "/") { - path = "/" + path + if testenv.Go1Point() >= 13 { + // file URLs on Windows must start with file:///. See golang.org/issue/6027. + path := filepath.ToSlash(dir) + if !strings.HasPrefix(path, "/") { + path = "/" + path + } + return "file://" + path + } else { + // Prior to go1.13, the Go command on Windows only accepted GOPROXY file URLs + // of the form file://C:/path/to/proxy. This was incorrect: when parsed, "C:" + // is interpreted as the host. See golang.org/issue/6027. This has been + // fixed in go1.13, but we emit the old format for old releases. + return "file://" + filepath.ToSlash(dir) } - return "file://" + path } diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index fe6afba4e1..ee8ef7199b 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -9,6 +9,7 @@ package testenv import ( "bytes" "fmt" + "go/build" "io/ioutil" "os" "os/exec" @@ -233,3 +234,37 @@ func ExitIfSmallMachine() { os.Exit(0) } } + +// Go1Point returns the x in Go 1.x. +func Go1Point() int { + for i := len(build.Default.ReleaseTags) - 1; i >= 0; i-- { + var version int + if _, err := fmt.Sscanf(build.Default.ReleaseTags[i], "go1.%d", &version); err != nil { + continue + } + return version + } + panic("bad release tags") +} + +// NeedsGo1Point skips t if the Go version used to run the test is older than +// 1.x. +func NeedsGo1Point(t Testing, x int) { + if t, ok := t.(helperer); ok { + t.Helper() + } + if Go1Point() < x { + t.Skipf("running Go version %q is version 1.%d, older than required 1.%d", runtime.Version(), Go1Point(), x) + } +} + +// SkipAfterGo1Point skips t if the Go version used to run the test is newer than +// 1.x. +func SkipAfterGo1Point(t Testing, x int) { + if t, ok := t.(helperer); ok { + t.Helper() + } + if Go1Point() > x { + t.Skipf("running Go version %q is version 1.%d, newer than maximum 1.%d", runtime.Version(), Go1Point(), x) + } +}