feat(toolchain): Extend Python Testing Toolchain with COVERAGE_RC Support #2246

ewianda · 2024-09-24T21:19:52Z

This PR enhances the Test Runner environment to support custom coverage configuration via the COVERAGE_RC environment variable. Inspired by this approach, this feature allows users to specify a .coveragerc file in any compatible format to customize coverage reporting.

Key Changes
Adds support for the COVERAGE_RC environment variable in the Test Runner environment.
Enables users to define and pass their own .coveragerc configurations, enhancing flexibility for various testing and reporting requirements.

close #1434

examples/bzlmod/BUILD.bazel

aignas

This is certainly interesting. It seems that there are //tests/python/python_tests.bzl failing due to the mock module_ctx not being updated.

Shall we mark this as Ready for review and ask others to also have a look? Before doing that could you please update the PR description?

examples/bzlmod/BUILD.bazel

examples/bzlmod/MODULE.bazel

ewianda · 2024-10-26T15:48:03Z

This is certainly interesting. It seems that there are //tests/python/python_tests.bzl failing due to the mock module_ctx not being updated.

Shall we mark this as Ready for review and ask others to also have a look? Before doing that could you please update the PR description?

I am going to work on the description and get the tests to pass, then we can open it up for review.

aignas

Added some extra thoughts about the API design

python/private/py_test_toolchain.bzl

python/private/python.bzl

examples/bzlmod/tests/BUILD.bazel

python/private/py_test_toolchain.bzl

python/private/python_test.bzl

examples/bzlmod/tests/BUILD.bazel

ewianda · 2024-10-31T03:51:02Z

The test under Bazel 6.4.0 test is failing. My guess is because this native java py_test, If that is the case is the a mechanism to run the test conditionally

aignas · 2024-11-01T14:09:01Z

Ah, this is because bazel 6.4 is not using the starlark implementation of the rules, so it is expected to not work for now.

examples/bzlmod/tests/BUILD.bazel

aignas · 2024-11-04T01:57:23Z

Idea for why the Windows build is failing - it is using a very different bootstrap mechanism and it could be that the coveragerc is not wired properly. I am not 100% sure how we should do this. @rickeylev, any thoughts here?

ewianda · 2024-11-05T14:55:27Z

@rickeylev @aignas I have tried to figure out the issue with windows with no success. I noticed that C:\Users\ELVISW~1\AppData\Local\Temp\Bazel.runfiles__dysvgw6\runfiles\_main\.coveragerc is missing C:\Users\ElvisWianda\Documents\rules_python\examples\bzlmod\bazel-bin\tests\coverage_rc_is_set_test.exe.runfiles\_main/.coveragerc
exists

Figured out the issue

ewianda · 2024-12-07T19:24:19Z

ping @aignas @rickeylev

aignas

I like the general direction, but my main question is about how we could implement the additions of the toolchain so that we could have different flavours of py_test.

aignas · 2024-12-08T03:09:30Z

examples/bzlmod/tests/BUILD.bazel

@@ -6,6 +7,12 @@ load("@rules_python//python:defs.bzl", "py_binary", "py_test")
 load("@rules_python//python/config_settings:transition.bzl", py_versioned_binary = "py_binary", py_versioned_test = "py_test")
 load("@rules_shell//shell:sh_test.bzl", "sh_test")

+IS_BAZEL_7_OR_HIGHER = version >= "7"


I think this can be dropped, we are no longer testing bazel 6 with bazelmod.

aignas · 2024-12-09T07:52:22Z

CHANGELOG.md

@@ -48,6 +48,9 @@ A brief description of the categories of changes:

 {#v0-0-0-added}
 ### Added
+* (toolchain) Using testing toolchain to configure py_test coverage.
+   This opens the potential to configure differnt test runners.
+  ([#2246](https://github.com/bazelbuild/rules_python/pull/2246)).


Just to ensure that this gets updated, I think a rebase is needed.

aignas · 2024-12-09T07:53:21Z

python/config_settings/transition.bzl

+    # Add the environment from the target if it has RunEnvironmentInfo.
+    # RunEnvironmentInfo contains environment variables configured by exec_group toolchain
+    if RunEnvironmentInfo in target:
+        env.update(target[RunEnvironmentInfo].environment)


Is this a bug that is fixed by this PR or is it a new feature?

aignas · 2024-12-09T07:54:27Z

python/extensions/python_test.bzl

+use_repo(python_test, "py_test_toolchain")
+register_toolchains("@py_test_toolchain//:all")


The python_test extension in theory could register to toolchain itself, just like python does it. What do you think?

aignas · 2024-12-09T07:56:15Z

python/private/py_executable.bzl

@@ -326,6 +329,12 @@ def _get_runtime_details(ctx, semantics):
                direct.append(effective_runtime.coverage_tool)
            if effective_runtime.coverage_files:
                transitive.append(effective_runtime.coverage_files)
+        if is_test:
+            py_test_toolchain = ctx.exec_groups["test"].toolchains[PY_TEST_TOOLCHAIN_TYPE]


I think this should be something that is a parameter to this function. I am thinking that we could somehow allow users creating executable rules with their own toolchain types. I would love to be able to move toward the direction of #1647.

I think we should use the ability to use a callback macro at the level of the toolchain. I didn't use that approach here since I was not adding an executable. Will change the implementation.

aignas · 2024-12-09T07:57:18Z

python/private/py_executable.bzl

+            py_test_toolchain = ctx.exec_groups["test"].toolchains[PY_TEST_TOOLCHAIN_TYPE]
+            if py_test_toolchain:
+                coverage_rc = py_test_toolchain.py_test_info.coverage_rc
+                extra_test_env = {"COVERAGE_RC": coverage_rc.files.to_list()[0].short_path}


I think in general we should check the ctx.configuration.coverage_enabled and add this in only then.

Regarding my comment above, if we would like to generalize it, do you have ideas how we could do it?

aignas · 2024-12-09T08:05:07Z

python/private/py_test_rule.bzl

+    exec_groups = {
+        "test": exec_group(toolchains = [config_common.toolchain_type(PY_TEST_TOOLCHAIN_TYPE, mandatory = False)]),
+    },


Ideally I would love to be able to express this as

Suggested change

exec_groups = {

"test": exec_group(toolchains = [config_common.toolchain_type(PY_TEST_TOOLCHAIN_TYPE, mandatory = False)]),

},

extra_toolchains = [PY_TEST_TOOLCHAIN_TYPE],

extend_foo = _my_function_to_add_env_and_set_coverage,

exec_groups = {

"test": exec_group(toolchains = [config_common.toolchain_type(PY_TEST_TOOLCHAIN_TYPE, mandatory = False)]),

},

My goal is for us to extend py_test with toolchain such that it would be possible to define a different flavour of py_test with its own toolchain and for that we would be reusing the create_executable_rule snippet like above.

Do you have any ideas?

Based on the C++ Testing Toolchain approach, it is better to have this configured at the toolchain level rather than the rule level. If I understand this correctly, users will be required to specify extend_foo on every py_test rule

I think I don't understand how _my_function_to_add_env_and_set_coverage will access the information provided by the toolchain.

Inspired by https://github.com/trybka/scraps/blob/master/cc_test.md This PR extends Test Runner enviroment to provide a coveragerc enviroment variable COVERAGE_RC, allowing user to provide coverage resource in what ever format

Create partially-bound function for configuring py_test

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch from c7a142b to 173853e Compare September 24, 2024 21:20

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch 4 times, most recently from 4c0451e to 5a16df4 Compare October 22, 2024 19:02

ewianda commented Oct 22, 2024

View reviewed changes

examples/bzlmod/BUILD.bazel Show resolved Hide resolved

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch from 5a16df4 to 039012f Compare October 22, 2024 19:11

aignas reviewed Oct 26, 2024

View reviewed changes

examples/bzlmod/BUILD.bazel Show resolved Hide resolved

examples/bzlmod/MODULE.bazel Outdated Show resolved Hide resolved

aignas reviewed Oct 27, 2024

View reviewed changes

python/private/py_test_toolchain.bzl Outdated Show resolved Hide resolved

python/private/python.bzl Outdated Show resolved Hide resolved

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch from 039012f to 0d3a405 Compare October 30, 2024 06:19

ewianda changed the title ~~Feat configure coverage rc env variable~~ feat(toolchain): Extend Python Testing Toolchain with COVERAGE_RC Support Oct 30, 2024

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch 2 times, most recently from f256992 to 9ebd564 Compare October 30, 2024 13:35

ewianda commented Oct 30, 2024

View reviewed changes

examples/bzlmod/tests/BUILD.bazel Show resolved Hide resolved

ewianda commented Oct 30, 2024

View reviewed changes

python/private/py_test_toolchain.bzl Show resolved Hide resolved

aignas reviewed Oct 31, 2024

View reviewed changes

python/private/py_test_toolchain.bzl Outdated Show resolved Hide resolved

python/private/python_test.bzl Show resolved Hide resolved

examples/bzlmod/tests/BUILD.bazel Show resolved Hide resolved

ewianda mentioned this pull request Oct 31, 2024

feat(toolchain) Add coveragepy configuration attribute #2224

Closed

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch 2 times, most recently from 02d40e0 to 9e4665c Compare October 31, 2024 15:56

aignas reviewed Nov 1, 2024

View reviewed changes

examples/bzlmod/tests/BUILD.bazel Show resolved Hide resolved

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch 2 times, most recently from 0371be3 to 3060b83 Compare November 1, 2024 21:16

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch from de0204d to d7181bc Compare November 5, 2024 18:44

ewianda marked this pull request as ready for review November 5, 2024 18:53

ewianda requested a review from rickeylev as a code owner November 5, 2024 18:53

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch from d7181bc to 2270aa0 Compare November 5, 2024 20:03

ewianda requested a review from aignas November 5, 2024 20:03

aignas reviewed Dec 10, 2024

View reviewed changes

ewianda added 4 commits December 11, 2024 23:32

feat(toolchain): Python testing toolchain

a8c0e35

Inspired by https://github.com/trybka/scraps/blob/master/cc_test.md This PR extends Test Runner enviroment to provide a coveragerc enviroment variable COVERAGE_RC, allowing user to provide coverage resource in what ever format

Add env variable to rule transitions

2b30712

Move toolchain logic into get runtime details

f7d0f6b

Factor out Default and RuntimeInfo providers

acbd8c7

Create partially-bound function for configuring py_test

ewianda force-pushed the feat-configure-coverage-rc-env-variable branch from 2270aa0 to acbd8c7 Compare December 14, 2024 19:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(toolchain): Extend Python Testing Toolchain with COVERAGE_RC Support #2246

feat(toolchain): Extend Python Testing Toolchain with COVERAGE_RC Support #2246

ewianda commented Sep 24, 2024 •

edited

Loading

aignas left a comment

ewianda commented Oct 26, 2024

aignas left a comment

ewianda commented Oct 31, 2024

aignas commented Nov 1, 2024

aignas commented Nov 4, 2024

ewianda commented Nov 5, 2024 •

edited

Loading

ewianda commented Dec 7, 2024

aignas left a comment

aignas Dec 8, 2024

aignas Dec 9, 2024

aignas Dec 9, 2024

aignas Dec 9, 2024

aignas Dec 9, 2024

ewianda Dec 12, 2024

aignas Dec 9, 2024

aignas Dec 9, 2024

ewianda Dec 12, 2024 •

edited

Loading

		use_repo(python_test, "py_test_toolchain")
		register_toolchains("@py_test_toolchain//:all")

feat(toolchain): Extend Python Testing Toolchain with COVERAGE_RC Support #2246

Are you sure you want to change the base?

feat(toolchain): Extend Python Testing Toolchain with COVERAGE_RC Support #2246

Conversation

ewianda commented Sep 24, 2024 • edited Loading

aignas left a comment

Choose a reason for hiding this comment

ewianda commented Oct 26, 2024

aignas left a comment

Choose a reason for hiding this comment

ewianda commented Oct 31, 2024

aignas commented Nov 1, 2024

aignas commented Nov 4, 2024

ewianda commented Nov 5, 2024 • edited Loading

ewianda commented Dec 7, 2024

aignas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewianda Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

ewianda commented Sep 24, 2024 •

edited

Loading

ewianda commented Nov 5, 2024 •

edited

Loading

ewianda Dec 12, 2024 •

edited

Loading