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

Editor support broken for gazelle generated go_test for packages containing both internal and external tests #3015

Closed
rohitagarwal003 opened this issue Nov 23, 2021 · 8 comments · Fixed by #3160 or #3116
Labels
IDE Go package driver and IDE support

Comments

@rohitagarwal003
Copy link

What version of rules_go are you using?

v0.29.0

What version of gazelle are you using?

v0.24.0

What version of Bazel are you using?

4.2.1

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

I am using the latest versions already.

What operating system and processor architecture are you using?

Intel Mac

Any other potentially useful information about your toolchain?

I have created the initial setup here: https://gist.github.com/rohitagarwal003/d9087ca9f87271b1b4f1d392a9dea593.

If you download all those files in a directory named mypkg and run bazel run //:gazelle from there, the updated BUILD.bazel will not work correctly with VSCode.

BUILD.bazel file generated by gazelle that doesn't work with VSCode

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

# gazelle:prefix example.com/mypkg
gazelle(name = "gazelle")

go_library(
    name = "mypkg",
    srcs = ["mypkg.go"],
    importpath = "example.com/mypkg",
    visibility = ["//visibility:public"],
)

go_test(
    name = "mypkg_test",
    srcs = [
        "mypkg_external_test.go",
        "mypkg_internal_test.go",
    ],
    embed = [":mypkg"],
)

error-with-gazelle-generated-rules

Error: package mypkg_test; expected mypkg compiler(MismatchedPkgName)

BUILD.bazel file that does work

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

go_library(
    name = "mypkg",
    srcs = ["mypkg.go"],
    importpath = "example.com/mypkg",
    visibility = ["//visibility:public"],
)

go_test(
    name = "mypkg_external_test",
    srcs = ["mypkg_external_test.go"],
    deps = [":mypkg"],
)

go_test(
    name = "mypkg_internal_test",
    srcs = ["mypkg_internal_test.go"],
    embed = [":mypkg"],
)

no-error-with-manual-rules


cc @steeve @linzhp

@linzhp
Copy link
Contributor

linzhp commented Nov 23, 2021

Does bazel build : mypkg_test work?

@rohitagarwal003
Copy link
Author

Yes. bazel build :mypkg_test and bazel test :mypkg_test both work

@JamyDev
Copy link
Contributor

JamyDev commented Nov 23, 2021

image

@rohitagarwal003 I tried your setup with a barebones VSCode config and it seems to be working fine. Do you have anything custom on your settings.json? (Both workspace and global)

@rohitagarwal003
Copy link
Author

I added my workspace's .vscode/settings.json to the above gist: https://gist.github.com/rohitagarwal003/d9087ca9f87271b1b4f1d392a9dea593#file-vscode-workspace-settings-json

Here's my global settings.json:
{
    "workbench.editor.enablePreview": false,
    "ruby.useLanguageServer": false,
    "sourcegraph.url": "https://sourcegraph.d.musta.ch",
    "redhat.telemetry.enabled": false,
    "gitlens.codeLens.scopes": [
        "document",
        "containers"
    ],
    "gitlens.codeLens.enabled": false,
    "workbench.iconTheme": "vs-minimal",
    "gitlens.currentLine.enabled": false,
    "bazel.enableCodeLens": true,
}

Can you make sure you still don't see the error after you Reload Window or restart the editor with this BUILD file etc.?

@JamyDev
Copy link
Contributor

JamyDev commented Nov 23, 2021

After adding the GOPACKAGESDRIVER setting I'm able to repro it too, yes.

@ian-h-chamberlain
Copy link
Contributor

Just in case it adds more context, I also ran into this and in my case this is the output of gopackagesdriver, using rules_go v0.29.0, gazelle 0.23.0, and Bazel 4.2.0:

$ echo '{}' | ./tools/bazelbuild/go/gopackagesdriver.sh file=go/src/dnf/querier_test.go
INFO: Invocation ID: 5b604f89-0b84-4ea0-9ae0-4f4a8c1e7130
INFO: Analyzed target @io_bazel_rules_go//go/tools/gopackagesdriver:gopackagesdriver (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target @io_bazel_rules_go//go/tools/gopackagesdriver:gopackagesdriver up-to-date:
  bazel-bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/gopackagesdriver_/gopackagesdriver
INFO: Elapsed time: 0.247s, Critical Path: 0.03s
INFO: 1 process: 1 internal.
INFO: Running command line: bazel-bin/external/io_bazel_rules_go/go/tools/gopackagesdriver/gopackagesdriver_/gopackagesdriver 'file=go/src/dnf/querier_test.go'
INFO: Build Event Protocol files produced successfully.
INFO: Build completed successfully, 1 total action
Running: [bazel info --tool_tag=gopackagesdriver --ui_actions_shown=0]
INFO: Invocation ID: 6943998b-57ed-4139-8fc8-c20c805ec567
Running: [bazel query --tool_tag=gopackagesdriver --ui_actions_shown=0 --ui_event_filters=-info,-stderr --noshow_progress --order_output=no --output=label --nodep_deps --noimplicit_deps --notool_deps kind("go_library|go_test", same_pkg_direct_rdeps("go/src/dnf/querier_test.go"))]
Running: [bazel build --tool_tag=gopackagesdriver --ui_actions_shown=0 --show_result=0 --build_event_json_file=/var/folders/wb/7n13y1cx0v1d28zb304bw8bm0000gn/T/gopackagesdriver_bep_612121100 --build_event_json_file_path_conversion=no --experimental_convenience_symlinks=ignore --ui_event_filters=-info,-stderr --noshow_progress --aspects=@io_bazel_rules_go//go/tools/gopackagesdriver:aspect.bzl%go_pkg_info_aspect --output_groups=go_pkg_driver_json_file,go_pkg_driver_stdlib_json_file,go_pkg_driver_srcs --keep_going ]
ERROR: Skipping '': the empty string is not a valid target
WARNING: Target pattern parsing failed.
ERROR: Failed to parse target pattern even though it was previously parsed successfully: the empty string is not a valid target

And the JSON payload that comes with it:

{"NotHandled":false,"Sizes":{"WordSize":8,"MaxAlign":8},"Packages":[]}

@ian-h-chamberlain
Copy link
Contributor

I've done a little more digging and found something else:

https://github.com/bazelbuild/rules_go/blob/master/go/tools/gopackagesdriver/bazel_json_builder.go#L38

This query checks for go_library and go_test rule kinds, but in my case the rules were defined as go_transition_test kind. My organization uses # gazelle:map_kind for go_test, but our mapped macro is very simple:

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

def go_test(name, race = "on", **kwargs):
    """The same as upstream go_test but the race detector is on by default."""
    real_go_test(name, race = race, **kwargs)

By adding go_transition_test in the file query in bazel_json_builder.go, I was able to get a bit further along and the aspect is now being used, but something is still not working as the final JSON from the driver still looks like

{
  "NotHandled": false,
  "Sizes": {
    "WordSize": 8,
    "MaxAlign": 8
  },
  "Packages": []
}

@ian-h-chamberlain
Copy link
Contributor

Follow-up to my above comment: I found the other place where go_test is referenced but go_transition_test also needs to be considered. This patch seems to work for me but I'm not sure if there might be any other side effects, so I'll leave it to the maintainers to decide how to move forward.

diff --git a/go/tools/gopackagesdriver/aspect.bzl b/go/tools/gopackagesdriver/aspect.bzl
index de9eacd2..7e1ac2dc 100644
--- a/go/tools/gopackagesdriver/aspect.bzl
+++ b/go/tools/gopackagesdriver/aspect.bzl
@@ -98,7 +98,7 @@ def _go_pkg_info_aspect_impl(target, ctx):
         # the test source files are there. Then, create the pkg json file directly. Only
         # do that for direct dependencies that are not defined as deps, and use the
         # importpath to find which.
-        if ctx.rule.kind == "go_test":
+        if ctx.rule.kind in ["go_test", "go_transition_test"]:
             deps_targets = [
                 dep[GoArchive].data.importpath
                 for dep in ctx.rule.attr.deps
diff --git a/go/tools/gopackagesdriver/bazel.go b/go/tools/gopackagesdriver/bazel.go
index 881ccc70..c2e5290c 100644
--- a/go/tools/gopackagesdriver/bazel.go
+++ b/go/tools/gopackagesdriver/bazel.go
@@ -138,15 +138,13 @@ func (b *Bazel) Query(ctx context.Context, args ...string) ([]string, error) {
 	if err != nil {
 		return nil, fmt.Errorf("bazel query failed: %w", err)
 	}
-	return strings.Split(strings.TrimSpace(output), "\n"), nil
-}
 
-func (b *Bazel) QueryLabels(ctx context.Context, args ...string) ([]string, error) {
-	output, err := b.run(ctx, "query", args...)
-	if err != nil {
-		return nil, fmt.Errorf("bazel query failed: %w", err)
+	trimmedOutput := strings.TrimSpace(output)
+	if len(trimmedOutput) == 0 {
+		return nil, nil
 	}
-	return strings.Split(strings.TrimSpace(output), "\n"), nil
+
+	return strings.Split(trimmedOutput, "\n"), nil
 }
 
 func (b *Bazel) WorkspaceRoot() string {
diff --git a/go/tools/gopackagesdriver/bazel_json_builder.go b/go/tools/gopackagesdriver/bazel_json_builder.go
index 14f00188..c1e3ca29 100644
--- a/go/tools/gopackagesdriver/bazel_json_builder.go
+++ b/go/tools/gopackagesdriver/bazel_json_builder.go
@@ -35,7 +35,7 @@ func (b *BazelJSONBuilder) fileQuery(filename string) string {
 		fp, _ := filepath.Rel(b.bazel.WorkspaceRoot(), filename)
 		filename = fp
 	}
-	return fmt.Sprintf(`kind("go_library|go_test", same_pkg_direct_rdeps("%s"))`, filename)
+	return fmt.Sprintf(`kind("go_library|go_test|go_transition_test", same_pkg_direct_rdeps("%s"))`, filename)
 }
 
 func (b *BazelJSONBuilder) packageQuery(importPath string) string {

For others looking to fix this, you should just be able to use this as a patch to rules_go in WORKSPACE to work around this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDE Go package driver and IDE support
Projects
None yet
4 participants