Skip to content

Commit

Permalink
golang: fix exception when coverage enabled and external tests import…
Browse files Browse the repository at this point in the history
… the base package (#21452)

As reported in #21386, Pants
raises an exception when coverage is enabled and a package has external
tests (i.e., "xtests") which import the base package. The bug occurred
because the code which injects the base package dependency did not
properly pass through the coverage flag. Consequently, the compile graph
ended up with two different variants of the base package: one with
coverage codegen and the other without coverage codegen. This resulted
in two different copies of the package archive which Pants cannot merge
together into a single `Digest`.

The solution is to properly pass through the coverage flag when
injecting the base package dependency on the external test package to
ensure there are not multiple variants of the base package in the
compile graph.

Fixes #21386.
  • Loading branch information
tdyas authored Sep 25, 2024
1 parent 69ce39e commit 8c420c9
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
4 changes: 4 additions & 0 deletions docs/notes/2.24.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ Pants will now warn if any errors are encountered while fingerprinting candidate

Added a new `cache_scope` field to `adhoc_tool` and `shell_command` targets to allow configuration of the "cache scope" of the invoked process. The cache scope determines how long Pants will cache the result of the invoked process absent any other invalidation of the result via source or dependency changes.

#### Go

Fix a bug where Pants raised an internal exception which occurred when compiling a Go package with coverage support when the package also had an external test which imported the base package.

### Plugin API changes

The `path_metadata_request` intrinsic rule can now access metadata for paths in the local system outside of the build root. Use the new `namespace` field on `PathMetadataRequest` to request metdata on local system paths using namespace `PathNamespace.SYSTEM`.
Expand Down
42 changes: 42 additions & 0 deletions src/python/pants/backend/go/goals/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,3 +714,45 @@ def test_profile_options_write_results(rule_runner: RuleRunner) -> None:
"test_runner",
"trace.out",
]


def test_external_test_with_use_coverage(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"foo/BUILD": "go_mod(name='mod')\ngo_package()",
"foo/go.mod": "module foo",
"foo/add.go": textwrap.dedent(
"""
package foo
func Add(x, y int) int {
return x + y
}
"""
),
"foo/add_test.go": textwrap.dedent(
"""
package foo_test
import (
"foo"
"testing"
)
func TestAdd(t *testing.T) {
if foo.Add(2, 3) != 5 {
t.Fail()
}
}
"""
),
}
)
tgt = rule_runner.get_target(Address("foo", generated_name="./"))
rule_runner.set_options(
[
"--test-use-coverage",
],
env_inherit={"PATH"},
)
result = rule_runner.request(
TestResult, [GoTestRequest.Batch("", (GoTestFieldSet.create(tgt),), None)]
)
assert result.exit_code == 0
5 changes: 4 additions & 1 deletion src/python/pants/backend/go/util_rules/build_pkg_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ async def setup_build_go_package_target_request(
maybe_base_pkg_dep = await Get(
FallibleBuildGoPackageRequest,
BuildGoPackageTargetRequest(
request.address, for_tests=True, build_opts=request.build_opts
request.address,
for_tests=True,
with_coverage=request.with_coverage,
build_opts=request.build_opts,
),
)
if maybe_base_pkg_dep.request is None:
Expand Down

0 comments on commit 8c420c9

Please sign in to comment.