Skip to content

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

Open
Syndic wants to merge 3 commits intobazel-contrib:mainfrom
Syndic:coverage-warn-when-missing-tested
Open

feat(coverage): warn when bundled coverage tool has no wheel for requested python_version/platform with tests#3767
Syndic wants to merge 3 commits intobazel-contrib:mainfrom
Syndic:coverage-warn-when-missing-tested

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. This pr includes a small refactoring for testability and some tests. (If the refactor isn't desired, #3766 is the warning change only.)

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 added 3 commits May 10, 2026 10:24
…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.
Pull the (python_version, platform) -> (url, sha256) lookup out of
coverage_dep into a pure function so it can be unit-tested. The
side-effecting maybe(http_archive, ...) registration stays in
coverage_dep; the warning continues to fire from coverage_dep. No
behavior change.
Five cases:
- supported (version, platform) returns a (url, sha256) pair with the
  expected wheel-filename fragments and a 64-char sha
- cp314 is present in the bundled set (regression guard for the
  motivation behind the warning)
- the cp314 freethreaded variant is present (regression guard for
  freethreaded entries)
- an unsupported version (3.7) returns None — the path that triggers
  the warning in coverage_dep
- a windows platform returns None — the path that stays silent
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 introduces a warning message when the bundled coverage tool lacks a wheel for a specific Python version and platform, preventing silent failures. It refactors the dependency lookup into a testable coverage_url_sha256 function and adds a comprehensive test suite. Feedback suggests using implicit string concatenation for the warning message to align with Starlark style guidelines.

Comment thread python/private/coverage_deps.bzl
@Syndic Syndic force-pushed the coverage-warn-when-missing-tested branch from c8e8c02 to 2dc3dc3 Compare May 10, 2026 19:06
@aignas
Copy link
Copy Markdown
Collaborator

aignas commented May 10, 2026

NOTE: Starlark unit tests have no way to capture stdout/stderr, so we can't directly assert that the warning is printed. This pr includes a small refactoring for testability and some tests. (If the refactor isn't desired, #3766 is the warning change only.)

From what I remember you can create a logger and pass it a custom printer function that can be list.append and you will be able to verify the behaviour. Let me know if that is too difficult to implement and I can look deeper.

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