Fix issue where we have duplicate packages in tests

Author: Jon Poole

build_defs/go.build_defs

@@ -217,22 +217,28 @@ def go_stdlib(name:str, tags:list=CONFIG.GO.BUILD_TAGS, labels:list=[], visibili
     )
 
 
-def _pkg_sources(name: str, deps: list, test_only:bool):
+def _pkg_local_deps(name: str, deps: list, test_only:bool):
     """
-    This rule generates a list of go srcs for any rule in the deps list that is local to this package.
+    Used by go_test to generate the deps for a non-external test lib.
 
-    This is how we avoid transitively depending on all sources for a target in order to compile non-external tests.
-    Without this, we'd have to transitively pull in all third party sources for the rule which is very expensive,
-    especially for remote execution.
+    It works by finding any package-local dependencies, generating a filegroup that exports their sources, and filtering
+    them out of the returned dependency list. The library then compiles against all sources it finds in the package
+    directory, rather than just the srcs pass in via `srcs = []`.
     """
     package_prefix = canonicalise(":all").removesuffix("all")
-    return filegroup(
-       name = name,
-       tag = "pkg_srcs",
-       exported_deps = [dep for dep in deps if canonicalise(dep).startswith(package_prefix)],
-       requires = ["go_src"],
-       test_only = test_only,
-   )
+    pkg_local_deps = [dep for dep in deps if canonicalise(dep).startswith(package_prefix)]
+    deps = [dep for dep in deps if not canonicalise(dep).startswith(package_prefix)]
+
+    deps = deps + [
+        filegroup(
+           name = tag(name, "test_srcs"),
+           tag = "pkg_srcs",
+           exported_deps = pkg_local_deps,
+           requires = ["go_src"],
+           test_only = test_only,
+       )
+    ]
+    return deps
 
 def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs:list=None, deps:list=[],
                visibility:list=None, test_only:bool&testonly=False, complete:bool=True,
@@ -307,7 +313,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs:
     # We could just depend on go_src for all our deps but that would mean bringing in srcs for targets outside this
     # package like third party sources which is especially slow on systems with slow syscall performance (macOS)
     if _all_srcs:
-        deps += [_pkg_sources(name, deps, test_only)]
+        deps = _pkg_local_deps(name, deps, test_only)
     if CONFIG.GO.STDLIB:
         deps += _stdlib()
 
@@ -730,13 +736,12 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps:
       env (dict): Additional environment variables to set for the test
     """
 
-    test_package = name + '_lib'
+    test_package = name + '_lib' if external else None
     # Unfortunately we have to recompile this to build the test together with its library.
     lib_rule = go_library(
         name = f'_{name}#lib',
         srcs = srcs,
         resources = resources,
-        package = test_package,
         deps = deps,
         _all_srcs=not external,
         labels = labels,
@@ -750,7 +755,7 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps:
     )
 
     if cgo:
-        archive_name = test_package + "_cgo"
+        archive_name = f"_{name}#lib_cgo"
         lib_rule = merge_cgo_obj(
             name = name,
             tag='cgo',
@@ -762,12 +767,12 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps:
             package = archive_name,
         )
     else:
-        archive_name = test_package
+        archive_name = f"_{name}#lib"
 
     import_config = build_rule(
         name=f'_{name}#lib_import_config',
         cmd = _write_import_config_cmd(archive_name, test_package),
-        outs = [f'{test_package}.importconfig'],
+        outs = [f'{name}_test_lib.importconfig'],
         visibility = visibility,
         test_only = True,
         labels = labels,
@@ -779,8 +784,8 @@ def go_test(name:str, srcs:list, resources:list=None, data:list|dict=None, deps:
         deps = deps,
         test_only = True,
         labels = labels,
-        test_package = test_package,
         external = external,
+        test_package = test_package,
     )
     modinfo = _go_modinfo(
         name = name,

test/normal_test/BUILD

@@ -0,0 +1,12 @@
+subinclude("///go//build_defs:go")
+
+go_library(
+    name = "foo",
+    srcs = ["foo.go"],
+)
+
+go_test(
+    name = "foo_test",
+    srcs = ["foo_test.go"],
+    deps = [":foo"],
+)

test/normal_test/foo.go

@@ -0,0 +1,5 @@
+package foo
+
+func foo() string {
+	return "foo"
+}

test/normal_test/foo_test.go

@@ -0,0 +1,11 @@
+package foo
+
+import "testing"
+
+func TestFoo(t *testing.T) {
+	res := foo()
+
+	if res != "foo" {
+		t.Fail()
+	}
+}