x/tools/internal/testenv: remove ExitIfSmallMachine

[bcmills]

Go version

N/A

What operating system and processor architecture are you using (go env)?

N/A

What did you do?

In https://go.dev/cl/192578 (for #32834) I added a testenv.ExitIfSmallMachine helper function that causes the entire test process to exit based on heuristics about the GO_BUILDER_NAME environment variable.

What did you expect to see?

I expected that we would remove ExitIfSmallMachine by either reducing the memory footprint of the x/tools tests or increasing the available memory on the builders.

What did you see instead?

ExitIfSmallMachine has become an attractive nuisance.

• It prevents go test -list from listing the tests in these binaries on affected machines, which may interfere with structured test runners (such as LUCI).
• It is now used in four test binaries, and encodes assumptions about machine size for six of the Go project's builders — but those assumptions do not prevent the same tests from failing if Go users run them on their own machines with similar specs.
• It too easy a tool to reach for when a test fails, and gets in the way of applying more appropriate tools.
  • In particular, it conflates together “slow” and “memory-constrained”, which really ought to be treated separately.

In terms of “more appropriate tools”:

• For tests that are just slow, and for individual test cases that require very large amounts of memory, we should skip the tests if testing.Short() is set.

  • If we don't want to lose coverage in the default TryBot set / LUCI try-set, we could consider either adding longtest builders by default for the x/tools repo, or opting the test back in in short mode based on GO_BUILDER_NAME (and perhaps GOOS/GOARCH). (Builder-based opt-ins are generally much better than opt-outs because they don't cause spurious failures for external users.)

• For tests that require a lot of memory due to running multiple subtests concurrently, we should either adjust the builders to pass a less aggressive -parallel flag to go test, or add explicit semaphores to restrict the concurrency of the test more than what is already implied by that flag.

[findleyr]

It is now used in four test binaries

It is also used in all of the gopls integration and marker tests -- a total of 13 binaries. We can experiment with simply removing this guard, as the tests have gotten faster and smaller since it was originally added. I expect something will fail, but perhaps we can replace it with a more accurate guard (such as skipping GOOS=android, ios).