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

[6.2.0]Add test coverage support to android_local_test #17467

Merged
merged 4 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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