Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add jacocorunner to java_toolchain. #8378

Closed
wants to merge 4 commits into from

Conversation

iirina
Copy link
Contributor

@iirina iirina commented May 17, 2019

The jacoco runner was previously retrieved via an implicit attribute $jacocorunner on the java_binary and java_test rules. Retrieving the runner via the implicit attribute makes testing the tools in the java_tools release difficult and inconsistent (almost all other tools are defined via java_toolchain).

This PR makes jacocorunner part of java_toolchain. If jacocorunner is not found in the toolchain it falls back to the $jacocorunner attribute. The fallback behavior can be removed after a bazel release with this change.

@iirina iirina force-pushed the jacoco-java-toolchain branch from 6180000 to 95d8fcf Compare May 17, 2019 13:05
@iirina iirina changed the title Jacoco java toolchain Add jacocorunner to java_toolchain. May 17, 2019
@iirina iirina marked this pull request as ready for review May 17, 2019 13:10
@iirina iirina requested a review from lberki as a code owner May 17, 2019 13:10
@iirina iirina requested review from cushon and hlopko and removed request for lberki May 17, 2019 13:10
@cushon
Copy link
Contributor

cushon commented May 17, 2019

Looks OK. Can you add a test, or migrate existing tests to use the new attribute?

@iirina
Copy link
Contributor Author

iirina commented May 20, 2019

Actually we don't have any open-sourced tests for java_toolchain. I opened #8404 to keep track of open sourcing the tests. For now I added the $jacocorunner attribute to java_toolchain in java_tools. src/test/shell/bazel:bazel_java_test_jdk${java_version}_toolchain_head will run the java integration tests using the jacocorunner from the toolchain (among all other tools in the toolchain).

@iirina
Copy link
Contributor Author

iirina commented May 21, 2019

@hlopko Can you take a look today?

Copy link
Member

@hlopko hlopko left a comment

Choose a reason for hiding this comment

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

Thanks! \o/

This broke in 33f7b77, as the new code
path was not handling this case correctly.

Fixes bazelbuild#8402.

PiperOrigin-RevId: 249201968
@iirina iirina force-pushed the jacoco-java-toolchain branch from ed44034 to 9f4a0da Compare May 21, 2019 11:46
@iirina iirina requested a review from a team as a code owner May 21, 2019 11:46
@iirina iirina force-pushed the jacoco-java-toolchain branch from 9f4a0da to 5e7ad2f Compare May 21, 2019 11:47
@bazel-io bazel-io closed this in d654510 May 21, 2019
bazel-io pushed a commit that referenced this pull request May 22, 2019
BranchDetailAnalyzer should have also been updated as part of ff1f745 when the jacoco version in bazel was updated.

Changes like this will be avoided after #8378 is merged, because it enables testing with the java coverage tools at head.

Closes #8417.

PiperOrigin-RevId: 249400130
bazel-io pushed a commit that referenced this pull request May 22, 2019
Also enable jacocorunner in BUILD.java_tools since #8378 is now merged.

RELNOTES: None.
PiperOrigin-RevId: 249442764
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
The jacoco runner was previously retrieved via an implicit attribute `$jacocorunner` on the `java_binary` and `java_test` rules. Retrieving the runner via the implicit attribute makes testing the tools in the `java_tools` release difficult and inconsistent (almost all other tools are defined via `java_toolchain`).

This PR makes `jacocorunner` part of `java_toolchain`. If `jacocorunner` is not found in the toolchain it falls back to the `$jacocorunner` attribute. The fallback behavior can be removed after a bazel release with this change.

Closes bazelbuild#8378.

PiperOrigin-RevId: 249241175
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
BranchDetailAnalyzer should have also been updated as part of bazelbuild@ff1f745 when the jacoco version in bazel was updated.

Changes like this will be avoided after bazelbuild#8378 is merged, because it enables testing with the java coverage tools at head.

Closes bazelbuild#8417.

PiperOrigin-RevId: 249400130
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
Also enable jacocorunner in BUILD.java_tools since bazelbuild#8378 is now merged.

RELNOTES: None.
PiperOrigin-RevId: 249442764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants