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.

Sign up for GitHub

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(coverage): add test attributes in transition module #1649

Merged
merged 5 commits into from
Dec 21, 2023
Merged

fix(coverage): add test attributes in transition module #1649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8fa4a21
fix(coverage): add test attr in transition module
tanan Dec 21, 2023
12a6072
fix(docs): change attributes
tanan Dec 21, 2023
6b46710
fix(docs): add an issue reference
tanan Dec 21, 2023
4e93b7b
fix(docs): add comments for coverage attributes
tanan Dec 21, 2023
e7a9f6d
fix conflict
tanan Dec 21, 2023
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ A brief description of the categories of changes:
instead with a location to the patch that could be used to silence the warning.
Copy the patch to your workspace and add it to the list if patches for the wheel
file if you decide to do so.
* (coverage): coverage reports are now created when the version-aware
rules are used.
([#1600](https://github.com/bazelbuild/rules_python/issues/1600))

### Added

Expand Down
21 changes: 19 additions & 2 deletions python/config_settings/transition.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,33 @@ _COMMON_ATTRS = {
),
}

_PY_TEST_ATTRS = {
# Magic attribute to help C++ coverage work. There's no
# docs about this; see TestActionBuilder.java
"_collect_cc_coverage": attr.label(
default = "@bazel_tools//tools/test:collect_cc_coverage",
executable = True,
cfg = "exec",
),
# Magic attribute to make coverage work. There's no
# docs about this; see TestActionBuilder.java
"_lcov_merger": attr.label(
default = configuration_field(fragment = "coverage", name = "output_generator"),
executable = True,
cfg = "exec",
),
}

_transition_py_binary = rule(
_transition_py_impl,
attrs = _COMMON_ATTRS,
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it needed in both, py_binary and py_test as opposed just py_test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aignas My understanding is that code coverage is collected from py_binary targets as well.
One of the reference is in https://bazel.build/reference/be/python#py_runtime.coverage_tool

These attributes are defined in common py_binary rule in this repo.
https://github.com/bazelbuild/rules_python/blob/main/python/private/common/py_binary_rule_bazel.bzl

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thank you, makes sense, just wanted to double check.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if a test runs a binary, then the binary needs to be instrumented so it can produce the coverage rules (which the outer test can collect and process)

cfg = _transition_python_version,
executable = True,
)

_transition_py_test = rule(
_transition_py_impl,
attrs = _COMMON_ATTRS,
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
cfg = _transition_python_version,
test = True,
)
Expand Down
4 changes: 4 additions & 0 deletions python/private/common/py_binary_rule_bazel.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ load(
)

_PY_TEST_ATTRS = {
# Magic attribute to help C++ coverage work. There's no
# docs about this; see TestActionBuilder.java
"_collect_cc_coverage": attr.label(
default = "@bazel_tools//tools/test:collect_cc_coverage",
executable = True,
cfg = "exec",
),
# Magic attribute to make coverage work. There's no
# docs about this; see TestActionBuilder.java
"_lcov_merger": attr.label(
default = configuration_field(fragment = "coverage", name = "output_generator"),
executable = True,
Expand Down