Charles Mita

I'm going to close this PR because I really think the coverage API needs some cleaning up and I don't want to increase its current surface area anymore at this...

Oops. Yes, this should be imported.

Do we need to handle version-skew between Bazel and java_tools? If so, we may need to add a bit more logic to JacocoCoverageRunner?

> > Do we need to handle version-skew between Bazel and java_tools? If so, we may need to add a bit more logic to JacocoCoverageRunner? > > Yes, I think...

> Just a quick clarification before I properly review, my understanding is this change is currently doing two things: > > 1. Export the manifest jar path from the stub...

I don't really get the rationale for that and it doesn't simplify the logic at all. The diff is simply: ``` @@ -380,7 +380,7 @@ public class JacocoCoverageRunner { System.err.println("Classpath...

> Lets not disable the tests. For the new test case that currently only works for java_tools built at HEAD, maybe exit early if `JAVA_TOOLS == 'released"`? > > Also...

What do you want to do with this? Use of Jacoco is very much an implementation detail as far as the Java rules are concerned and we make no promises...

> An idea from the sidelines: Since the stub script knows best whether it created a manifest jar, it could pass along its path as an environment variable or Java...

rules_scala has a pending PR to fix the issue: https://github.com/bazelbuild/rules_scala/pull/1554 rules_webtesting will then need to update the version of rules_scala used to one that includes that change.