Fix up newly discovered bugs in decoupled Android data processing

- Always generate a symbols file - the new resource processing pipeline likes
  knowing that this is non-null, and it shouldn't cost much extra to create.

- A misleading method signature (which I made) led to me forgetting about
  Proguard artifacts. Properly propogate them into the ResourceApk object.

- Don't get Aapt version directly from AndroidConfiguration - there's some
  additional logic in AndroidAaptVersion not exposed elsewhere

- Split library tests that expect resources and assets to be processed together
  to have a new version where they're processed seperately.

- Tests use ValidatedAndroidData interface rather than ResourceContainer object

- Properly move some LocalTest magic around resource JAR out of the old
  pipeline only, as it should apply to both old and new pipelines.

- Processing action defaults to empty resource and asset deps rather than null

RELNOTES: none
PiperOrigin-RevId: 195253161

1 files changed, 211 insertions, 13 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java
index 4e5f05e1d7..54ca21a176 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AarImportTest.java
@@ -91,7 +91,9 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testResourcesProvided() throws Exception {
+  public void testResourcesProvided_NotDecoupled() throws Exception {
+    useConfiguration("--noandroid_decouple_data_processing");
+
     ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:foo");
 
     NestedSet<ValidatedAndroidData> directResources =
@@ -113,7 +115,37 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testResourcesExtractor() throws Exception {
+  public void testResourcesProvided() throws Exception {
+    useConfiguration("--android_decouple_data_processing");
+
+    ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:foo");
+
+    NestedSet<ValidatedAndroidData> directResources =
+        aarImportTarget.get(AndroidResourcesInfo.PROVIDER).getDirectAndroidResources();
+    assertThat(directResources).hasSize(1);
+
+    ValidatedAndroidData resourceContainer = directResources.iterator().next();
+    assertThat(resourceContainer.getManifest()).isNotNull();
+
+    Artifact resourceTreeArtifact = Iterables.getOnlyElement(resourceContainer.getResources());
+    assertThat(resourceTreeArtifact.isTreeArtifact()).isTrue();
+    assertThat(resourceTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/resources/foo");
+
+    NestedSet<ParsedAndroidAssets> directAssets =
+        aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getDirectParsedAssets();
+    assertThat(directAssets).hasSize(1);
+
+    ParsedAndroidAssets assets = directAssets.iterator().next();
+    assertThat(assets.getSymbols()).isNotNull();
+
+    Artifact assetsTreeArtifact = Iterables.getOnlyElement(assets.getAssets());
+    assertThat(assetsTreeArtifact.isTreeArtifact()).isTrue();
+    assertThat(assetsTreeArtifact.getExecPathString()).endsWith("_aar/unzipped/assets/foo");
+  }
+
+  @Test
+  public void testResourcesExtractor_NotDecoupled() throws Exception {
+    useConfiguration("--noandroid_decouple_data_processing");
     ValidatedAndroidData resourceContainer =
         getConfiguredTarget("//a:foo")
             .get(AndroidResourcesInfo.PROVIDER)
@@ -125,9 +157,9 @@ public class AarImportTest extends BuildViewTestCase {
     Artifact assetsTreeArtifact = resourceContainer.getAssets().get(0);
     Artifact aarResourcesExtractor =
         getHostConfiguredTarget(
-            ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor")
-        .getProvider(FilesToRunProvider.class)
-        .getExecutable();
+                ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor")
+            .getProvider(FilesToRunProvider.class)
+            .getExecutable();
 
     assertThat(getGeneratingSpawnAction(resourceTreeArtifact).getArguments())
         .containsExactly(
@@ -141,8 +173,45 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testDepsCheckerActionExistsForLevelError() throws Exception {
-    useConfiguration("--experimental_import_deps_checking=ERROR");
+  public void testResourcesExtractor() throws Exception {
+    useConfiguration("--android_decouple_data_processing");
+    ValidatedAndroidData resourceContainer =
+        getConfiguredTarget("//a:foo")
+            .get(AndroidResourcesInfo.PROVIDER)
+            .getDirectAndroidResources()
+            .toList()
+            .get(0);
+
+    Artifact resourceTreeArtifact = resourceContainer.getResources().get(0);
+    Artifact aarResourcesExtractor =
+        getHostConfiguredTarget(
+                ruleClassProvider.getToolsRepository() + "//tools/android:aar_resources_extractor")
+            .getProvider(FilesToRunProvider.class)
+            .getExecutable();
+
+    ParsedAndroidAssets assets =
+        getConfiguredTarget("//a:foo")
+            .get(AndroidAssetsInfo.PROVIDER)
+            .getDirectParsedAssets()
+            .toList()
+            .get(0);
+    Artifact assetsTreeArtifact = assets.getAssets().get(0);
+
+    assertThat(getGeneratingSpawnAction(resourceTreeArtifact).getArguments())
+        .containsExactly(
+            aarResourcesExtractor.getExecPathString(),
+            "--input_aar",
+            "a/foo.aar",
+            "--output_res_dir",
+            resourceTreeArtifact.getExecPathString(),
+            "--output_assets_dir",
+            assetsTreeArtifact.getExecPathString());
+  }
+
+  @Test
+  public void testDepsCheckerActionExistsForLevelErrorNotDecoupled() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=ERROR", "--noandroid_decouple_data_processing");
     ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last");
     OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
     NestedSet<Artifact> outputGroup =
@@ -172,8 +241,50 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testDepsCheckerActionExistsForLevelStrictError() throws Exception {
-    useConfiguration("--experimental_import_deps_checking=STRICT_ERROR");
+  public void testDepsCheckerActionExistsForLevelError() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=ERROR", "--android_decouple_data_processing");
+    ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last");
+    OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
+    NestedSet<Artifact> outputGroup =
+        outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL);
+    assertThat(outputGroup).hasSize(2);
+
+    // We should force asset merging to happen
+    Artifact mergedAssetsZip =
+        aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getValidationResult();
+    assertThat(outputGroup).contains(mergedAssetsZip);
+
+    // Get the other artifact from the output group
+    Artifact artifact = ActionsTestUtil.getFirstArtifactEndingWith(outputGroup, ".txt");
+
+    assertThat(artifact.isTreeArtifact()).isFalse();
+    assertThat(artifact.getExecPathString())
+        .endsWith("_aar/last/aar_import_deps_checker_result.txt");
+
+    SpawnAction checkerAction = getGeneratingSpawnAction(artifact);
+    List<String> arguments = checkerAction.getArguments();
+    assertThat(arguments)
+        .containsAllOf(
+            "--bootclasspath_entry",
+            "--classpath_entry",
+            "--input",
+            "--output",
+            "--fail_on_errors");
+    ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/bar/classes_and_libs_merged.jar");
+    ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/baz/java/baz-ijar.jar");
+    ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/baz/classes_and_libs_merged.jar");
+    ensureArgumentsHaveClassEntryOptionWithSuffix(arguments, "/foo/classes_and_libs_merged.jar");
+    ensureArgumentsHaveClassEntryOptionWithSuffix(
+        arguments, "/intermediate/classes_and_libs_merged.jar");
+    assertThat(arguments.stream().filter(arg -> "--classpath_entry".equals(arg)).count())
+        .isEqualTo(5);
+  }
+
+  @Test
+  public void testDepsCheckerActionExistsForLevelStrictError_NotDecoupled() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=STRICT_ERROR", "--noandroid_decouple_data_processing");
     ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last");
     OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
     NestedSet<Artifact> outputGroup =
@@ -199,13 +310,84 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testDepsCheckerActionExistsForLevelWarning() throws Exception {
-    useConfiguration("--experimental_import_deps_checking=WARNING");
+  public void testDepsCheckerActionExistsForLevelStrictError() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=STRICT_ERROR", "--android_decouple_data_processing");
+    ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:last");
+    OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
+    NestedSet<Artifact> outputGroup =
+        outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL);
+    assertThat(outputGroup).hasSize(2);
+
+    // We should force asset merging to happen
+    Artifact mergedAssetsZip =
+        aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getValidationResult();
+    assertThat(outputGroup).contains(mergedAssetsZip);
+
+    // Get the other artifact from the output group
+    Artifact artifact = ActionsTestUtil.getFirstArtifactEndingWith(outputGroup, ".txt");
+    assertThat(artifact.isTreeArtifact()).isFalse();
+    assertThat(artifact.getExecPathString())
+        .endsWith("_aar/last/aar_import_deps_checker_result.txt");
+
+    SpawnAction checkerAction = getGeneratingSpawnAction(artifact);
+    List<String> arguments = checkerAction.getArguments();
+    assertThat(arguments)
+        .containsAllOf(
+            "--bootclasspath_entry",
+            "--classpath_entry",
+            "--input",
+            "--output",
+            "--fail_on_errors");
+    ensureArgumentsHaveClassEntryOptionWithSuffix(
+        arguments, "/intermediate/classes_and_libs_merged.jar");
+    assertThat(arguments.stream().filter(arg -> "--classpath_entry".equals(arg)).count())
+        .isEqualTo(1);
+  }
+
+  @Test
+  public void testDepsCheckerActionExistsForLevelWarning_NotDecoupled() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=WARNING", "--noandroid_decouple_data_processing");
     ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar");
     OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
     NestedSet<Artifact> outputGroup =
         outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL);
     Artifact artifact = Iterables.getOnlyElement(outputGroup);
+
+    assertThat(artifact.isTreeArtifact()).isFalse();
+    assertThat(artifact.getExecPathString())
+        .endsWith("_aar/bar/aar_import_deps_checker_result.txt");
+
+    SpawnAction checkerAction = getGeneratingSpawnAction(artifact);
+    List<String> arguments = checkerAction.getArguments();
+    assertThat(arguments)
+        .containsAllOf(
+            "--bootclasspath_entry",
+            "--classpath_entry",
+            "--input",
+            "--output",
+            "--nofail_on_errors");
+  }
+
+  @Test
+  public void testDepsCheckerActionExistsForLevelWarning() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=WARNING", "--android_decouple_data_processing");
+    ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar");
+    OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
+    NestedSet<Artifact> outputGroup =
+        outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL);
+    assertThat(outputGroup).hasSize(2);
+
+    // We should force asset merging to happen
+    Artifact mergedAssetsZip =
+        aarImportTarget.get(AndroidAssetsInfo.PROVIDER).getValidationResult();
+    assertThat(outputGroup).contains(mergedAssetsZip);
+
+    // Get the other artifact from the output group
+    Artifact artifact = ActionsTestUtil.getFirstArtifactEndingWith(outputGroup, ".txt");
+
     assertThat(artifact.isTreeArtifact()).isFalse();
     assertThat(artifact.getExecPathString())
         .endsWith("_aar/bar/aar_import_deps_checker_result.txt");
@@ -246,8 +428,9 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testDepsCheckerActionNotExistsForLevelOff() throws Exception {
-    useConfiguration("--experimental_import_deps_checking=OFF");
+  public void testDepsCheckerActionNotExistsForLevelOff_NotDecoupled() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=OFF", "--noandroid_decouple_data_processing");
     ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar");
     OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
     NestedSet<Artifact> outputGroup =
@@ -256,6 +439,21 @@ public class AarImportTest extends BuildViewTestCase {
   }
 
   @Test
+  public void testDepsCheckerActionNotExistsForLevelOff() throws Exception {
+    useConfiguration(
+        "--experimental_import_deps_checking=OFF", "--android_decouple_data_processing");
+    ConfiguredTarget aarImportTarget = getConfiguredTarget("//a:bar");
+    OutputGroupInfo outputGroupInfo = aarImportTarget.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
+
+    // The deps checker action should not exist, but we should still create an action for asset
+    // merging
+    NestedSet<Artifact> outputGroup =
+        outputGroupInfo.getOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL);
+    assertThat(outputGroup).hasSize(1);
+    assertThat(outputGroup.toList().get(0).getFilename()).isEqualTo("assets.zip");
+  }
+
+  @Test
   public void testNativeLibsProvided() throws Exception {
     ConfiguredTarget androidLibraryTarget = getConfiguredTarget("//java:lib");