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

Fix tests with --enable_bzlmod #1851

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
102 changes: 82 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,100 @@ 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 = r.Open(*gazelleBinaryPath)
if err != nil {
t.Fatalf("Failed to open gazelle binary %s in runfiles. Error: %v", *gazelleBinaryPath, err)
}
fs.WalkDir(r, ".", func(p string, d fs.DirEntry, err error) error {
println("p: ", p)
return nil
})
err = fs.WalkDir(r, repo, func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return nil
}
println("p: ", p)
// 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.