Skip to content

Commit

Permalink
Fix tests with --enable_bzlmod
Browse files Browse the repository at this point in the history
* Move off of the deprecated `bazel.ListRunfiles`
* Fix `fix_test` Go SDK discovery
  • Loading branch information
fmeum committed Jul 31, 2024
1 parent 50f6959 commit eb19027
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 96 deletions.
9 changes: 0 additions & 9 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ tasks:
test_targets:
- "..."
- "-//internal:bazel_test"
- "-//cmd/gazelle:gazelle_test"
macos_arm64:
name: Mac OS Arm 64 with WORKSPACE
platform: macos_arm64
Expand All @@ -119,7 +118,6 @@ tasks:
test_targets:
- "..."
- "-//internal:bazel_test"
- "-//cmd/gazelle:gazelle_test"
macos:
name: Mac OS with WORKSPACE
platform: macos
Expand All @@ -143,13 +141,9 @@ tasks:
test_targets:
- "--"
- "..."
- "-//cmd/gazelle:gazelle_test"
# autogazelle is only supported on UNIX-like platforms.
# It requires UNIX domain sockets.
- "-//cmd/autogazelle/..."
# Stardoc produces different line-endings on windows,
# so the documentation it generates doesn't match the checked in files
- "-//docs:all"
# Fails to execute, apparently due to command-line length limit.
- "-//internal:bazel_test"
# gazelle prints file paths with backward slashes on windows,
Expand All @@ -174,9 +168,6 @@ tasks:
# autogazelle is only supported on UNIX-like platforms.
# It requires UNIX domain sockets.
- "-//cmd/autogazelle/..."
# Stardoc produces different line-endings on windows,
# so the documentation it generates doesn't match the checked in files
- "-//docs:all"
# Fails to execute, apparently due to command-line length limit.
- "-//internal:bazel_test"
# gazelle prints file paths with backward slashes on windows,
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.1.1
7.2.1
9 changes: 1 addition & 8 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module(
bazel_dep(name = "bazel_features", version = "1.9.1")
bazel_dep(name = "bazel_skylib", version = "1.5.0")
bazel_dep(name = "protobuf", version = "3.19.6", repo_name = "com_google_protobuf")
bazel_dep(name = "rules_go", version = "0.46.0", repo_name = "io_bazel_rules_go")
bazel_dep(name = "rules_go", version = "0.49.0", repo_name = "io_bazel_rules_go")
bazel_dep(name = "rules_proto", version = "4.0.0")

go_sdk = use_extension("@io_bazel_rules_go//go:extensions.bzl", "go_sdk")
Expand Down Expand Up @@ -68,10 +68,3 @@ use_repo(
go_sdk_dev,
go_sdk = "go_default_sdk",
)

# This will only apply in the root module. temporarily needed to fix `bazel_features` in CI.
single_version_override(
module_name = "rules_go",
patch_strip = 1,
patches = ["//third_party/patches:rules_go.patch"],
)
2 changes: 1 addition & 1 deletion cmd/gazelle/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ go_test(
"//internal/wspace",
"//testtools",
"@com_github_google_go_cmp//cmp",
"@io_bazel_rules_go//go/tools/bazel:go_default_library",
"@io_bazel_rules_go//go/runfiles:go_default_library",
],
)

Expand Down
25 changes: 7 additions & 18 deletions cmd/gazelle/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import (
"flag"
"fmt"
"os"
"path"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/bazelbuild/bazel-gazelle/testtools"
"github.com/bazelbuild/rules_go/go/tools/bazel"
"github.com/bazelbuild/rules_go/go/runfiles"
)

var goSdk = flag.String("go_sdk", "", "name of the go_sdk repository when invoked by Bazel")
Expand Down Expand Up @@ -64,25 +65,13 @@ func TestMain(m *testing.M) {
if *goSdk != "" {
// This flag is only set when the test is run by Bazel. Figure out where
// the Go binary is and set GOROOT appropriately.
entries, err := bazel.ListRunfiles()
if err != nil {
fmt.Fprintln(os.Stderr, err)
return
}

var goToolPath string
ext := ""
goBasename := "go"
if runtime.GOOS == "windows" {
ext = ".exe"
goBasename += ".exe"
}
for _, entry := range entries {
if entry.Workspace == *goSdk && entry.ShortPath == "bin/go"+ext {
goToolPath = entry.Path
break
}
}
if goToolPath == "" {
fmt.Fprintln(os.Stderr, "could not locate go tool")
goToolPath, err := runfiles.Rlocation(path.Join(*goSdk, "bin", goBasename))
if err != nil {
fmt.Fprintf(os.Stderr, "could not locate go tool: %v\n", err)
return
}
os.Setenv("GOROOT", filepath.Dir(filepath.Dir(goToolPath)))
Expand Down
4 changes: 2 additions & 2 deletions extend.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ The generation test expects a file structure like the following:
```
|-- <testDataPath>
|-- some_test
|-- WORKSPACE
|-- WORKSPACE or MODULE.bazel or REPO.bazel
|-- README.md --> README describing what the test does.
|-- arguments.txt --> newline delimited list of arguments to pass in (ignored if empty).
|-- expectedStdout.txt --> Expected stdout for this test.
Expand All @@ -174,7 +174,7 @@ To update the expected files, run `UPDATE_SNAPSHOTS=true bazel run //path/to:the
| :------------- | :------------- | :------------- |
| <a id="gazelle_generation_test-name"></a>name | The name of the test. | none |
| <a id="gazelle_generation_test-gazelle_binary"></a>gazelle_binary | The name of the gazelle binary target. For example, //path/to:my_gazelle. | none |
| <a id="gazelle_generation_test-test_data"></a>test_data | A list of target of the test data files you will pass to the test. This can be a https://bazel.build/reference/be/general#filegroup. | none |
| <a id="gazelle_generation_test-test_data"></a>test_data | A list of target of the test data files you will pass to the test. This can be a https://bazel.build/reference/be/general#filegroup. Only test data files in the same repository as the gazelle_binary are supported. | none |
| <a id="gazelle_generation_test-build_in_suffix"></a>build_in_suffix | The suffix for the input BUILD.bazel files. Defaults to .in. By default, will use files named BUILD.in as the BUILD files before running gazelle. | `".in"` |
| <a id="gazelle_generation_test-build_out_suffix"></a>build_out_suffix | The suffix for the expected BUILD.bazel files after running gazelle. Defaults to .out. By default, will use files named check the results of the gazelle run against files named BUILD.out. | `".out"` |
| <a id="gazelle_generation_test-gazelle_timeout_seconds"></a>gazelle_timeout_seconds | <p align="center"> - </p> | `2` |
Expand Down
93 changes: 73 additions & 20 deletions internal/generationtest/generation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@ package generationtest

import (
"flag"
"io"
"io/fs"
"os"
"path"
"path/filepath"
"strings"
"testing"
"time"

"github.com/bazelbuild/bazel-gazelle/testtools"
"github.com/bazelbuild/rules_go/go/runfiles"
"github.com/bazelbuild/rules_go/go/tools/bazel"
)

var (
gazelleBinaryPath = flag.String("gazelle_binary_path", "", "Path to the gazelle binary to test.")
gazelleBinaryPath = flag.String("gazelle_binary_path", "", "Runfiles path of the gazelle binary to test.")
buildInSuffix = flag.String("build_in_suffix", ".in", "The suffix on the test input BUILD.bazel files. Defaults to .in. "+
" By default, will use files named BUILD.in as the BUILD files before running gazelle.")
buildOutSuffix = flag.String("build_out_suffix", ".out", "The suffix on the expected BUILD.bazel files after running gazelle. Defaults to .out. "+
Expand All @@ -38,44 +44,91 @@ var (
// workspaces and confirms that the generated BUILD files match expectation.
func TestFullGeneration(t *testing.T) {
tests := []*testtools.TestGazelleGenerationArgs{}
runfiles, err := bazel.ListRunfiles()
r, err := runfiles.New()
if err != nil {
t.Fatalf("bazel.ListRunfiles() error: %v", err)
t.Fatalf("Failed to create runfiles: %v", err)
}
// Convert workspace relative path for gazelle binary into an absolute path.
// E.g. path/to/gazelle_binary -> /absolute/path/to/workspace/path/to/gazelle/binary.
absoluteGazelleBinary, err := bazel.Runfile(*gazelleBinaryPath)
gazelleBinary, err := r.Rlocation(*gazelleBinaryPath)
if err != nil {
t.Fatalf("Could not convert gazelle binary path %s to absolute path. Error: %v", *gazelleBinaryPath, err)
t.Fatalf("Failed to find gazelle binary %s in runfiles. Error: %v", *gazelleBinaryPath, err)
}
for _, f := range runfiles {
// Look through runfiles for WORKSPACE files. Each WORKSPACE is a test case.
if filepath.Base(f.Path) == "WORKSPACE" {
// absolutePathToTestDirectory is the absolute
// path to the test case directory. For example, /home/<user>/wksp/path/to/test_data/my_test_case
absolutePathToTestDirectory := filepath.Dir(f.Path)
// relativePathToTestDirectory is the workspace relative path
// to this test case directory. For example, path/to/test_data/my_test_case
relativePathToTestDirectory := filepath.Dir(f.ShortPath)

testDir, err := bazel.NewTmpDir("gazelle_generation_test")
if err != nil {
t.Fatalf("Failed to create temporary directory: %v", err)
}

// Collect tests in the same repo as the gazelle binary.
repo := strings.Split(*gazelleBinaryPath, "/")[0]
err = fs.WalkDir(r, repo, func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return nil
}
// Each repo boundary file marks a test case.
if d.Name() == "WORKSPACE" || d.Name() == "MODULE.bazel" || d.Name() == "REPO.bazel" {
repoRelativeDir := path.Dir(strings.TrimPrefix(p, repo+"/"))
// name is the name of the test directory. For example, my_test_case.
// The name of the directory doubles as the name of the test.
name := filepath.Base(absolutePathToTestDirectory)
name := filepath.Base(repoRelativeDir)
absolutePath := filepath.Join(testDir, filepath.FromSlash(repoRelativeDir))

tests = append(tests, &testtools.TestGazelleGenerationArgs{
Name: name,
TestDataPathAbsolute: absolutePathToTestDirectory,
TestDataPathRelative: relativePathToTestDirectory,
GazelleBinaryPath: absoluteGazelleBinary,
TestDataPathRelative: repoRelativeDir,
TestDataPathAbsolute: absolutePath,
GazelleBinaryPath: gazelleBinary,
BuildInSuffix: *buildInSuffix,
BuildOutSuffix: *buildOutSuffix,
Timeout: *timeout,
})
}
return nil
})
if err != nil {
t.Fatalf("Failed to collect tests in runfiles: %v", err)
}
if len(tests) == 0 {
t.Fatal("no tests found")
}

// Copy all files under test repos to the temporary directory.
for _, test := range tests {
err = fs.WalkDir(r, path.Join(repo, test.TestDataPathRelative), func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return nil
}
f, err := r.Open(p)
if err != nil {
return err
}
defer f.Close()

targetPath := filepath.Join(testDir, filepath.FromSlash(strings.TrimPrefix(p, repo+"/")))
if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil {
return err
}
out, err := os.Create(targetPath)
if err != nil {
return err
}
defer out.Close()

if _, err := io.Copy(out, f); err != nil {
return err
}
return nil
})
if err != nil {
t.Fatalf("Failed to copy tests from runfiles: %v", err)
}
}

for _, args := range tests {
testtools.TestGazelleGenerationOnPath(t, args)
}
Expand Down
6 changes: 4 additions & 2 deletions internal/generationtest/generationtest.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def gazelle_generation_test(name, gazelle_binary, test_data, build_in_suffix = "
```
|-- <testDataPath>
|-- some_test
|-- WORKSPACE
|-- WORKSPACE or MODULE.bazel or REPO.bazel
|-- README.md --> README describing what the test does.
|-- arguments.txt --> newline delimited list of arguments to pass in (ignored if empty).
|-- expectedStdout.txt --> Expected stdout for this test.
Expand All @@ -46,6 +46,7 @@ def gazelle_generation_test(name, gazelle_binary, test_data, build_in_suffix = "
gazelle_binary: The name of the gazelle binary target. For example, //path/to:my_gazelle.
test_data: A list of target of the test data files you will pass to the test.
This can be a https://bazel.build/reference/be/general#filegroup.
Only test data files in the same repository as the gazelle_binary are supported.
build_in_suffix: The suffix for the input BUILD.bazel files. Defaults to .in.
By default, will use files named BUILD.in as the BUILD files before running gazelle.
build_out_suffix: The suffix for the expected BUILD.bazel files after running gazelle. Defaults to .out.
Expand All @@ -59,10 +60,11 @@ def gazelle_generation_test(name, gazelle_binary, test_data, build_in_suffix = "
srcs = [Label("//internal/generationtest:generation_test.go")],
deps = [
Label("//testtools"),
Label("@io_bazel_rules_go//go/runfiles"),
Label("@io_bazel_rules_go//go/tools/bazel:go_default_library"),
],
args = [
"-gazelle_binary_path=$(rootpath %s)" % gazelle_binary,
"-gazelle_binary_path=$(rlocationpath %s)" % gazelle_binary,
"-build_in_suffix=%s" % build_in_suffix,
"-build_out_suffix=%s" % build_out_suffix,
"-timeout=%ds" % gazelle_timeout_seconds,
Expand Down
2 changes: 1 addition & 1 deletion tests/bcr/go_mod/.bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.1.1
7.2.1
2 changes: 1 addition & 1 deletion tests/bcr/go_work/.bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7.0.2
7.2.1
Empty file removed third_party/patches/BUILD.bazel
Empty file.
33 changes: 0 additions & 33 deletions third_party/patches/rules_go.patch

This file was deleted.

0 comments on commit eb19027

Please sign in to comment.