Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support Go1.21 testing.Testing() function #3686

Open
tiancaiamao opened this issue Sep 8, 2023 · 2 comments · May be fixed by #4096
Open

support Go1.21 testing.Testing() function #3686

tiancaiamao opened this issue Sep 8, 2023 · 2 comments · May be fixed by #4096

Comments

@tiancaiamao
Copy link

tiancaiamao commented Sep 8, 2023

Well, I don't know much about bazel, hope here is the right place for this issue.

What version of rules_go are you using?

http_archive(
    name = "io_bazel_rules_go",
    sha256 = "278b7ff5a826f3dc10f04feaf0b70d48b68748ccd512d7f98bf442077f043fe3",
    urls = [
        "http://bazel-cache.pingcap.net:8080/bazelbuild/rules_go/releases/download/v0.41.0/rules_go-v0.41.0.zip",
        "http://ats.apps.svc/bazelbuild/rules_go/releases/download/v0.41.0/rules_go-v0.41.0.zip",
        "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.41.0/rules_go-v0.41.0.zip",
        "https://github.com/bazelbuild/rules_go/releases/download/v0.41.0/rules_go-v0.41.0.zip",
    ],
)

What version of gazelle are you using?

http_archive(
    name = "bazel_gazelle",
    sha256 = "b8b6d75de6e4bf7c41b7737b183523085f56283f6db929b86c5e7e1f09cf59c9",
    urls = [
        "http://bazel-cache.pingcap.net:8080/bazelbuild/bazel-gazelle/releases/download/v0.31.1/bazel-gazelle-v0.31.1.tar.gz",
        "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.31.1/bazel-gazelle-v0.31.1.tar.gz",
        "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.31.1/bazel-gazelle-v0.31.1.tar.gz",
        "http://ats.apps.svc/bazelbuild/bazel-gazelle/releases/download/v0.31.1/bazel-gazelle-v0.31.1.tar.gz",
    ],
)

What version of Bazel are you using?

bazel version
Bazelisk version: v1.14.0
Build label: 6.3.2
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Tue Aug 8 15:48:33 2023 (1691509713)
Build timestamp: 1691509713
Build timestamp as int: 1691509713

Does this issue reproduce with the latest releases of all the above?

I think so.

What operating system and processor architecture are you using?

linux, x86_64

Any other potentially useful information about your toolchain?

What did you do?

I'm trying to use the testing.Testing() function in pingcap/tidb#46194
And I found some weird phenomenon.
Some tests are stable when I run it on my local environment, using the standard Go.
But they failed to run in our CI environment. Our CI is using bazel...

After debugging a while, I find that it seems testing.Testing() is not working as expected when using bazel.
As you can see from here, https://cs.opensource.google/go/go/+/refs/tags/go1.21.1:src/testing/testing.go;l=656-661

// Testing reports whether the current code is being run in a test.
// This will report true in programs created by "go test",
// false in programs created by "go build".
func Testing() bool {
	return testBinary == "1"
}

This function works by change the link flag when calling "go test"
Grep the go repo source code you can find this:

find . -name '*.go'|xargs grep 'testBinary'
grep: ./go/parser/testdata/issue42951/not_a_file.go: Is a directory
./debug/gosym/pclntab_test.go:  pclinetestBinary string
./debug/gosym/pclntab_test.go:  pclinetestBinary = filepath.Join(pclineTempDir, "pclinetest")
./debug/gosym/pclntab_test.go:  cmd := exec.Command(testenv.GoToolPath(t), "build", "-o", pclinetestBinary)
./debug/gosym/pclntab_test.go:          pclinetestBinary = ""
./debug/gosym/pclntab_test.go:  f, tab := crack(pclinetestBinary, t)
./testing/testing.go:// testBinary is set by cmd/go to "1" if this is a binary built by "go test".
./testing/testing.go:// because this is possible, the compiler will not optimize testBinary
./testing/testing.go:var testBinary = "0"
./testing/testing.go:   return testBinary == "1"
./cmd/go/internal/test/test.go:                 testBinary := testBinaryName(p)
./cmd/go/internal/test/test.go:                 pkgsForBinary[testBinary] = append(pkgsForBinary[testBinary], p)
./cmd/go/internal/test/test.go:         for testBinary, pkgs := range pkgsForBinary {
./cmd/go/internal/test/test.go:                         base.Errorf("cannot write test binary %s for multiple packages:\n%s", testBinary, buf.String())
./cmd/go/internal/test/test.go: testBinary := testBinaryName(p)
./cmd/go/internal/test/test.go: a.Target = testDir + testBinary + cfg.ExeSuffix
./cmd/go/internal/test/test.go:                 if strings.Contains(testBinary, bad) {
./cmd/go/internal/test/test.go:         target := filepath.Join(base.Cwd(), testBinary+cfg.ExeSuffix)
./cmd/go/internal/test/test.go:                                 target = filepath.Join(target, testBinary+cfg.ExeSuffix)
./cmd/go/internal/test/test.go:                                 target = filepath.Join(base.Cwd(), target, testBinary+cfg.ExeSuffix)
./cmd/go/internal/test/test.go:// testBinaryName can be used to create name for test binary executable.
./cmd/go/internal/test/test.go:func testBinaryName(p *load.Package) string {
./cmd/go/internal/load/test.go: ldflags := append(p.Internal.Ldflags, "-X", "testing.testBinary=1")

bazel test do not set the "testing.testBinary" appropriately.
A workaround for this is modify the BUILD.bazel and add x_defs = {"testing.testBinary": "1"}, to the rule go_test,
but maybe a better fix would be in the tools.

What did you expect to see?

Go1.21 testing.Testing() function return true when using bazel test.

What did you see instead?

It get false, the wrong value.

@fmeum
Copy link
Collaborator

fmeum commented Sep 8, 2023

Would you be interested in contributing this feature? We probably just need to populate x_defs in the implementation of go_test.

@paulgmiller
Copy link

Trying to address this with
x_defs = {
# remove when #3686 is fixed?
"testing.testBinary": "1",
},

locally but suprosed nobody else has hit this in a year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants