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 authored Mar 13, 2023
1 parent 6c89303 commit b874e5f
Show file tree
Hide file tree
Showing 16 changed files with 359 additions and 33 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit b874e5f

Please sign in to comment.