From c79c01b1c51c468123f4890fb7190cda24a42eb2 Mon Sep 17 00:00:00 2001 From: Heschi Kreinick Date: Thu, 14 May 2020 22:28:26 -0400 Subject: [PATCH] all: consolidate cgo requirement checks Many tools test check for the ability to compile cgo programs. Consolidate them all into testenv.NeedsTool("cgo"). Change-Id: I62c96e7b4dc72df34b8fdbf10326c7d19e0613e8 Reviewed-on: https://go-review.googlesource.com/c/tools/+/234108 Run-TryBot: Heschi Kreinick TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- cmd/gorename/cgo_test.go | 11 ----------- cmd/gorename/gorename_test.go | 6 +----- go/packages/packages115_test.go | 7 ++----- go/packages/packages_test.go | 22 ---------------------- go/packages/packagescgo_test.go | 12 ++---------- internal/lsp/regtest/cgo_test.go | 11 ++++------- internal/lsp/tests/tests.go | 13 ++++++------- internal/lsp/tests/tests_cgo.go | 7 ------- internal/testenv/testenv.go | 13 +++++++++++++ 9 files changed, 28 insertions(+), 74 deletions(-) delete mode 100644 cmd/gorename/cgo_test.go delete mode 100644 internal/lsp/tests/tests_cgo.go diff --git a/cmd/gorename/cgo_test.go b/cmd/gorename/cgo_test.go deleted file mode 100644 index fd07553fc6..0000000000 --- a/cmd/gorename/cgo_test.go +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright 2017 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 cgo - -package main_test - -func init() { - haveCGO = true -} diff --git a/cmd/gorename/gorename_test.go b/cmd/gorename/gorename_test.go index 3351977073..292805193e 100644 --- a/cmd/gorename/gorename_test.go +++ b/cmd/gorename/gorename_test.go @@ -18,8 +18,6 @@ import ( "golang.org/x/tools/internal/testenv" ) -var haveCGO bool - type test struct { offset, from, to string // specify the arguments fileSpecified bool // true if the offset or from args specify a specific file @@ -31,10 +29,8 @@ type test struct { // Test that renaming that would modify cgo files will produce an error and not modify the file. func TestGeneratedFiles(t *testing.T) { - if !haveCGO { - t.Skipf("skipping test: no cgo") - } testenv.NeedsTool(t, "go") + testenv.NeedsTool(t, "cgo") tmp, bin, cleanup := buildGorename(t) defer cleanup() diff --git a/go/packages/packages115_test.go b/go/packages/packages115_test.go index 3a3187efa0..222cd27719 100644 --- a/go/packages/packages115_test.go +++ b/go/packages/packages115_test.go @@ -12,6 +12,7 @@ import ( "golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/internal/testenv" ) // TestInvalidFilesInXTest checks the fix for golang/go#37971. @@ -48,11 +49,7 @@ func TestTypecheckCgo(t *testing.T) { } func testTypecheckCgo(t *testing.T, exporter packagestest.Exporter) { - // The android builders have a complex setup which causes this test to fail. See discussion on - // golang.org/cl/214943 for more details. - if !hasGoBuild() { - t.Skip("this test can't run on platforms without go build. See discussion on golang.org/cl/214943 for more details.") - } + testenv.NeedsTool(t, "cgo") const cgo = `package cgo import "C" diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go index 15aac9cd53..f3bf68a9a0 100644 --- a/go/packages/packages_test.go +++ b/go/packages/packages_test.go @@ -2768,25 +2768,3 @@ func copyAll(srcPath, dstPath string) error { return nil }) } - -// Stolen from internal/testenv package in core. -// hasGoBuild reports whether the current system can build programs with ``go build'' -// and then run them with os.StartProcess or exec.Command. -func hasGoBuild() bool { - if os.Getenv("GO_GCFLAGS") != "" { - // It's too much work to require every caller of the go command - // to pass along "-gcflags="+os.Getenv("GO_GCFLAGS"). - // For now, if $GO_GCFLAGS is set, report that we simply can't - // run go build. - return false - } - switch runtime.GOOS { - case "android", "js": - return false - case "darwin": - if strings.HasPrefix(runtime.GOARCH, "arm") { - return false - } - } - return true -} diff --git a/go/packages/packagescgo_test.go b/go/packages/packagescgo_test.go index ba4e9d7478..c2879497de 100644 --- a/go/packages/packagescgo_test.go +++ b/go/packages/packagescgo_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 cgo - package packages_test import ( @@ -73,11 +71,7 @@ func TestCgoNoSyntax(t *testing.T) { packagestest.TestAll(t, testCgoNoSyntax) } func testCgoNoSyntax(t *testing.T, exporter packagestest.Exporter) { - // The android builders have a complex setup which causes this test to fail. See discussion on - // golang.org/cl/214943 for more details. - if !hasGoBuild() { - t.Skip("this test can't run on platforms without go build. See discussion on golang.org/cl/214943 for more details.") - } + testenv.NeedsTool(t, "cgo") exported := packagestest.Export(t, exporter, []packagestest.Module{{ Name: "golang.org/fake", @@ -118,9 +112,7 @@ func TestCgoBadPkgConfig(t *testing.T) { packagestest.TestAll(t, testCgoBadPkgConfig) } func testCgoBadPkgConfig(t *testing.T, exporter packagestest.Exporter) { - if !hasGoBuild() { - t.Skip("this test can't run on platforms without go build. See discussion on golang.org/cl/214943 for more details.") - } + testenv.NeedsTool(t, "cgo") exported := packagestest.Export(t, exporter, []packagestest.Module{{ Name: "golang.org/fake", diff --git a/internal/lsp/regtest/cgo_test.go b/internal/lsp/regtest/cgo_test.go index 1761ab648f..84de603030 100644 --- a/internal/lsp/regtest/cgo_test.go +++ b/internal/lsp/regtest/cgo_test.go @@ -1,21 +1,18 @@ -//+build go1.15,cgo +//+build go1.15 package regtest import ( - "runtime" "testing" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/testenv" ) func TestRegenerateCgo(t *testing.T) { - // The android builders have a complex setup which causes this test to fail. See discussion on - // golang.org/cl/214943 for more details. - if runtime.GOOS == "android" { - t.Skip("android not supported") - } + testenv.NeedsTool(t, "cgo") + const workspace = ` -- go.mod -- module example.com diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 02619d70da..14901fa1e8 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -16,7 +16,6 @@ import ( "os" "path/filepath" "regexp" - "runtime" "sort" "strconv" "strings" @@ -30,6 +29,7 @@ import ( "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" + "golang.org/x/tools/internal/testenv" "golang.org/x/tools/txtar" ) @@ -231,8 +231,7 @@ func DefaultOptions() source.Options { } var ( - haveCgo = false - go115 = false + go115 = false ) // Load creates the folder structure required when testing with modules. @@ -453,8 +452,8 @@ func Run(t *testing.T, tests Tests, data *Data) { for i, e := range exp { t.Run(SpanName(src)+"_"+strconv.Itoa(i), func(t *testing.T) { t.Helper() - if (!haveCgo || runtime.GOOS == "android") && strings.Contains(t.Name(), "cgo") { - t.Skip("test requires cgo, not supported") + if strings.Contains(t.Name(), "cgo") { + testenv.NeedsTool(t, "cgo") } if !go115 && strings.Contains(t.Name(), "declarecgo") { t.Skip("test requires Go 1.15") @@ -614,8 +613,8 @@ func Run(t *testing.T, tests Tests, data *Data) { for spn, d := range data.Definitions { t.Run(SpanName(spn), func(t *testing.T) { t.Helper() - if (!haveCgo || runtime.GOOS == "android") && strings.Contains(t.Name(), "cgo") { - t.Skip("test requires cgo, not supported") + if strings.Contains(t.Name(), "cgo") { + testenv.NeedsTool(t, "cgo") } if !go115 && strings.Contains(t.Name(), "declarecgo") { t.Skip("test requires Go 1.15") diff --git a/internal/lsp/tests/tests_cgo.go b/internal/lsp/tests/tests_cgo.go deleted file mode 100644 index 0f823a59e4..0000000000 --- a/internal/lsp/tests/tests_cgo.go +++ /dev/null @@ -1,7 +0,0 @@ -// +build cgo - -package tests - -func init() { - haveCgo = true -} diff --git a/internal/testenv/testenv.go b/internal/testenv/testenv.go index 6c5fb98c45..a48f67c126 100644 --- a/internal/testenv/testenv.go +++ b/internal/testenv/testenv.go @@ -40,6 +40,18 @@ var checkGoGoroot struct { } func hasTool(tool string) error { + if tool == "cgo" { + out, err := exec.Command("go", "env", "CGO_ENABLED").CombinedOutput() + if err != nil { + return err + } + enabled := strings.TrimSpace(string(out)) + if enabled != "1" { + return fmt.Errorf("cgo not enabled: CGO_ENABLED=%q", enabled) + } + return nil + } + _, err := exec.LookPath(tool) if err != nil { return err @@ -125,6 +137,7 @@ func allowMissingTool(tool string) bool { } // NeedsTool skips t if the named tool is not present in the path. +// As a special case, "cgo" means "go" is present and can compile cgo programs. func NeedsTool(t Testing, tool string) { if t, ok := t.(helperer); ok { t.Helper()