Skip to content

Conversation

fmeum
Copy link
Member

@fmeum fmeum commented May 4, 2025

They are run on the test action's execution platform, which is resolved for the test exec group, not the default one.

@fmeum fmeum force-pushed the coverage-test-platform branch from 7f1556c to d8f9953 Compare May 4, 2025 10:07
They are run on the test action's execution platform, which is resolved for the `test` exec group, not the default one.

Since this adds a reference to the `test` exec group, which is only defined for test rules, `py_executable` attrs must not merge in coverage attributes if the rule to be created is not a test rule.
@fmeum fmeum force-pushed the coverage-test-platform branch from d8f9953 to a1fe2ff Compare May 4, 2025 10:08
@fmeum fmeum marked this pull request as ready for review May 4, 2025 10:09
@fmeum fmeum requested review from aignas and rickeylev as code owners May 4, 2025 10:09
@aignas
Copy link
Collaborator

aignas commented May 4, 2025

LGHM, but I lack the context behind the reason why we are fixing it. I am +1 for merging but love if you could point me to extra docs.

Does this fix a particular issue or does this just make things more tidy?

@fmeum
Copy link
Member Author

fmeum commented May 4, 2025

The docs don't exist yet, but I'm adding them in bazelbuild/bazel#25996. We can link to that once it lands (and wait for it to get merged if you prefer that).

In a cross-platform build with custom exec constraints on either the test or default exec group, using the wrong exec group would potentially result in a failure to run the coverage tool (e.g. if the test runs on Linux, but "build" actions for the py_test build on macOS - this would try to run the tool in a macOS JDK on Linux).

@rickeylev
Copy link
Collaborator

Would this fix #2850 ? The error in that report seems related to the test toolchain

@fmeum
Copy link
Member Author

fmeum commented May 5, 2025

It wouldn't, but I will send a fix for that other issue.

@rickeylev rickeylev added this pull request to the merge queue May 5, 2025
Merged via the queue into bazel-contrib:main with commit 1492ae4 May 5, 2025
3 checks passed
@fmeum fmeum deleted the coverage-test-platform branch May 8, 2025 14:26
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.

3 participants