Skip to content

Commit

Permalink
cmd/cgo/internal: skip in tests, not in TestMain
Browse files Browse the repository at this point in the history 
Many cgo integration tests do a lot of common setup in TestMain, and
that means they require a lot from the test environment to even get
off the ground. If something is missing, right now they print a "SKIP"
message to stderr and exit without running any tests.

Make these behave more like normal tests by instead setting a global
skip function if some precondition isn't satisfied, and having every
test call that. This way we run the tests and see them skip.

I would prefer something much more structured. For example, if we
replaced the global state set up by TestMain in these tests by instead
calling a function that returned that state (after setting it up on
the first call), that function could do the appropriate skips and
there would be no way to accidentally access this state without
checking the preconditions. But that's substantially more work and may
be much easier after we do further cleanup of these tests.

Change-Id: I92de569fd27596798c5e478402449cd735ec53a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/497096
Run-TryBot: Austin Clements <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
  • Loading branch information
@aclements
aclements committed May 22, 2023
1 parent a1f3dc3 commit 5abfdc8
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 31 deletions.
36 changes: 27 additions & 9 deletions src/cmd/cgo/internal/testcarchive/carchive_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import (
"unicode"
)

var globalSkip = func(t *testing.T) {}

// Program to run.
var bin []string

Expand All @@ -50,22 +52,23 @@ var testWork bool // If true, preserve temporary directories.
func TestMain(m *testing.M) {
flag.BoolVar(&testWork, "testwork", false, "if true, log and preserve the test's temporary working directory")
flag.Parse()

log.SetFlags(log.Lshortfile)
os.Exit(testMain(m))
}

func testMain(m *testing.M) int {
if testing.Short() && os.Getenv("GO_BUILDER_NAME") == "" {
fmt.Printf("SKIP - short mode and $GO_BUILDER_NAME not set\n")
os.Exit(0)
globalSkip = func(t *testing.T) { t.Skip("short mode and $GO_BUILDER_NAME not set") }
return m.Run()
}
if runtime.GOOS == "linux" {
if _, err := os.Stat("/etc/alpine-release"); err == nil {
fmt.Printf("SKIP - skipping failing test on alpine - go.dev/issue/19938\n")
os.Exit(0)
globalSkip = func(t *testing.T) { t.Skip("skipping failing test on alpine - go.dev/issue/19938") }
return m.Run()
}
}

log.SetFlags(log.Lshortfile)
os.Exit(testMain(m))
}

func testMain(m *testing.M) int {
// We need a writable GOPATH in which to run the tests.
// Construct one in a temporary directory.
var err error
Expand Down Expand Up @@ -461,6 +464,7 @@ func checkELFArchiveObject(t *testing.T, arname string, off int64, obj io.Reader
}

func TestInstall(t *testing.T) {
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -506,6 +510,7 @@ func TestEarlySignalHandler(t *testing.T) {
case "windows":
t.Skip("skipping signal test on Windows")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -549,6 +554,7 @@ func TestEarlySignalHandler(t *testing.T) {
}

func TestSignalForwarding(t *testing.T) {
globalSkip(t)
checkSignalForwardingTest(t)
buildSignalForwardingTest(t)

Expand Down Expand Up @@ -577,6 +583,7 @@ func TestSignalForwardingExternal(t *testing.T) {
} else if GOOS == "darwin" && GOARCH == "amd64" {
t.Skipf("skipping on %s/%s: runtime does not permit SI_USER SIGSEGV", GOOS, GOARCH)
}
globalSkip(t)
checkSignalForwardingTest(t)
buildSignalForwardingTest(t)

Expand Down Expand Up @@ -626,6 +633,7 @@ func TestSignalForwardingGo(t *testing.T) {
if runtime.GOOS == "darwin" && runtime.GOARCH == "amd64" {
t.Skip("not supported on darwin-amd64")
}
globalSkip(t)

checkSignalForwardingTest(t)
buildSignalForwardingTest(t)
Expand Down Expand Up @@ -779,6 +787,7 @@ func TestOsSignal(t *testing.T) {
case "windows":
t.Skip("skipping signal test on Windows")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -820,6 +829,7 @@ func TestSigaltstack(t *testing.T) {
case "windows":
t.Skip("skipping signal test on Windows")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -872,6 +882,7 @@ func TestExtar(t *testing.T) {
if runtime.Compiler == "gccgo" {
t.Skip("skipping -extar test when using gccgo")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -918,6 +929,7 @@ func TestPIE(t *testing.T) {
case "windows", "darwin", "ios", "plan9":
t.Skipf("skipping PIE test on %s", GOOS)
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -1015,6 +1027,7 @@ func TestSIGPROF(t *testing.T) {
case "darwin", "ios":
t.Skipf("skipping SIGPROF test on %s; see https://golang.org/issue/19320", GOOS)
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -1064,6 +1077,7 @@ func TestSIGPROF(t *testing.T) {
// will likely do it in the future. And it ought to work. This test
// was added because at one time it did not work on PPC Linux.
func TestCompileWithoutShared(t *testing.T) {
globalSkip(t)
// For simplicity, reuse the signal forwarding test.
checkSignalForwardingTest(t)
testenv.MustHaveGoBuild(t)
Expand Down Expand Up @@ -1131,6 +1145,7 @@ func TestCompileWithoutShared(t *testing.T) {

// Test that installing a second time recreates the header file.
func TestCachedInstall(t *testing.T) {
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -1174,6 +1189,7 @@ func TestCachedInstall(t *testing.T) {

// Issue 35294.
func TestManyCalls(t *testing.T) {
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -1236,6 +1252,7 @@ func TestPreemption(t *testing.T) {
if runtime.Compiler == "gccgo" {
t.Skip("skipping asynchronous preemption test with gccgo")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down Expand Up @@ -1293,6 +1310,7 @@ func TestPreemption(t *testing.T) {
// Issue 59294. Test calling Go function from C after using some
// stack space.
func TestDeepStack(t *testing.T) {
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-archive")
Expand Down
24 changes: 18 additions & 6 deletions src/cmd/cgo/internal/testcshared/cshared_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"unicode"
)

var globalSkip = func(t *testing.T) {}

// C compiler with args (from $(go env CC) $(go env GOGCCFLAGS)).
var cc []string

Expand All @@ -43,19 +45,19 @@ func testMain(m *testing.M) int {
log.SetFlags(log.Lshortfile)
flag.Parse()
if testing.Short() && os.Getenv("GO_BUILDER_NAME") == "" {
fmt.Printf("SKIP - short mode and $GO_BUILDER_NAME not set\n")
os.Exit(0)
globalSkip = func(t *testing.T) { t.Skip("short mode and $GO_BUILDER_NAME not set") }
return m.Run()
}
if runtime.GOOS == "linux" {
if _, err := os.Stat("/etc/alpine-release"); err == nil {
fmt.Printf("SKIP - skipping failing test on alpine - go.dev/issue/19938\n")
os.Exit(0)
globalSkip = func(t *testing.T) { t.Skip("skipping failing test on alpine - go.dev/issue/19938") }
return m.Run()
}
}
if !testenv.HasGoBuild() {
// Checking for "go build" is a proxy for whether or not we can run "go env".
fmt.Printf("SKIP - no go build")
os.Exit(0)
globalSkip = func(t *testing.T) { t.Skip("no go build") }
return m.Run()
}

GOOS = goEnv("GOOS")
Expand Down Expand Up @@ -348,6 +350,7 @@ func createHeadersOnce(t *testing.T) {

// test0: exported symbols in shared lib are accessible.
func TestExportedSymbols(t *testing.T) {
globalSkip(t)
testenv.MustHaveCGO(t)
testenv.MustHaveExec(t)

Expand Down Expand Up @@ -453,6 +456,7 @@ func TestNumberOfExportedFunctions(t *testing.T) {
if GOOS != "windows" {
t.Skip("skipping windows only test")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-shared")
Expand All @@ -472,6 +476,7 @@ func TestExportedSymbolsWithDynamicLoad(t *testing.T) {
if GOOS == "windows" {
t.Skipf("Skipping on %s", GOOS)
}
globalSkip(t)
testenv.MustHaveCGO(t)
testenv.MustHaveExec(t)

Expand Down Expand Up @@ -501,6 +506,7 @@ func TestUnexportedSymbols(t *testing.T) {
if GOOS == "windows" {
t.Skipf("Skipping on %s", GOOS)
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-shared")
Expand Down Expand Up @@ -538,6 +544,7 @@ func TestUnexportedSymbols(t *testing.T) {

// test3: tests main.main is exported on android.
func TestMainExportedOnAndroid(t *testing.T) {
globalSkip(t)
testenv.MustHaveCGO(t)
testenv.MustHaveExec(t)

Expand Down Expand Up @@ -570,6 +577,7 @@ func testSignalHandlers(t *testing.T, pkgname, cfile, cmd string) {
if GOOS == "windows" {
t.Skipf("Skipping on %s", GOOS)
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-shared")
Expand Down Expand Up @@ -619,6 +627,7 @@ func TestPIE(t *testing.T) {
default:
t.Skipf("Skipping on %s", GOOS)
}
globalSkip(t)

t.Parallel()

Expand Down Expand Up @@ -656,6 +665,7 @@ func TestPIE(t *testing.T) {

// Test that installing a second time recreates the header file.
func TestCachedInstall(t *testing.T) {
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-shared")
Expand Down Expand Up @@ -760,6 +770,7 @@ func TestGo2C2Go(t *testing.T) {
case "android":
t.Skip("test fails on android; issue 29087")
}
globalSkip(t)
testenv.MustHaveGoBuild(t)
testenv.MustHaveCGO(t)
testenv.MustHaveBuildMode(t, "c-shared")
Expand Down Expand Up @@ -812,6 +823,7 @@ func TestGo2C2Go(t *testing.T) {
}

func TestIssue36233(t *testing.T) {
globalSkip(t)
testenv.MustHaveCGO(t)

t.Parallel()
Expand Down
Loading

0 comments on commit 5abfdc8

Please sign in to comment.