From 6e4fdcfeb1a333b54ab39ae3413d4ded46d8958d Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Dec 2024 17:27:18 -0500 Subject: [PATCH] Support the go1.21 `testing.Testing()` function (#4190) **What type of PR is this?** Feature **What does this PR do? Why is it needed?** This change adds in the necessary flags in `go test` to allow [`testing.Testing`] to work as expected. According to the [upstream source], it is set with a `-X` option in the `go` tooling, so this change makes `rules_go` operate accordingly. This should be safe for older Go versions too, given that the link command [docs](https://pkg.go.dev/cmd/link) say that the `-X` arg is not "effective" when the target variable is not defined in the code. Tests are also provided, and `buildifier` moved around some unrelated lines. I'd be happy to undo the unrelated changes. [`testing.Testing`]: https://pkg.go.dev/testing#Testing [upstream source]: https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/testing/testing.go;l=647-661 **Which issues(s) does this PR fix?** Fixes #3686. Closes #4096 **Other notes for review** This is intended to replace #4096, which seems to be inactive. The functional code change in this PR was done at the same location. --- go/private/rules/test.bzl | 6 ++++++ tests/core/go_binary/BUILD.bazel | 24 +++++++++++++++++++++- tests/core/go_binary/testing_testing.go | 19 +++++++++++++++++ tests/core/go_test/BUILD.bazel | 8 +++++++- tests/core/go_test/testing_testing_test.go | 11 ++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 tests/core/go_binary/testing_testing.go create mode 100644 tests/core/go_test/testing_testing_test.go diff --git a/go/private/rules/test.bzl b/go/private/rules/test.bzl index 4ae32b82f..07bf2a6c9 100644 --- a/go/private/rules/test.bzl +++ b/go/private/rules/test.bzl @@ -155,6 +155,12 @@ def _go_test_impl(ctx): # in bzltestutil/init.go. test_gc_linkopts.extend(["-X", "+initfirst/github.com/bazelbuild/rules_go/go/tools/bzltestutil/chdir.RunDir=" + run_dir]) + # This is needed for the testing.Testing() function to work in go + # 1.21+. See + # https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/testing/testing.go;l=647-661 + # for more details. + test_gc_linkopts.extend(["-X", "testing.testBinary=1"]) + # Now compile the test binary itself test_deps = external_archive.direct + [external_archive] + ctx.attr._testmain_additional_deps if go.coverage_enabled: diff --git a/tests/core/go_binary/BUILD.bazel b/tests/core/go_binary/BUILD.bazel index d57f6cfbe..d43786835 100644 --- a/tests/core/go_binary/BUILD.bazel +++ b/tests/core/go_binary/BUILD.bazel @@ -1,3 +1,5 @@ +load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@bazel_skylib//rules:run_binary.bzl", "run_binary") load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test") load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test") load(":linkmode.bzl", "linkmode_pie_wrapper") @@ -241,7 +243,27 @@ go_binary( go_binary( name = "meaning2", srcs = [ - "//tests/core/go_library:use_syso_srcs", "meaning2.go", + "//tests/core/go_library:use_syso_srcs", ], ) + +# These three verify that testing.Testing() returns `false` in a +# `go_binary`. +go_binary( + name = "testing_testing_bin", + srcs = ["testing_testing.go"], +) + +run_binary( + name = "testing_testing_bin_run", + outs = ["testing_testing_bin_run.out"], + args = ["$(location :testing_testing_bin_run.out)"], + tags = ["manual"], + tool = ":testing_testing_bin", +) + +build_test( + name = "testing_testing_test", + targets = [":testing_testing_bin_run"], +) diff --git a/tests/core/go_binary/testing_testing.go b/tests/core/go_binary/testing_testing.go new file mode 100644 index 000000000..b2d3a1818 --- /dev/null +++ b/tests/core/go_binary/testing_testing.go @@ -0,0 +1,19 @@ +package main + +import ( + "fmt" + "testing" + "os" +) + +func main() { + if testing.Testing() { + panic("testing.Testing() returned 'true' in a binary") + } + + file, err := os.Create(os.Args[1]) + if err != nil { + panic(fmt.Sprintf("Failed to open output file %s", err)) + } + file.Close() +} diff --git a/tests/core/go_test/BUILD.bazel b/tests/core/go_test/BUILD.bazel index 506d75c77..f743e3e46 100644 --- a/tests/core/go_test/BUILD.bazel +++ b/tests/core/go_test/BUILD.bazel @@ -208,6 +208,12 @@ test_suite( tests = ["same_package_{}_test".format(i) for i in range(1, 80)], ) +# Verifies that testing.Testing() is true in a `go_test`. +go_test( + name = "testing_testing_test", + srcs = ["testing_testing_test.go"], +) + go_bazel_test( name = "testmain_without_exit_test", srcs = ["testmain_without_exit_test.go"], @@ -289,7 +295,7 @@ go_test( go_test( name = "syso_direct_test", srcs = [ - "//tests/core/go_library:use_syso_srcs", "syso_direct_test.go", + "//tests/core/go_library:use_syso_srcs", ], ) diff --git a/tests/core/go_test/testing_testing_test.go b/tests/core/go_test/testing_testing_test.go new file mode 100644 index 000000000..775a13709 --- /dev/null +++ b/tests/core/go_test/testing_testing_test.go @@ -0,0 +1,11 @@ +package testing_testing + +import ( + "testing" +) + +func TestMain(m *testing.M) { + if ! testing.Testing() { + panic("testing.Testing() returned 'false' in a test") + } +}