fix(coverage): add test attributes in transition module

[tanan]

This PR is to fix an issue that coverage report is empty when using transition module.
This is due to the absence of the _lcov_merger and _collect_cc_coverage attributes.
Coverage reports will be created adding these attributes.

Fixes #1600

[rickeylev]

Just some textual updates, otherwise LGTM.

I'd ask for a test, but I don't think we have any infrastructure setup to verify the coverage command is working.

[rickeylev]

Suggested change

	* (coverage): `_lcov_merger` attr added in `_transition_py_test` and
	coverage reports are now created when using the transition module.
	* (coverage): coverage reports are now created when the version-aware
	rules are used.
	([#1600](https://github.com/bazelbuild/rules_python/issues/1600))

[rickeylev]

Yay magic attributes. I'm guessing you copied this from the files in python/private/common? Lets also copy the comment over so we can give what little context we have about them. Based on your analysis in the issue, I updated the text a bit.

Suggested change

	_PY_TEST_ATTRS = {
	"_collect_cc_coverage": attr.label(
	default = "@bazel_tools//tools/test:collect_cc_coverage",
	executable = True,
	cfg = "exec",
	),
	"_lcov_merger": attr.label(
	default = configuration_field(fragment = "coverage", name = "output_generator"),
	executable = True,
	cfg = "exec",
	),
	}
	_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",
	),
	}

[tanan]

Thank you for your suggestion.
These attributes are also written In python/private/common/py_binary_rule_bazel.bzl, but there is no comment.
So, I'll copy the comment in it as well.

[aignas]

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

[tanan]

@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

[aignas]

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

[rickeylev]

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)