From 2f847a4eda4219d5b32eb4fb28082f9835a55432 Mon Sep 17 00:00:00 2001 From: Benjamin Lee Date: Thu, 9 Feb 2023 20:19:33 -0800 Subject: [PATCH] Add test coverage support to android_local_test Adding test coverage support to `android_local_test`. https://github.com/bazelbuild/bazel/issues/15827 Closes #15840. RELNOTES: Adds coverage metric support to android_local_test PiperOrigin-RevId: 508549884 Change-Id: I6977efa51ca1c7a6df1f776fe1a326d07989a185 --- .../build/lib/bazel/rules/android/BUILD | 1 + .../rules/android/BazelAndroidLocalTest.java | 15 +- .../android/BazelAndroidLocalTestRule.java | 7 +- .../rules/android/AndroidLocalTestBase.java | 59 ++++- .../android/BazelAndroidLocalTestTest.java | 10 - src/test/shell/bazel/android/BUILD | 19 ++ .../android_local_test_integration_test.sh | 242 ++++++++++++++++++ 7 files changed, 338 insertions(+), 15 deletions(-) create mode 100755 src/test/shell/bazel/android/android_local_test_integration_test.sh diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD index 831d75a9cdf367..a5bb63d31a42a6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD @@ -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", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java index 4343ce6f09333e..8f366aaf2fbe40 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java @@ -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; @@ -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); } @@ -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 diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestRule.java index 0a91c6e0343850..1d44a3cab88653 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestRule.java @@ -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; @@ -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(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java index 33905dd3b0a9bf..3e7672e02753df 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java @@ -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; @@ -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; @@ -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; @@ -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); @@ -364,6 +367,60 @@ public ConfiguredTarget create(RuleContext ruleContext) JavaInfo.Builder javaInfoBuilder = JavaInfo.Builder.create(); + NestedSetBuilder> coverageEnvironment = NestedSetBuilder.stableOrder(); + NestedSetBuilder 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()); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestTest.java index 69c031fa812444..41a6cfaeb08b8f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTestTest.java @@ -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", diff --git a/src/test/shell/bazel/android/BUILD b/src/test/shell/bazel/android/BUILD index 3d01aadb4f8c94..f0b63e53795237 100644 --- a/src/test/shell/bazel/android/BUILD +++ b/src/test/shell/bazel/android/BUILD @@ -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", diff --git a/src/test/shell/bazel/android/android_local_test_integration_test.sh b/src/test/shell/bazel/android/android_local_test_integration_test.sh new file mode 100755 index 00000000000000..bb12631064b96d --- /dev/null +++ b/src/test/shell/bazel/android/android_local_test_integration_test.sh @@ -0,0 +1,242 @@ +#!/bin/bash + +# +# Copyright 2015 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# For these tests to run do the following: +# +# 1. Install an Android SDK from https://developer.android.com +# 2. Set the $ANDROID_HOME environment variable +# 3. Uncomment the line in WORKSPACE containing android_sdk_repository +# +# Note that if the environment is not set up as above android_integration_test +# will silently be ignored and will be shown as passing. + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +source "${CURRENT_DIR}/android_helper.sh" \ + || { echo "android_helper.sh not found!" >&2; exit 1; } +fail_if_no_android_sdk + +source "${CURRENT_DIR}/../../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +if [[ "$1" = '--with_platforms' ]]; then + # TODO(b/161709111): With platforms, the below fails with + # "no attribute `$android_sdk_toolchain_type`" on AspectAwareAttributeMapper. + echo "android_local_test_integration_test.sh does not support --with_platforms!" >&2 + exit 0 +fi + +function setup_android_local_test_env() { + mkdir -p java/com/bin/res/values + mkdir -p javatests/com/bin + + # Targets for android_local_test to depend on + cat > java/com/bin/BUILD < java/com/bin/AndroidManifest.xml < + +EOF + cat > java/com/bin/Collatz.java < java/com/bin/res/values/values.xml < + + +EOF + + # android_local_test targets + cat > javatests/com/bin/robolectric-deps.properties < javatests/com/bin/BUILD < javatests/com/bin/AndroidManifest.xml < + + +EOF + cat > javatests/com/bin/TestCollatz.java < +was not found in actual coverage report: +<$( cat "$output_file" )>" +} + +function test_android_local_test() { + create_new_workspace + setup_android_sdk_support + setup_android_local_test_env + + bazel clean + bazel test --test_output=all \ + //javatests/com/bin:test &>$TEST_log || fail "Tests for //javatests/com/bin:test failed" +} + +function test_android_local_test_with_coverage() { + create_new_workspace + setup_android_sdk_support + setup_android_local_test_env + + bazel clean + bazel coverage --test_output=all \ + //javatests/com/bin:test &>$TEST_log || fail "Test with coverage for //javatests/com/bin:test failed" + + local coverage_file_path="$( get_coverage_file_path_from_test_log )" + local expected_result="SF:java/com/bin/Collatz.java +FN:3,com/bin/Collatz:: ()V +FN:6,com/bin/Collatz::getCollatzFinal (I)I +FNDA:0,com/bin/Collatz:: ()V +FNDA:1,com/bin/Collatz::getCollatzFinal (I)I +FNF:2 +FNH:1 +BRDA:6,0,0,1 +BRDA:6,0,1,1 +BRDA:9,0,0,1 +BRDA:9,0,1,1 +BRF:4 +BRH:4 +DA:3,0 +DA:6,1 +DA:7,1 +DA:9,1 +DA:10,1 +DA:12,1 +LH:5 +LF:6 +end_of_record" + + assert_coverage_result "$expected_result" "$coverage_file_path" +} + +function test_android_local_test_with_combined_coverage() { + create_new_workspace + setup_android_sdk_support + setup_android_local_test_env + + bazel clean + bazel coverage --test_output=all --coverage_report_generator=@bazel_tools//tools/test:coverage_report_generator --combined_report=lcov \ + //javatests/com/bin:test &>$TEST_log || fail "Test with coverage for //javatests/com/bin:test failed" + + local coverage_file_path="./bazel-out/_coverage/_coverage_report.dat" + local expected_result="SF:java/com/bin/Collatz.java +FN:3,com/bin/Collatz:: ()V +FN:6,com/bin/Collatz::getCollatzFinal (I)I +FNDA:0,com/bin/Collatz:: ()V +FNDA:1,com/bin/Collatz::getCollatzFinal (I)I +FNF:2 +FNH:1 +BRDA:6,0,0,1 +BRDA:6,0,1,1 +BRDA:9,0,0,1 +BRDA:9,0,1,1 +BRF:4 +BRH:4 +DA:3,0 +DA:6,1 +DA:7,1 +DA:9,1 +DA:10,1 +DA:12,1 +LH:5 +LF:6 +end_of_record" + + assert_coverage_result "$expected_result" "$coverage_file_path" +} + +run_suite "android_local_test integration tests"