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

source.CallExprArgs fails to open target source file when run under bazel #274

Closed
cstrahan opened this issue Sep 11, 2023 · 3 comments · Fixed by #276
Closed

source.CallExprArgs fails to open target source file when run under bazel #274

cstrahan opened this issue Sep 11, 2023 · 3 comments · Fixed by #276
Labels

Comments

@cstrahan
Copy link
Contributor

When running under bazel test, the source.CallExprArgs function will fail to open the target source file.

I have a minimal reproducer here: https://github.com/cstrahan/assert_bazel_repro

For posterity (and convenience), here's the code from that repo:

package assert_repro_test

import (
	"fmt"
	"log"
	"os"
	"syscall"
	"testing"

	"gotest.tools/v3/assert"
)

func TestAssertShouldWorkUnderBazel(t *testing.T) {
	// to prevent std{out,err} being out of order
	syscall.Dup2(1, 2)

	cwd, _ := os.Getwd()
	fmt.Println("===> CWD: ", cwd)

	fmt.Println("===> BEGIN FILES")
	entries, err := os.ReadDir("./")
	if err != nil {
		log.Fatal(err)
	}
	for _, e := range entries {
		fmt.Println(e.Name())
	}
	fmt.Println("===> END FILES")

	a := 1
	b := 2
	assert.Assert(t, a == b, "a should equal b")
}

When I invoke go test directly, I see:

$ GOTESTTOOLS_DEBUG=1 go test ./some_package
===> CWD:  /Users/charles/src/assert_bazel_repro/some_package
===> BEGIN FILES
BUILD.bazel
repro_test.go
===> END FILES
DEBUG: call stack position: /Users/charles/src/assert_bazel_repro/some_package/repro_test.go:32
DEBUG: found node: (*ast.ExprStmt) assert.Assert(t, a == b, "a should equal b")
DEBUG: visit: (*ast.ExprStmt) assert.Assert(t, a == b, "a should equal b")
DEBUG: visit: (*ast.CallExpr) assert.Assert(t, a == b, "a should equal b")
DEBUG: callExpr: (*ast.CallExpr) assert.Assert(t, a == b, "a should equal b")
--- FAIL: TestAssertShouldWorkUnderBazel (0.00s)
    repro_test.go:32: assertion failed: a is not b: a should equal b
FAIL
FAIL    github.com/cstrahan/assert_bazel_repro/some_package     0.242s
FAIL

The repro_test.go:32: assertion failed: a is not b: a should equal b looks good 👍

However, when run under bazel:

$ bazel test  --test_env=GOTESTTOOLS_DEBUG=1 --test_output=all --cache_test_results=no //some_package:some_package_test
INFO: Analyzed target //some_package:some_package_test (1 packages loaded, 2 targets configured).
INFO: Found 1 test target...
FAIL: //some_package:some_package_test (see /private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/execroot/__main__/bazel-out/darwin_arm64-fastbuild/testlogs/some_package/some_package_test/test.log)
INFO: From Testing //some_package:some_package_test:
==================== Test output for //some_package:some_package_test:
===> CWD:  /private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/sandbox/darwin-sandbox/177/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/some_package/some_package_test_/some_package_test.runfiles/__main__/some_package
===> BEGIN FILES
repro_test.go
some_package_test_
===> END FILES
DEBUG: call stack position: some_package/repro_test.go:32
--- FAIL: TestAssertShouldWorkUnderBazel (0.00s)
    repro_test.go:32: failed to parse source file some_package/repro_test.go: open some_package/repro_test.go: no such file or directory
FAIL
================================================================================
Target //some_package:some_package_test up-to-date:
  bazel-bin/some_package/some_package_test_/some_package_test
INFO: Elapsed time: 1.417s, Critical Path: 1.18s
INFO: 7 processes: 1 internal, 6 darwin-sandbox.
INFO: Build completed, 1 test FAILED, 7 total actions
//some_package:some_package_test                                         FAILED in 0.3s
  /private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/execroot/__main__/bazel-out/darwin_arm64-fastbuild/testlogs/some_package/some_package_test/test.log

Executed 1 out of 1 test: 1 fails locally.

Instead of showing the user defined assertion message, we see that source.CallExprArgs chokes when trying to open some_package/repro_test.go:

repro_test.go:32: failed to parse source file some_package/repro_test.go: open some_package/repro_test.go: no such file or directory

This makes sense, as the effective path to be opened will be:

/private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/sandbox/darwin-sandbox/177/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/some_package/some_package_test_/some_package_test.runfiles/__main__/some_package/some_package/repro_test.go

It's probably more immediately apparent if we let SRC_ROOT=/private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/sandbox/darwin-sandbox/177/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/some_package/some_package_test_/some_package_test.runfiles/__main__/:

$SRC_ROOT/some_package/some_package/repro_test.go

That ought to be:

$SRC_ROOT/some_package/repro_test.go

I don't know why the filename that runtime.Caller returns (here) is an absolute path under go test, but a relative path under bazel test. I think I'll poke the rules_go devs to see if there's anything they can do from their end to make this consistent across both environments.

At any rate, it seems that a workaround shouldn't be too difficult to implement within gotest.tools:

  1. Cache a global var origWorkingDir, _ = os.Getwd() (in case the test code Chdirs)
  2. If the path returned from runtime.Caller is relative, join origWorkingDir and the basename of filename.

Admittedly, that falls down if Assert (or whatever) is called from somewhere other than directly within the package under test.

Workarounds aside, it would be nice if this library would gracefully handle failure to resolve original source file. My proposal would be to print the user supplied assertion message, and then give a warning. I would bet that 99% of people running into this would be bazel users, so I think it would be worthwhile having a friendly targeted message, also giving them a heads up that they might be forgetting to add their source files to their go_test.data param, as I did here: https://github.com/cstrahan/assert_bazel_repro/blob/575739d5201400c10f634053a5eb365f054d4dd4/some_package/BUILD.bazel#L7

load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_test(
    name = "some_package_test",
    srcs = ["repro_test.go"],
    deps = ["@tools_gotest_v3//assert"],
    data = glob(["*.go"]), # <---- without this (or similar), bazel won't include repro_test.go in the test sandbox
)

A hint to look at https://github.com/bazelbuild/rules_go#how-do-i-access-go_binary-executables-from-go_test would be great.

Would you be interested in accepting a PR to implement some or all of the above?

Thanks! 😄

@cstrahan
Copy link
Contributor Author

I'm tracking the go test vs bazel test discrepancy here: bazelbuild/rules_go#3690

@cstrahan
Copy link
Contributor Author

Looking at the env vars under bazel test, I see

TEST_SRCDIR=/private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/sandbox/darwin-sandbox/10/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/some_package/some_package_test_/some_package_test.runfiles

There's also:

BAZEL_TEST=1

And this matches what's documented here: https://bazel.build/reference/test-encyclopedia

So my new recommendation would be that, when the file paths are relative, we attempt to join the file path to $TEST_SRCDIR. If that then fails, we give bazel users a friendly reminder (via warning) to add something like data = glob(["*.go"]) to their go_tests, and still print the assertion message (when they're calling assert.Assert; similar reasoning for the other functions).

@dnephin dnephin added the bug label Sep 13, 2023
@dnephin
Copy link
Member

dnephin commented Sep 13, 2023

Thanks for opening this issue! Reading the env vars set by Bazel sounds like a good approach, as does the warning message to include the go source files when BAZEL_TEST=1. A PR to fix this would be very much appreciated, thank you!

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

Successfully merging a pull request may close this issue.

2 participants