Skip to content

Commit

Permalink
all: replace build tags in tests with testenv helper
Browse files Browse the repository at this point in the history
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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
  • Loading branch information
heschi committed May 27, 2020
1 parent 7527cb2 commit 8e7acdb
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 107 deletions.
6 changes: 4 additions & 2 deletions go/internal/gcimporter/gcimporter11_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
// 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 (
"go/types"
"runtime"
"strings"
"testing"

"golang.org/x/tools/internal/testenv"
)

var importedObjectTests = []struct {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 0 additions & 11 deletions go/packages/packages110_test.go

This file was deleted.

5 changes: 2 additions & 3 deletions go/packages/packages114_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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",
Expand Down
13 changes: 4 additions & 9 deletions go/packages/packages115_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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",
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go/packages/packagestest/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
// 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 (
"path/filepath"
"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
Expand Down
5 changes: 3 additions & 2 deletions internal/imports/mod_112_test.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down
5 changes: 3 additions & 2 deletions internal/imports/mod_114_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
5 changes: 3 additions & 2 deletions internal/imports/mod_pre114_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
8 changes: 5 additions & 3 deletions internal/imports/mod_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build go1.11

package imports

import (
Expand All @@ -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"
)
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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{},
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 0 additions & 20 deletions internal/imports/proxy_112_test.go

This file was deleted.

24 changes: 0 additions & 24 deletions internal/imports/proxy_113_test.go

This file was deleted.

6 changes: 4 additions & 2 deletions internal/lsp/regtest/diagnostics_114_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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 = `
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -125,6 +126,7 @@ func main() {
}

func TestNewFileBadImports_Issue36960(t *testing.T) {
testenv.NeedsGo1Point(t, 14)
const simplePackage = `
-- go.mod --
module mod.com
Expand Down
18 changes: 0 additions & 18 deletions internal/proxydir/proxydir_112.go

This file was deleted.

22 changes: 15 additions & 7 deletions internal/proxydir/proxydir_113.go
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
}
Loading

0 comments on commit 8e7acdb

Please sign in to comment.