Skip to content

feat(coverage): warn when bundled coverage tool has no wheel for requested python_version/platform#3766

Merged
aignas merged 3 commits into
bazel-contrib:mainfrom
Syndic:coverage-warn-when-missing
May 20, 2026
Merged

feat(coverage): warn when bundled coverage tool has no wheel for requested python_version/platform#3766
aignas merged 3 commits into
bazel-contrib:mainfrom
Syndic:coverage-warn-when-missing

Conversation

@Syndic
Copy link
Copy Markdown
Contributor

@Syndic Syndic commented May 10, 2026

Addresses review feedback on #3764:

NOTE: Starlark unit tests have no way to capture stdout/stderr, so we can't directly assert that the warning is printed. I am not sure if the refactoring-for-testability I have considered would be appreciated, so this PR is a minimal approach. (I will also prepare an alternative PR that includes a small refactor and some tests, in case that is preferred.) A logger has been added, and is used in testing.

Previously, when configure_coverage_tool = True was set but the bundled coverage.py wheel set had no entry for the requested (python_version, platform), coverage_dep returned None silently. The result was that bazel coverage produced empty per-test lcov files for py_test targets with no signal to the user that coverage was unconfigured.

Print a WARNING in that path so the misconfiguration is visible. Preserves the existing silent return for the windows branch, which is intentionally quiet because the upstream coverage wrapper does not support windows.

@Syndic Syndic requested review from aignas and rickeylev as code owners May 10, 2026 18:12
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates coverage_deps.bzl to issue a warning when a bundled coverage wheel is unavailable for a specific Python version and platform, replacing the previous silent failure. The warning includes instructions for manual configuration or version pinning to resolve the issue. The CHANGELOG.md was also updated to reflect this change. I have no feedback to provide as there were no review comments.

Comment thread python/private/coverage_deps.bzl Outdated
Copy link
Copy Markdown
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

Thanks! The general direction is good, when I have time I'll look at merging this.

Syndic added 3 commits May 19, 2026 19:38
…ested python_version/platform

Previously, when `configure_coverage_tool = True` was set but the bundled
`coverage.py` wheel set had no entry for the requested (python_version,
platform), `coverage_dep` returned None silently. The result was that
`bazel coverage` produced empty per-test lcov files for `py_test` targets
with no signal to the user that coverage was unconfigured.

Print a WARNING in that path so the misconfiguration is visible. Preserve
the existing silent return for the windows branch, which is intentionally
quiet because the upstream coverage wrapper does not support windows.
Per review on #3766, replace the raw `print(...)` warning with
`logger.warn(...)` from `repo_utils.logger`, with the logger threaded
in from the caller.

- `coverage_dep` takes an optional `logger` parameter and falls back to
  a default-constructed logger if none is supplied.
- `python_register_toolchains` accepts a private `_internal_module_ctx`
  kwarg (mirroring the existing `_internal_bzlmod_toolchain_call`
  pattern). When invoked from the bzlmod path, it builds the logger
  with the real `module_ctx` so module-root filtering applies (see
  #3760). For the
  WORKSPACE/macro path it constructs a minimal stand-in struct, which
  is all the logger needs.
- `python.bzl` passes `module_ctx` through.

Adds tests/coverage_deps/ with two cases (unsupported version warns,
windows stays silent) using the captured-printer pattern already
established in tests/pypi/hub_builder/hub_builder_tests.bzl.
Add a comment explaining that the supported-wheel path of coverage_dep
calls maybe(http_archive, ...) which calls native.existing_rule(),
which is only valid during BUILD/macro/finalizer evaluation, not during
rule analysis where rules_testing analysis tests run. The path is
covered end-to-end by real bazel coverage runs.
@Syndic Syndic force-pushed the coverage-warn-when-missing branch from 4686ae0 to 5668d54 Compare May 20, 2026 02:38
Copy link
Copy Markdown
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

Thank you!

@aignas aignas added this pull request to the merge queue May 20, 2026
Merged via the queue into bazel-contrib:main with commit f4ebb5b May 20, 2026
4 checks passed
@Syndic Syndic deleted the coverage-warn-when-missing branch May 20, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants