From e3b3c0100d02d580162d63ee52e0ea505b2ea233 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 11 Nov 2022 09:51:39 -0500 Subject: [PATCH] go/pointer: break TestInput into subtests and update skips TestInput is fiendishly slow and memory-hungry. Breaking it into subtests reveals that the vast majority of that cost can be attributed to a single test case: "testdata/a_test.go". Update the address-space-based skip to skip only that one input, skip it under the race detector (it is timing out on the new "linux-amd64-longtest-race" builder), and run the remaining inputs in parallel (to reduce test latency now that we've better identified the culprit). Updates golang/go#14113. Updates golang/go#54630. Change-Id: I105cfa173edc74692eaa41efd50ae08eeacf0d7d Reviewed-on: https://go-review.googlesource.com/c/tools/+/449518 TryBot-Result: Gopher Robot gopls-CI: kokoro Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills Reviewed-by: Alan Donovan --- go/pointer/pointer_race_test.go | 12 +++++++++ go/pointer/pointer_test.go | 47 ++++++++++++++++++++------------- 2 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 go/pointer/pointer_race_test.go diff --git a/go/pointer/pointer_race_test.go b/go/pointer/pointer_race_test.go new file mode 100644 index 0000000000..d3c9b475e2 --- /dev/null +++ b/go/pointer/pointer_race_test.go @@ -0,0 +1,12 @@ +// Copyright 2022 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. + +//go:build race +// +build race + +package pointer_test + +func init() { + raceEnabled = true +} diff --git a/go/pointer/pointer_test.go b/go/pointer/pointer_test.go index 47074f620f..05fe981f86 100644 --- a/go/pointer/pointer_test.go +++ b/go/pointer/pointer_test.go @@ -66,6 +66,8 @@ var inputs = []string{ // "testdata/timer.go", // TODO(adonovan): fix broken assumptions about runtime timers } +var raceEnabled = false + // Expectation grammar: // // @calls f -> g @@ -609,10 +611,6 @@ func TestInput(t *testing.T) { if testing.Short() { t.Skip("skipping in short mode; this test requires tons of memory; https://golang.org/issue/14113") } - if unsafe.Sizeof(unsafe.Pointer(nil)) <= 4 { - t.Skip("skipping memory-intensive test on platform with small address space; https://golang.org/issue/14113") - } - ok := true wd, err := os.Getwd() if err != nil { @@ -627,23 +625,34 @@ func TestInput(t *testing.T) { fmt.Fprintf(os.Stderr, "Entering directory `%s'\n", wd) for _, filename := range inputs { - content, err := ioutil.ReadFile(filename) - if err != nil { - t.Errorf("couldn't read file '%s': %s", filename, err) - continue - } + filename := filename + t.Run(filename, func(t *testing.T) { + if filename == "testdata/a_test.go" { + // For some reason this particular file is way more expensive than the others. + if unsafe.Sizeof(unsafe.Pointer(nil)) <= 4 { + t.Skip("skipping memory-intensive test on platform with small address space; https://golang.org/issue/14113") + } + if raceEnabled { + t.Skip("skipping memory-intensive test under race detector; https://golang.org/issue/14113") + } + } else { + t.Parallel() + } - fpath, err := filepath.Abs(filename) - if err != nil { - t.Errorf("couldn't get absolute path for '%s': %s", filename, err) - } + content, err := ioutil.ReadFile(filename) + if err != nil { + t.Fatalf("couldn't read file '%s': %s", filename, err) + } - if !doOneInput(t, string(content), fpath) { - ok = false - } - } - if !ok { - t.Fail() + fpath, err := filepath.Abs(filename) + if err != nil { + t.Fatalf("couldn't get absolute path for '%s': %s", filename, err) + } + + if !doOneInput(t, string(content), fpath) { + t.Fail() + } + }) } }