Add test for service loader configuration with feature splits

Bug: 157426812
Change-Id: I98ac21e5329c0d1c2ad9f9c0c7c5b3664a06459d
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java
index 8f6a2d6..3e3a02c 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterInlineRegression.java
@@ -38,7 +38,7 @@
 
   @Parameters(name = "{0}")
   public static TestParametersCollection params() {
-    return getTestParameters().withAllRuntimes().build();
+    return getTestParameters().withAllRuntimes().withAllApiLevels().build();
   }
 
   private final TestParameters parameters;
@@ -91,6 +91,7 @@
 
   @NeverMerge
   public abstract static class BaseSuperClass implements RunInterface {
+    @Override
     public void run() {
       System.out.println(getFromFeature());
     }
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
index 067f920..698f9da 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterMergeRegression.java
@@ -99,6 +99,7 @@
   @NeverMerge
   public static class BaseClass implements RunInterface {
 
+    @Override
     @NeverInline
     public void run() {
       System.out.println(BaseWithStatic.getBase42());
@@ -119,6 +120,7 @@
 
   public static class FeatureClass extends BaseClass {
 
+    @Override
     public void run() {
       super.run();
       System.out.println(AFeatureWithStatic.getFoobar());
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitServiceLoaderTest.java b/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitServiceLoaderTest.java
new file mode 100644
index 0000000..39a1b03
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitServiceLoaderTest.java
@@ -0,0 +1,231 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.dexsplitter;
+
+import static com.android.tools.r8.rewrite.ServiceLoaderRewritingTest.getServiceLoaderLoads;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.DataEntryResource;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.Pair;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.util.ServiceLoader;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class R8FeatureSplitServiceLoaderTest extends SplitterTestBase {
+
+  private final TestParameters parameters;
+
+  @Parameters(name = "{0}")
+  public static TestParametersCollection data() {
+    return getTestParameters().withDexRuntimes().withAllApiLevels().build();
+  }
+
+  public R8FeatureSplitServiceLoaderTest(TestParameters parameters) {
+    this.parameters = parameters;
+  }
+
+  @Test
+  public void testR8AllServiceConfigurationInBase() throws Exception {
+    Path base = temp.newFile("base.zip").toPath();
+    Path feature1Path = temp.newFile("feature1.zip").toPath();
+    Path feature2Path = temp.newFile("feature2.zip").toPath();
+    testForR8(parameters.getBackend())
+        .addProgramClasses(Base.class, I.class)
+        .setMinApi(parameters.getApiLevel())
+        .addKeepMainRule(Base.class)
+        .addFeatureSplit(
+            builder -> simpleSplitProvider(builder, feature1Path, temp, Feature1I.class))
+        .addFeatureSplit(
+            builder -> simpleSplitProvider(builder, feature2Path, temp, Feature2I.class))
+        .addDataEntryResources(
+            DataEntryResource.fromBytes(
+                StringUtils.lines(Feature1I.class.getTypeName(), Feature2I.class.getTypeName())
+                    .getBytes(),
+                "META-INF/services/" + I.class.getTypeName(),
+                Origin.unknown()))
+        .compile()
+        .inspect(
+            inspector -> {
+              // TODO(b/157426812): This should be 1.
+              assertEquals(0, getServiceLoaderLoads(inspector, Base.class));
+            })
+        .writeToZip(base)
+        .addRunClasspathFiles(feature1Path, feature2Path)
+        .run(parameters.getRuntime(), Base.class)
+        .assertSuccessWithOutputLines("Feature1I.foo()", "Feature2I.foo()");
+  }
+
+  @Test
+  public void testR8AllLoaded() throws Exception {
+    Path base = temp.newFile("base.zip").toPath();
+    Path feature1Path = temp.newFile("feature1.zip").toPath();
+    Path feature2Path = temp.newFile("feature2.zip").toPath();
+    testForR8(parameters.getBackend())
+        .addProgramClasses(Base.class, I.class)
+        .setMinApi(parameters.getApiLevel())
+        .addKeepMainRule(Base.class)
+        .addFeatureSplit(
+            builder ->
+                splitWithNonJavaFile(
+                    builder,
+                    feature1Path,
+                    temp,
+                    ImmutableList.of(
+                        new Pair<>(
+                            "META-INF/services/" + I.class.getTypeName(),
+                            StringUtils.lines(Feature1I.class.getTypeName()))),
+                    true,
+                    Feature1I.class))
+        .addFeatureSplit(
+            builder ->
+                splitWithNonJavaFile(
+                    builder,
+                    feature2Path,
+                    temp,
+                    ImmutableList.of(
+                        new Pair<>(
+                            "META-INF/services/" + I.class.getTypeName(),
+                            StringUtils.lines(Feature2I.class.getTypeName()))),
+                    true,
+                    Feature2I.class))
+        .compile()
+        .inspect(
+            inspector -> {
+              assertEquals(1, getServiceLoaderLoads(inspector, Base.class));
+            })
+        .writeToZip(base)
+        .addRunClasspathFiles(feature1Path, feature2Path)
+        .run(parameters.getRuntime(), Base.class)
+        // TODO(b/157426812): Should output
+        // .assertSuccessWithOutputLines("Feature1I.foo()", "Feature2I.foo()");
+        .assertSuccessWithOutput("");
+  }
+
+  @Test
+  public void testR8WithServiceFileInSeparateFeature() throws Exception {
+    Path base = temp.newFile("base.zip").toPath();
+    Path feature1Path = temp.newFile("feature1.zip").toPath();
+    Path feature2Path = temp.newFile("feature2.zip").toPath();
+    Path feature3Path = temp.newFile("feature3.zip").toPath();
+    testForR8(parameters.getBackend())
+        .addProgramClasses(Base.class, I.class)
+        .setMinApi(parameters.getApiLevel())
+        .addKeepMainRule(Base.class)
+        .addFeatureSplit(
+            builder -> simpleSplitProvider(builder, feature1Path, temp, Feature1I.class))
+        .addFeatureSplit(
+            builder -> simpleSplitProvider(builder, feature2Path, temp, Feature2I.class))
+        .addFeatureSplit(
+            builder ->
+                splitWithNonJavaFile(
+                    builder,
+                    feature3Path,
+                    temp,
+                    ImmutableList.of(
+                        new Pair<>(
+                            "META-INF/services/" + I.class.getTypeName(),
+                            StringUtils.lines(
+                                Feature1I.class.getTypeName(), Feature2I.class.getTypeName()))),
+                    true,
+                    Feature3Dummy.class))
+        .compile()
+        .inspect(
+            inspector -> {
+              assertEquals(1, getServiceLoaderLoads(inspector, Base.class));
+            })
+        .writeToZip(base)
+        .addRunClasspathFiles(feature1Path, feature2Path, feature3Path)
+        .run(parameters.getRuntime(), Base.class)
+        // TODO(b/157426812): Should output
+        // .assertSuccessWithOutputLines("Feature1I.foo()", "Feature2I.foo()");
+        .assertSuccessWithOutput("");
+  }
+
+  @Test
+  public void testR8NotFound() throws Exception {
+    Path base = temp.newFile("base.zip").toPath();
+    Path feature1Path = temp.newFile("feature1.zip").toPath();
+    Path feature2Path = temp.newFile("feature2.zip").toPath();
+    testForR8(parameters.getBackend())
+        .addProgramClasses(Base.class, I.class)
+        .setMinApi(parameters.getApiLevel())
+        .addKeepMainRule(Base.class)
+        .addFeatureSplit(
+            builder ->
+                splitWithNonJavaFile(
+                    builder,
+                    feature1Path,
+                    temp,
+                    ImmutableList.of(
+                        new Pair<>(
+                            "META-INF/services/" + I.class.getTypeName(),
+                            StringUtils.lines(Feature1I.class.getTypeName()))),
+                    true,
+                    Feature1I.class))
+        .addFeatureSplit(
+            builder ->
+                splitWithNonJavaFile(
+                    builder,
+                    feature2Path,
+                    temp,
+                    ImmutableList.of(
+                        new Pair<>(
+                            "META-INF/services/" + I.class.getTypeName(),
+                            StringUtils.lines(Feature2I.class.getTypeName()))),
+                    true,
+                    Feature2I.class))
+        .compile()
+        .inspect(
+            inspector -> {
+              assertEquals(1, getServiceLoaderLoads(inspector, Base.class));
+            })
+        .writeToZip(base)
+        .addRunClasspathFiles(feature2Path)
+        .run(parameters.getRuntime(), Base.class)
+        // TODO(b/157426812): Should output
+        // .assertSuccessWithOutputLines("Feature2I.foo()");
+        .assertSuccessWithOutput("");
+  }
+
+  public interface I {
+    void foo();
+  }
+
+  public static class Base {
+
+    public static void main(String[] args) {
+      for (I i : ServiceLoader.load(I.class, null)) {
+        i.foo();
+      }
+    }
+  }
+
+  public static class Feature1I implements I {
+
+    @Override
+    public void foo() {
+      System.out.println("Feature1I.foo()");
+    }
+  }
+
+  public static class Feature2I implements I {
+
+    @Override
+    public void foo() {
+      System.out.println("Feature2I.foo()");
+    }
+  }
+
+  public static class Feature3Dummy {}
+}
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitTest.java b/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitTest.java
index 6f609de..60477b6 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitTest.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/R8FeatureSplitTest.java
@@ -18,6 +18,7 @@
 import com.android.tools.r8.TestParametersCollection;
 import com.android.tools.r8.ToolHelper.ProcessResult;
 import com.android.tools.r8.dex.Marker;
+import com.android.tools.r8.utils.Pair;
 import com.android.tools.r8.utils.StringUtils;
 import com.android.tools.r8.utils.codeinspector.CodeInspector;
 import com.google.common.collect.ImmutableList;
@@ -40,7 +41,7 @@
 
   @Parameters(name = "{0}")
   public static TestParametersCollection params() {
-    return getTestParameters().withDexRuntimes().build();
+    return getTestParameters().withDexRuntimes().withAllApiLevels().build();
   }
 
   private final TestParameters parameters;
@@ -122,11 +123,12 @@
     Path basePath = temp.newFile("base.zip").toPath();
     Path feature1Path = temp.newFile("feature1.zip").toPath();
     Path feature2Path = temp.newFile("feature2.zip").toPath();
-    Collection<String> nonJavaFiles = ImmutableList.of("foobar", "barfoo");
+    Collection<Pair<String, String>> nonJavaFiles =
+        ImmutableList.of(new Pair<>("foobar", "foobar"), new Pair<>("barfoo", "barfoo"));
 
     testForR8(parameters.getBackend())
         .addProgramClasses(BaseClass.class, RunInterface.class, SplitRunner.class)
-        .setMinApi(parameters.getRuntime())
+        .setMinApi(parameters.getApiLevel())
         .addFeatureSplit(
             builder ->
                 splitWithNonJavaFile(
@@ -140,16 +142,16 @@
         .writeToZip(basePath);
     for (Path feature : ImmutableList.of(feature1Path, feature2Path)) {
       ZipFile zipFile = new ZipFile(feature.toFile());
-      for (String nonJavaFile : nonJavaFiles) {
-        ZipEntry entry = zipFile.getEntry(nonJavaFile);
+      for (Pair<String, String> nonJavaFile : nonJavaFiles) {
+        ZipEntry entry = zipFile.getEntry(nonJavaFile.getFirst());
         assertNotNull(entry);
         String content = new String(ByteStreams.toByteArray(zipFile.getInputStream(entry)));
-        assertEquals(content, nonJavaFile);
+        assertEquals(content, nonJavaFile.getSecond());
       }
     }
     ZipFile zipFile = new ZipFile(basePath.toFile());
-    for (String nonJavaFile : nonJavaFiles) {
-      ZipEntry entry = zipFile.getEntry(nonJavaFile);
+    for (Pair<String, String> nonJavaFile : nonJavaFiles) {
+      ZipEntry entry = zipFile.getEntry(nonJavaFile.getFirst());
       assertNull(entry);
     }
   }
@@ -160,12 +162,14 @@
     Path feature1Path = temp.newFile("feature1.zip").toPath();
     Path feature2Path = temp.newFile("feature2.zip").toPath();
     // Make the content of the data resource be class names
-    Collection<String> nonJavaFiles =
-        ImmutableList.of(FeatureClass.class.getName(), FeatureClass2.class.getName());
+    Collection<Pair<String, String>> nonJavaFiles =
+        ImmutableList.of(
+            new Pair<>(FeatureClass.class.getName(), FeatureClass.class.getName()),
+            new Pair<>(FeatureClass2.class.getName(), FeatureClass2.class.getName()));
 
     testForR8(parameters.getBackend())
         .addProgramClasses(BaseClass.class, RunInterface.class, SplitRunner.class)
-        .setMinApi(parameters.getRuntime())
+        .setMinApi(parameters.getApiLevel())
         .addFeatureSplit(
             builder ->
                 splitWithNonJavaFile(
@@ -181,16 +185,16 @@
         .writeToZip(basePath);
     for (Path feature : ImmutableList.of(feature1Path, feature2Path)) {
       ZipFile zipFile = new ZipFile(feature.toFile());
-      for (String nonJavaFile : nonJavaFiles) {
-        ZipEntry entry = zipFile.getEntry(nonJavaFile);
+      for (Pair<String, String> nonJavaFile : nonJavaFiles) {
+        ZipEntry entry = zipFile.getEntry(nonJavaFile.getFirst());
         assertNotNull(entry);
         String content = new String(ByteStreams.toByteArray(zipFile.getInputStream(entry)));
-        assertNotEquals(content, nonJavaFile);
+        assertNotEquals(content, nonJavaFile.getSecond());
       }
     }
     ZipFile zipFile = new ZipFile(basePath.toFile());
-    for (String nonJavaFile : nonJavaFiles) {
-      ZipEntry entry = zipFile.getEntry(nonJavaFile);
+    for (Pair<String, String> nonJavaFile : nonJavaFiles) {
+      ZipEntry entry = zipFile.getEntry(nonJavaFile.getFirst());
       assertNull(entry);
     }
   }
@@ -261,7 +265,7 @@
 
       testForR8(parameters.getBackend())
           .addProgramClasses(BaseClass.class, RunInterface.class, SplitRunner.class)
-          .setMinApi(parameters.getRuntime())
+          .setMinApi(parameters.getApiLevel())
           .addFeatureSplit(
               builder -> simpleSplitProvider(builder, feature1Path, temp, FeatureClass.class))
           .addFeatureSplit(
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/R8SplitterInlineToFeature.java b/src/test/java/com/android/tools/r8/dexsplitter/R8SplitterInlineToFeature.java
index 331403a..4e4d9b0 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/R8SplitterInlineToFeature.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/R8SplitterInlineToFeature.java
@@ -72,6 +72,7 @@
 
   @NeverMerge
   public abstract static class BaseSuperClass implements RunInterface {
+    @Override
     public void run() {
       System.out.println(getFromFeature());
     }
diff --git a/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java b/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
index 16291c4..536d19d 100644
--- a/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
+++ b/src/test/java/com/android/tools/r8/dexsplitter/SplitterTestBase.java
@@ -1,6 +1,7 @@
 package com.android.tools.r8.dexsplitter;
 
 import static junit.framework.TestCase.assertTrue;
+import static junit.framework.TestCase.fail;
 import static org.junit.Assume.assumeTrue;
 
 import com.android.tools.r8.ByteDataView;
@@ -19,6 +20,7 @@
 import com.android.tools.r8.dexsplitter.DexSplitter.Options;
 import com.android.tools.r8.utils.ArchiveResourceProvider;
 import com.android.tools.r8.utils.DescriptorUtils;
+import com.android.tools.r8.utils.Pair;
 import com.android.tools.r8.utils.ThrowingConsumer;
 import com.android.tools.r8.utils.ZipUtils;
 import com.google.common.collect.ImmutableList;
@@ -42,11 +44,11 @@
 public class SplitterTestBase extends TestBase {
 
   public static FeatureSplit simpleSplitProvider(
-      FeatureSplit.Builder builder, Path outputPath, TemporaryFolder temp, Class... classes) {
+      FeatureSplit.Builder builder, Path outputPath, TemporaryFolder temp, Class<?>... classes) {
     return simpleSplitProvider(builder, outputPath, temp, Arrays.asList(classes));
   }
 
-  public static FeatureSplit simpleSplitProvider(
+  private static FeatureSplit simpleSplitProvider(
       FeatureSplit.Builder builder,
       Path outputPath,
       TemporaryFolder temp,
@@ -59,7 +61,7 @@
       FeatureSplit.Builder builder,
       Path outputPath,
       TemporaryFolder temp,
-      Collection<String> nonJavaResources,
+      Collection<Pair<String, String>> nonJavaResources,
       boolean ensureClassesInOutput,
       Collection<Class<?>> classes) {
     List<String> classNames = classes.stream().map(Class::getName).collect(Collectors.toList());
@@ -82,15 +84,18 @@
           next = inputStream.getNextEntry();
         }
 
-        for (String nonJavaResource : nonJavaResources) {
+        for (Pair<String, String> nonJavaResource : nonJavaResources) {
           ZipUtils.writeToZipStream(
-              outputStream, nonJavaResource, nonJavaResource.getBytes(), ZipEntry.STORED);
+              outputStream,
+              nonJavaResource.getFirst(),
+              nonJavaResource.getSecond().getBytes(),
+              ZipEntry.STORED);
         }
         outputStream.close();
         featureJar = newFeatureJar;
       }
     } catch (IOException e) {
-      assertTrue(false);
+      fail();
       return;
     }
 
@@ -115,18 +120,19 @@
             });
   }
 
-  protected static FeatureSplit splitWithNonJavaFile(
+  static FeatureSplit splitWithNonJavaFile(
       FeatureSplit.Builder builder,
       Path outputPath,
       TemporaryFolder temp,
-      Collection<String> nonJavaFiles,
+      Collection<Pair<String, String>> nonJavaFiles,
       boolean ensureClassesInOutput,
       Class<?>... classes) {
-    addConsumers(builder, outputPath, temp, nonJavaFiles, true, Arrays.asList(classes));
+    addConsumers(
+        builder, outputPath, temp, nonJavaFiles, ensureClassesInOutput, Arrays.asList(classes));
     return builder.build();
   }
 
-  protected <E extends Throwable> ProcessResult testR8Splitter(
+  <E extends Throwable> ProcessResult testR8Splitter(
       TestParameters parameters,
       Set<Class<?>> baseClasses,
       Set<Class<?>> featureClasses,
@@ -150,7 +156,7 @@
         .addProgramClasses(baseClasses)
         .addFeatureSplit(
             builder -> simpleSplitProvider(builder, featureOutput, temp, featureClasses))
-        .setMinApi(parameters.getRuntime())
+        .setMinApi(parameters.getApiLevel())
         .addKeepMainRule(SplitRunner.class)
         .addKeepClassRules(toRun);
 
@@ -165,7 +171,7 @@
 
   // Compile the passed in classes plus RunInterface and SplitRunner using R8, then split
   // based on the base/feature sets. toRun must implement the BaseRunInterface
-  protected <E extends Throwable> ProcessResult testDexSplitter(
+  <E extends Throwable> ProcessResult testDexSplitter(
       TestParameters parameters,
       Set<Class<?>> baseClasses,
       Set<Class<?>> featureClasses,
@@ -189,7 +195,7 @@
             .addClasspathClasses(baseClasses)
             .addClasspathClasses(RunInterface.class)
             .addKeepAllClassesRule()
-            .setMinApi(parameters.getRuntime())
+            .setMinApi(parameters.getApiLevel())
             .compile()
             .writeToZip();
     if (parameters.isDexRuntime()) {
@@ -199,7 +205,7 @@
       testForD8()
           .addProgramClasses(SplitRunner.class, RunInterface.class)
           .addProgramClasses(baseClasses)
-          .setMinApi(parameters.getRuntime())
+          .setMinApi(parameters.getApiLevel())
           .compile()
           .run(
               parameters.getRuntime(),
@@ -220,7 +226,7 @@
 
     R8FullTestBuilder r8FullTestBuilder =
         builder
-            .setMinApi(parameters.getRuntime())
+            .setMinApi(parameters.getApiLevel())
             .addProgramClasses(SplitRunner.class, RunInterface.class)
             .addProgramClasses(baseClasses)
             .addProgramClasses(featureClasses)
@@ -252,7 +258,7 @@
         toRun, splitterBaseDexFile, splitterFeatureDexFile, parameters.getRuntime());
   }
 
-  protected ProcessResult runFeatureOnArt(
+  ProcessResult runFeatureOnArt(
       Class toRun, Path splitterBaseDexFile, Path splitterFeatureDexFile, TestRuntime runtime)
       throws IOException {
     assumeTrue(runtime.isDex());
@@ -261,8 +267,7 @@
     commandBuilder.appendProgramArgument(toRun.getName());
     commandBuilder.appendProgramArgument(splitterFeatureDexFile.toString());
     commandBuilder.setMainClass(SplitRunner.class.getName());
-    ProcessResult processResult = ToolHelper.runArtRaw(commandBuilder);
-    return processResult;
+    return ToolHelper.runArtRaw(commandBuilder);
   }
 
   public interface RunInterface {
diff --git a/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderRewritingTest.java b/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderRewritingTest.java
index 5f46a45..5e18734 100644
--- a/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderRewritingTest.java
+++ b/src/test/java/com/android/tools/r8/rewrite/ServiceLoaderRewritingTest.java
@@ -326,7 +326,7 @@
     assertNotNull(zip.getEntry("META-INF/services/" + service.getFinalName()));
   }
 
-  private static long getServiceLoaderLoads(CodeInspector inspector, Class<?> clazz) {
+  public static long getServiceLoaderLoads(CodeInspector inspector, Class<?> clazz) {
     ClassSubject classSubject = inspector.clazz(clazz);
     assertTrue(classSubject.isPresent());
     return classSubject.allMethods().stream()