Skip to content

Commit

Permalink
[6.2.0]Add test coverage support to android_local_test (bazelbuild#17467
Browse files Browse the repository at this point in the history
)

* Propagate runfiles for singlejar.

Since the singlejar toolchain attribute is currently defined as a single file and carried around as an Artifact instead of a FilesToRunProvider, it's not possible to implement it as a wrapper script that dispatches to an actual implementation somewhere in its runfiles. This would be useful to experiment with cross-platform action sharing.

PiperOrigin-RevId: 492487124
Change-Id: Ib0f80314eae09bd865b3f31a4180bf068833cdf4

* Add test coverage support to android_local_test

Adding test coverage support to `android_local_test`.

bazelbuild#15827

Closes bazelbuild#15840.

RELNOTES: Adds coverage metric support to android_local_test
PiperOrigin-RevId: 508549884
Change-Id: I6977efa51ca1c7a6df1f776fe1a326d07989a185

---------

Co-authored-by: Googler <tjgq@google.com>
Co-authored-by: Benjamin Lee <ben@ben.cm>
Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
  • Loading branch information
4 people committed Mar 13, 2023
1 parent 6c89303 commit b874e5f
Show file tree
Hide file tree
Showing 16 changed files with 359 additions and 33 deletions.
Expand Up @@ -27,6 +27,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:actions/custom_command_line",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/composing_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
Expand Down
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.bazel.rules.android;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Artifact;
Expand All @@ -34,6 +35,9 @@
/** An implementation for the "android_local_test" rule. */
public class BazelAndroidLocalTest extends AndroidLocalTestBase {

private static final String JACOCO_COVERAGE_RUNNER_MAIN_CLASS =
"com.google.testing.coverage.JacocoCoverageRunner";

public BazelAndroidLocalTest() {
super(BazelAndroidSemantics.INSTANCE);
}
Expand Down Expand Up @@ -76,9 +80,14 @@ protected String addCoverageSupport(
JavaTargetAttributes.Builder attributesBuilder,
String mainClass)
throws RuleErrorException {
// coverage does not yet work with android_local_test
ruleContext.throwWithRuleError("android_local_test does not yet support coverage");
return "";
// This method can be called only for *_binary/*_test targets.
Preconditions.checkNotNull(executable);

helper.addCoverageSupport();

// We do not add the instrumented jar to the runtime classpath, but provide it in the shell
// script via an environment variable.
return JACOCO_COVERAGE_RUNNER_MAIN_CLASS;
}

@Override
Expand Down
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses.BaseJavaBinaryRule;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.RuleClass;
Expand Down Expand Up @@ -82,7 +83,11 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
.removeAttribute("main_class")
.removeAttribute("resources")
.removeAttribute("use_testrunner")
.removeAttribute(":java_launcher")
.removeAttribute(":java_launcher") // Input files for test actions collecting code coverage
.add(
attr(":lcov_merger", LABEL)
.cfg(ExecutionTransitionFactory.create())
.value(BaseRuleClasses.getCoverageOutputGeneratorLabel()))
.cfg(
new ConfigFeatureFlagTransitionFactory(AndroidFeatureFlagSetProvider.FEATURE_FLAG_ATTR))
.build();
Expand Down
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
Expand Down Expand Up @@ -403,7 +404,7 @@ private static SpawnAction createAarEmbeddedJarsExtractorActions(
private static SpawnAction createAarJarsMergingActions(
RuleContext ruleContext, Artifact jarsTreeArtifact, Artifact mergedJar, Artifact paramFile) {
SpawnAction.Builder builder = new SpawnAction.Builder().useDefaultShellEnvironment();
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
return builder
.setExecutable(singleJar)
.setMnemonic("AarJarsMerger")
Expand Down
Expand Up @@ -1957,7 +1957,7 @@ private static SpawnAction.Builder createSpawnActionBuilder(RuleContext ruleCont
}

private static SpawnAction.Builder singleJarSpawnActionBuilder(RuleContext ruleContext) {
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
SpawnAction.Builder builder =
createSpawnActionBuilder(ruleContext).useDefaultShellEnvironment();
builder.setExecutable(singleJar);
Expand Down
Expand Up @@ -20,6 +20,7 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.Allowlist;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
Expand All @@ -29,6 +30,7 @@
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.SourceManifestAction;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.actions.Substitution;
import com.google.devtools.build.lib.analysis.actions.Template;
Expand Down Expand Up @@ -66,6 +68,7 @@
import com.google.devtools.build.lib.rules.java.OneVersionCheckActionBuilder;
import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -292,7 +295,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
originalMainClass,
filesToBuildBuilder,
javaExecutable,
/* createCoverageMetadataJar= */ true);
/* createCoverageMetadataJar= */ false);

Artifact oneVersionOutputArtifact = null;
JavaConfiguration javaConfig = ruleContext.getFragment(JavaConfiguration.class);
Expand Down Expand Up @@ -364,6 +367,60 @@ public ConfiguredTarget create(RuleContext ruleContext)

JavaInfo.Builder javaInfoBuilder = JavaInfo.Builder.create();

NestedSetBuilder<Pair<String, String>> coverageEnvironment = NestedSetBuilder.stableOrder();
NestedSetBuilder<Artifact> coverageSupportFiles = NestedSetBuilder.stableOrder();
if (ruleContext.getConfiguration().isCodeCoverageEnabled()) {

// Create an artifact that contains the runfiles relative paths of the jars on the runtime
// classpath. Using SourceManifestAction is the only reliable way to match the runfiles
// creation code.
Artifact runtimeClasspathArtifact =
ruleContext.getUniqueDirectoryArtifact(
"runtime_classpath_for_coverage",
"runtime_classpath.txt",
ruleContext.getBinOrGenfilesDirectory());
ruleContext.registerAction(
new SourceManifestAction(
SourceManifestAction.ManifestType.SOURCES_ONLY,
ruleContext.getActionOwner(),
runtimeClasspathArtifact,
new Runfiles.Builder(
ruleContext.getWorkspaceName(),
ruleContext.getConfiguration().legacyExternalRunfiles())
// This matches the code below in collectDefaultRunfiles.
.addTransitiveArtifactsWrappedInStableOrder(javaCommon.getRuntimeClasspath())
.build(),
null,
true));
filesToBuildBuilder.add(runtimeClasspathArtifact);

// Pass the artifact through an environment variable in the coverage environment so it
// can be read by the coverage collection script.
coverageEnvironment.add(
new Pair<>(
"JAVA_RUNTIME_CLASSPATH_FOR_COVERAGE", runtimeClasspathArtifact.getExecPathString()));
// Add the file to coverageSupportFiles so it ends up as an input for the test action
// when coverage is enabled.
coverageSupportFiles.add(runtimeClasspathArtifact);

// Make single jar reachable from the coverage environment because it needs to be executed
// by the coverage collection script.
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
coverageEnvironment.add(
new Pair<>("SINGLE_JAR_TOOL", singleJar.getExecutable().getExecPathString()));
coverageSupportFiles.addTransitive(singleJar.getFilesToRun());
}

javaCommon.addTransitiveInfoProviders(
builder,
javaInfoBuilder,
filesToBuild,
classJar,
coverageEnvironment.build(),
coverageSupportFiles.build());
javaCommon.addGenJarsProvider(
builder, javaInfoBuilder, outputs.genClass(), outputs.genSource());

javaCommon.addTransitiveInfoProviders(builder, javaInfoBuilder, filesToBuild, classJar);
javaCommon.addGenJarsProvider(
builder, javaInfoBuilder, outputs.genClass(), outputs.genSource());
Expand Down
Expand Up @@ -418,7 +418,7 @@ private void signApk(

private static void setSingleJarAsExecutable(
RuleContext ruleContext, SpawnAction.Builder builder) {
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
builder.setExecutable(singleJar);
}

Expand Down
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.actions.ParamFileInfo;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
Expand Down Expand Up @@ -420,7 +421,7 @@ public void build() throws InterruptedException {
inputs.add(libModules);
}

Artifact singlejar = JavaToolchainProvider.from(ruleContext).getSingleJar();
FilesToRunProvider singlejar = JavaToolchainProvider.from(ruleContext).getSingleJar();

String toolchainIdentifier = null;
try {
Expand Down
Expand Up @@ -31,6 +31,7 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.Allowlist;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
Expand Down Expand Up @@ -510,9 +511,10 @@ public ConfiguredTarget create(RuleContext ruleContext)

// Make single jar reachable from the coverage environment because it needs to be executed
// by the coverage collection script.
Artifact singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
coverageEnvironment.add(new Pair<>("SINGLE_JAR_TOOL", singleJar.getExecPathString()));
coverageSupportFiles.add(singleJar);
FilesToRunProvider singleJar = JavaToolchainProvider.from(ruleContext).getSingleJar();
coverageEnvironment.add(
new Pair<>("SINGLE_JAR_TOOL", singleJar.getExecutable().getExecPathString()));
coverageSupportFiles.addTransitive(singleJar.getFilesToRun());
}

common.addTransitiveInfoProviders(
Expand Down
Expand Up @@ -78,7 +78,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
.get("reduced_classpath_incompatible_processors", Type.STRING_LIST));
boolean forciblyDisableHeaderCompilation =
ruleContext.attributes().get("forcibly_disable_header_compilation", Type.BOOLEAN);
Artifact singleJar = ruleContext.getPrerequisiteArtifact("singlejar");
FilesToRunProvider singleJar = ruleContext.getExecutablePrerequisite("singlejar");
FilesToRunProvider oneVersion = ruleContext.getExecutablePrerequisite("oneversion");
Artifact oneVersionAllowlist = ruleContext.getPrerequisiteArtifact("oneversion_whitelist");
Artifact genClass = ruleContext.getPrerequisiteArtifact("genclass");
Expand Down
Expand Up @@ -112,7 +112,7 @@ public static JavaToolchainProvider create(
ImmutableSet<String> headerCompilerBuiltinProcessors,
ImmutableSet<String> reducedClasspathIncompatibleProcessors,
boolean forciblyDisableHeaderCompilation,
Artifact singleJar,
FilesToRunProvider singleJar,
@Nullable FilesToRunProvider oneVersion,
@Nullable Artifact oneVersionAllowlist,
Artifact genClass,
Expand Down Expand Up @@ -174,7 +174,7 @@ public static JavaToolchainProvider create(
private final ImmutableSet<String> headerCompilerBuiltinProcessors;
private final ImmutableSet<String> reducedClasspathIncompatibleProcessors;
private final boolean forciblyDisableHeaderCompilation;
private final Artifact singleJar;
private final FilesToRunProvider singleJar;
@Nullable private final FilesToRunProvider oneVersion;
@Nullable private final Artifact oneVersionAllowlist;
private final Artifact genClass;
Expand Down Expand Up @@ -208,7 +208,7 @@ private JavaToolchainProvider(
ImmutableSet<String> headerCompilerBuiltinProcessors,
ImmutableSet<String> reducedClasspathIncompatibleProcessors,
boolean forciblyDisableHeaderCompilation,
Artifact singleJar,
FilesToRunProvider singleJar,
@Nullable FilesToRunProvider oneVersion,
@Nullable Artifact oneVersionAllowlist,
Artifact genClass,
Expand Down Expand Up @@ -334,14 +334,14 @@ public boolean getForciblyDisableHeaderCompilation() {
return forciblyDisableHeaderCompilation;
}

/** Returns the {@link Artifact} of the SingleJar deploy jar */
/** Returns the {@link FilesToRunProvider} of the SingleJar tool. */
@Override
public Artifact getSingleJar() {
public FilesToRunProvider getSingleJar() {
return singleJar;
}

/**
* Return the {@link Artifact} of the binary that enforces one-version compliance of java
* Return the {@link FilesToRunProvider} of the tool that enforces one-version compliance of Java
* binaries.
*/
@Override
Expand Down
Expand Up @@ -153,7 +153,6 @@ The Java target version (e.g., '6' or '7'). It specifies for which Java runtime
.add(
attr("singlejar", LABEL_LIST)
.mandatory()
.singleArtifact()
// This needs to be in the execution configuration.
.cfg(ExecutionTransitionFactory.create())
.allowedFileTypes(FileTypeSet.ANY_FILE)
Expand Down
Expand Up @@ -49,13 +49,13 @@ public interface JavaToolchainStarlarkApiProviderApi extends StructApi {
@StarlarkMethod(name = "target_version", doc = "The java target version.", structField = true)
String getTargetVersion();

@StarlarkMethod(name = "single_jar", doc = "The SingleJar deploy jar.", structField = true)
FileApi getSingleJar();
@StarlarkMethod(name = "single_jar", doc = "The SingleJar tool.", structField = true)
FilesToRunProviderApi<? extends FileApi> getSingleJar();

@Nullable
@StarlarkMethod(
name = "one_version_tool",
doc = "The artifact that enforces One-Version compliance of java binaries.",
doc = "The tool that enforces One-Version compliance of java binaries.",
structField = true,
allowReturnNones = true)
FilesToRunProviderApi<? extends FileApi> getOneVersionBinary();
Expand Down
Expand Up @@ -69,16 +69,6 @@ public void testDisallowPrecompiledJars() throws Exception {
" srcs = ['lib.jar'])");
}

@Test
public void testCoverageThrowsError() throws Exception {
useConfiguration("--collect_code_coverage");
checkError("java/test",
"test",
"android_local_test does not yet support coverage",
"android_local_test(name = 'test',",
" srcs = ['test.java'])");
}

@Test
public void testNoAndroidAllJarsPropertiesFileThrowsError() throws Exception {
checkError("java/test",
Expand Down
19 changes: 19 additions & 0 deletions src/test/shell/bazel/android/BUILD
Expand Up @@ -49,6 +49,25 @@ android_sh_test(
],
)

android_sh_test(
name = "android_local_test_integration_test",
size = "large",
srcs = ["android_local_test_integration_test.sh"],
data = [
":android_helper",
# TODO(b/226204219): Remove this dep once Bazel properly loads d8
# in create_android_sdk_rules()
"//external:android/d8_jar_import",
"//external:android_sdk_for_testing",
"//src/test/shell/bazel:test-deps",
],
# See https://github.com/bazelbuild/bazel/issues/8235
tags = [
"no-remote",
"no_windows",
],
)

android_sh_test(
name = "aapt_integration_test",
size = "large",
Expand Down

0 comments on commit b874e5f

Please sign in to comment.