Fix ServiceLoader tests checking the wrong META-INF files The tests were not taking into account class renaming / merging, and that META-INF files are not actually removed. Also adds a test for ServiceLoader where no META-INF is present. Bug: 291923475 Change-Id: I7ed52413ad58a65bfe57ba662eb6ca90980c37c3
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderClassLoaderRewritingTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderClassLoaderRewritingTest.java index 1df6b3a..9695e90 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderClassLoaderRewritingTest.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderClassLoaderRewritingTest.java
@@ -4,25 +4,17 @@ package com.android.tools.r8.optimize.serviceloader; -import static junit.framework.TestCase.assertNull; - -import com.android.tools.r8.DataEntryResource; import com.android.tools.r8.NeverInline; 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.StringUtils; -import java.nio.file.Path; import java.util.ServiceLoader; -import java.util.zip.ZipFile; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ServiceLoaderClassLoaderRewritingTest extends ServiceLoaderTestBase { - - private final TestParameters parameters; private final String EXPECTED_OUTPUT = StringUtils.lines("Hello World!"); public interface Service { @@ -67,30 +59,17 @@ } public ServiceLoaderClassLoaderRewritingTest(TestParameters parameters) { - this.parameters = parameters; + super(parameters); } @Test public void testRewritings() throws Exception { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderClassLoaderRewritingTest.class) + serviceLoaderTest(Service.class, ServiceImpl.class) .addKeepMainRule(MainRunner.class) - .setMinApi(parameters) .enableInliningAnnotations() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .compile() - .writeToZip(path) .inspect(inspector -> verifyNoServiceLoaderLoads(inspector.clazz(MainRunner.class))) .run(parameters.getRuntime(), MainRunner.class) .assertSuccessWithOutput(EXPECTED_OUTPUT); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName())); } }
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderConstClassFromCalleeTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderConstClassFromCalleeTest.java index da90e4a..a6556a2 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderConstClassFromCalleeTest.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderConstClassFromCalleeTest.java
@@ -5,41 +5,30 @@ import static junit.framework.TestCase.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.StringUtils; import java.util.ServiceLoader; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; @RunWith(Parameterized.class) public class ServiceLoaderConstClassFromCalleeTest extends ServiceLoaderTestBase { - @Parameter(0) - public TestParameters parameters; - @Parameters(name = "{0}") public static TestParametersCollection data() { return getTestParameters().withAllRuntimesAndApiLevels().build(); } + public ServiceLoaderConstClassFromCalleeTest(TestParameters parameters) { + super(parameters); + } + @Test public void test() throws Exception { - testForR8(parameters.getBackend()) - .addInnerClasses(getClass()) + serviceLoaderTest(Service.class, ServiceImpl.class, ServiceImpl2.class) .addKeepMainRule(Main.class) - .setMinApi(parameters) - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName(), ServiceImpl2.class.getTypeName()) - .getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .run(parameters.getRuntime(), Main.class) .assertSuccessWithOutputLines("Hello, world!") // Check that the call to ServiceLoader.load is removed.
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsSameMethodTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsSameMethodTest.java index 149fd33..111badc 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsSameMethodTest.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsSameMethodTest.java
@@ -5,31 +5,24 @@ package com.android.tools.r8.optimize.serviceloader; import static junit.framework.TestCase.assertEquals; -import static junit.framework.TestCase.assertNull; import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertFalse; import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.DataEntryResource; import com.android.tools.r8.NeverInline; 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.StringUtils; import com.android.tools.r8.utils.codeinspector.FoundClassSubject; import java.io.IOException; -import java.nio.file.Path; import java.util.ServiceLoader; import java.util.concurrent.ExecutionException; -import java.util.zip.ZipFile; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ServiceLoaderMultipleCallsSameMethodTest extends ServiceLoaderTestBase { - - private final TestParameters parameters; private final String EXPECTED_OUTPUT = StringUtils.lines("Hello World!", "Hello World!"); public interface Service { @@ -45,13 +38,6 @@ } } - public static class ServiceImpl2 implements Service { - - @Override - public void print() { - System.out.println("Hello World 2!"); - } - } public static class MainRunner { @@ -77,31 +63,22 @@ } public ServiceLoaderMultipleCallsSameMethodTest(TestParameters parameters) { - this.parameters = parameters; + super(parameters); } @Test public void testRewritings() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderMultipleCallsSameMethodTest.class) + serviceLoaderTest(Service.class, ServiceImpl.class) .addKeepMainRule(MainRunner.class) - .setMinApi(parameters) .enableInliningAnnotations() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .compile() - .writeToZip(path) .run(parameters.getRuntime(), MainRunner.class) .assertSuccessWithOutput(EXPECTED_OUTPUT) - // Check that we have actually rewritten the calls to ServiceLoader.load. - .inspect(this::verifyNoServiceLoaderLoads) - .inspect(this::verifyNoClassLoaders) .inspect( inspector -> { + verifyNoServiceLoaderLoads(inspector); + verifyNoClassLoaders(inspector); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class); // Check the synthesize service loader method is a single shared method. // Due to minification we just check there is only a single synthetic class with a // single static method. @@ -115,9 +92,5 @@ } } }); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName())); } }
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsTest.java index c688bd4..5b2d452 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsTest.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderMultipleCallsTest.java
@@ -5,31 +5,24 @@ package com.android.tools.r8.optimize.serviceloader; import static junit.framework.TestCase.assertEquals; -import static junit.framework.TestCase.assertNull; import static junit.framework.TestCase.assertTrue; import static org.junit.Assert.assertFalse; import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.DataEntryResource; import com.android.tools.r8.NeverInline; 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.StringUtils; import com.android.tools.r8.utils.codeinspector.FoundClassSubject; import java.io.IOException; -import java.nio.file.Path; import java.util.ServiceLoader; import java.util.concurrent.ExecutionException; -import java.util.zip.ZipFile; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ServiceLoaderMultipleCallsTest extends ServiceLoaderTestBase { - - private final TestParameters parameters; private final String EXPECTED_OUTPUT = StringUtils.lines("Hello World!", "Hello World!"); public interface Service { @@ -45,14 +38,6 @@ } } - public static class ServiceImpl2 implements Service { - - @Override - public void print() { - System.out.println("Hello World 2!"); - } - } - public static class MainRunner { public static void main(String[] args) { @@ -81,31 +66,22 @@ } public ServiceLoaderMultipleCallsTest(TestParameters parameters) { - this.parameters = parameters; + super(parameters); } @Test public void testRewritings() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderMultipleCallsTest.class) + serviceLoaderTest(Service.class, ServiceImpl.class) .addKeepMainRule(MainRunner.class) - .setMinApi(parameters) .enableInliningAnnotations() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .compile() - .writeToZip(path) .run(parameters.getRuntime(), MainRunner.class) .assertSuccessWithOutput(EXPECTED_OUTPUT) .inspect( inspector -> { // Check that we have actually rewritten the calls to ServiceLoader.load. assertEquals(0, getServiceLoaderLoads(inspector)); - // Check the synthesize service loader method is a single shared method. + // Check the synthesized service loader method is a single shared method. // Due to minification we just check there is only a single synthetic class with a // single static method. boolean found = false; @@ -118,9 +94,5 @@ } } }); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName())); } }
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingLineSeparatorTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingLineSeparatorTest.java index 7610575..c142442 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingLineSeparatorTest.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingLineSeparatorTest.java
@@ -5,7 +5,6 @@ package com.android.tools.r8.optimize.serviceloader; import static junit.framework.TestCase.assertEquals; -import static junit.framework.TestCase.assertNull; import com.android.tools.r8.CompilationFailedException; import com.android.tools.r8.DataEntryResource; @@ -13,10 +12,8 @@ import com.android.tools.r8.origin.Origin; import com.android.tools.r8.utils.StringUtils; import java.io.IOException; -import java.nio.file.Path; import java.util.List; import java.util.concurrent.ExecutionException; -import java.util.zip.ZipFile; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -24,8 +21,6 @@ @RunWith(Parameterized.class) public class ServiceLoaderRewritingLineSeparatorTest extends ServiceLoaderTestBase { - - private final TestParameters parameters; private final Separator lineSeparator; private final String EXPECTED_OUTPUT = @@ -55,18 +50,16 @@ } public ServiceLoaderRewritingLineSeparatorTest(TestParameters parameters, Separator separator) { - this.parameters = parameters; + super(parameters); this.lineSeparator = separator; } @Test public void testRewritingWithMultipleWithLineSeparator() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) + serviceLoaderTest(null) .addInnerClasses(ServiceLoaderRewritingTest.class) .addKeepMainRule(ServiceLoaderRewritingTest.MainRunner.class) - .setMinApi(parameters) .addDataEntryResources( DataEntryResource.fromBytes( StringUtils.join( @@ -77,14 +70,17 @@ "META-INF/services/" + ServiceLoaderRewritingTest.Service.class.getTypeName(), Origin.unknown())) .compile() - .writeToZip(path) .run(parameters.getRuntime(), ServiceLoaderRewritingTest.MainRunner.class) .assertSuccessWithOutput(EXPECTED_OUTPUT + StringUtils.lines("Hello World 2!")) // Check that we have actually rewritten the calls to ServiceLoader.load. - .inspect(inspector -> assertEquals(0, getServiceLoaderLoads(inspector))); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services")); + .inspect( + inspector -> { + assertEquals(0, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf( + inspector, + ServiceLoaderRewritingTest.Service.class, + ServiceLoaderRewritingTest.ServiceImpl.class, + ServiceLoaderRewritingTest.ServiceImpl2.class); + }); } }
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingTest.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingTest.java index 1483561..c9fc05e 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingTest.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderRewritingTest.java
@@ -4,30 +4,22 @@ package com.android.tools.r8.optimize.serviceloader; -import static com.android.tools.r8.TestBase.getTestParameters; -import static junit.framework.Assert.assertNotNull; -import static junit.framework.TestCase.assertEquals; -import static junit.framework.TestCase.assertNull; -import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertEquals; import static org.junit.Assume.assumeTrue; import com.android.tools.r8.CompilationFailedException; -import com.android.tools.r8.DataEntryResource; import com.android.tools.r8.DiagnosticsMatcher; import com.android.tools.r8.NeverInline; import com.android.tools.r8.TestParameters; import com.android.tools.r8.TestParametersCollection; import com.android.tools.r8.ToolHelper.DexVm.Version; import com.android.tools.r8.ir.optimize.ServiceLoaderRewriterDiagnostic; -import com.android.tools.r8.origin.Origin; import com.android.tools.r8.utils.StringUtils; -import com.android.tools.r8.utils.codeinspector.ClassSubject; -import com.android.tools.r8.utils.codeinspector.CodeInspector; import java.io.IOException; -import java.nio.file.Path; +import java.util.List; +import java.util.NoSuchElementException; import java.util.ServiceLoader; import java.util.concurrent.ExecutionException; -import java.util.zip.ZipFile; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -35,7 +27,6 @@ @RunWith(Parameterized.class) public class ServiceLoaderRewritingTest extends ServiceLoaderTestBase { - private final TestParameters parameters; private final String EXPECTED_OUTPUT = StringUtils.lines("Hello World!", "Hello World!", "Hello World!"); @@ -156,202 +147,124 @@ } public ServiceLoaderRewritingTest(TestParameters parameters) { - this.parameters = parameters; + super(parameters); + } + + @Test + public void testRewritingWithNoImpls() + throws IOException, CompilationFailedException, ExecutionException { + serviceLoaderTest(null) + .addKeepMainRule(MainRunner.class) + .compile() + .assertAllInfosMatch( + DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) + .run(parameters.getRuntime(), MainRunner.class) + .assertFailureWithErrorThatThrows(NoSuchElementException.class) + .inspectFailure(inspector -> assertEquals(1, getServiceLoaderLoads(inspector))); } @Test public void testRewritings() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) + serviceLoaderTest(Service.class, ServiceImpl.class) .addKeepMainRule(MainRunner.class) - .setMinApi(parameters) - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .compile() - .writeToZip(path) .run(parameters.getRuntime(), MainRunner.class) .assertSuccessWithOutput(EXPECTED_OUTPUT) - // Check that we have actually rewritten the calls to ServiceLoader.load. - .inspect(inspector -> assertEquals(0, getServiceLoaderLoads(inspector))); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName())); + .inspect( + inspector -> { + List<String> lines = dataResourceConsumer.get("META-INF/services/com.example.Foo"); + assertEquals(0, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class); + }); } @Test public void testRewritingWithMultiple() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) + serviceLoaderTest(Service.class, ServiceImpl.class, ServiceImpl2.class) .addKeepMainRule(MainRunner.class) - .setMinApi(parameters) - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName(), ServiceImpl2.class.getTypeName()) - .getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .compile() - .writeToZip(path) .run(parameters.getRuntime(), MainRunner.class) .assertSuccessWithOutput(EXPECTED_OUTPUT + StringUtils.lines("Hello World 2!")) - // Check that we have actually rewritten the calls to ServiceLoader.load. - .inspect(inspector -> assertEquals(0, getServiceLoaderLoads(inspector))); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName())); + .inspect( + inspector -> { + assertEquals(0, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class, ServiceImpl2.class); + }); } @Test public void testRewritingsWithCatchHandlers() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) + serviceLoaderTest(Service.class, ServiceImpl.class, ServiceImpl2.class) .addKeepMainRule(MainWithTryCatchRunner.class) - .setMinApi(parameters) - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName(), ServiceImpl2.class.getTypeName()) - .getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) .compile() - .writeToZip(path) .run(parameters.getRuntime(), MainWithTryCatchRunner.class) .assertSuccessWithOutput(StringUtils.lines("Hello World!")) - // Check that we have actually rewritten the calls to ServiceLoader.load. - .inspect(inspector -> assertEquals(0, getServiceLoaderLoads(inspector))); - - // Check that we have removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - assertNull(zip.getEntry("META-INF/services/" + Service.class.getTypeName())); + .inspect( + inspector -> { + assertEquals(0, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class, ServiceImpl2.class); + }); } @Test public void testDoNoRewrite() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - CodeInspector inspector = - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) - .addKeepMainRule(OtherRunner.class) - .setMinApi(parameters) - .addKeepRules( - "-whyareyounotinlining class " - + ServiceLoader.class.getTypeName() - + " { *** load(...); }") - .enableExperimentalWhyAreYouNotInlining() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) - .allowDiagnosticInfoMessages() - .compile() - .assertAllInfosMatch( - DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) - .assertAtLeastOneInfoMessage() - .writeToZip(path) - .run(parameters.getRuntime(), OtherRunner.class) - .assertSuccessWithOutput(EXPECTED_OUTPUT) - .inspector(); - - // Check that we have not rewritten the calls to ServiceLoader.load. - assertEquals(3, getServiceLoaderLoads(inspector)); - - // Check that we have not removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - ClassSubject serviceImpl = inspector.clazz(ServiceImpl.class); - assertTrue(serviceImpl.isPresent()); - assertNotNull(zip.getEntry("META-INF/services/" + serviceImpl.getFinalName())); + serviceLoaderTest(Service.class, ServiceImpl.class) + .addKeepMainRule(OtherRunner.class) + .allowDiagnosticInfoMessages() + .compile() + .assertAllInfosMatch( + DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) + .assertAtLeastOneInfoMessage() + .run(parameters.getRuntime(), OtherRunner.class) + .assertSuccessWithOutput(EXPECTED_OUTPUT) + .inspect( + inspector -> { + assertEquals(3, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class); + }); } @Test public void testDoNoRewriteWhenEscaping() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - CodeInspector inspector = - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) - .addKeepMainRule(EscapingRunner.class) - .enableInliningAnnotations() - .setMinApi(parameters) - .addKeepRules( - "-whyareyounotinlining class " - + ServiceLoader.class.getTypeName() - + " { *** load(...); }") - .enableExperimentalWhyAreYouNotInlining() - .addDontObfuscate() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) - .allowDiagnosticInfoMessages() - .compile() - .assertAllInfosMatch( - DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) - .assertAtLeastOneInfoMessage() - .writeToZip(path) - .run(parameters.getRuntime(), EscapingRunner.class) - .assertSuccessWithOutput(EXPECTED_OUTPUT) - .inspector(); - - // Check that we have not rewritten the calls to ServiceLoader.load. - assertEquals(3, getServiceLoaderLoads(inspector)); - - // Check that we have not removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - ClassSubject serviceImpl = inspector.clazz(ServiceImpl.class); - assertTrue(serviceImpl.isPresent()); - assertNotNull(zip.getEntry("META-INF/services/" + serviceImpl.getFinalName())); + serviceLoaderTest(Service.class, ServiceImpl.class) + .addKeepMainRule(EscapingRunner.class) + .enableInliningAnnotations() + .addDontObfuscate() + .allowDiagnosticInfoMessages() + .compile() + .assertAllInfosMatch( + DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) + .assertAtLeastOneInfoMessage() + .run(parameters.getRuntime(), EscapingRunner.class) + .assertSuccessWithOutput(EXPECTED_OUTPUT) + .inspect( + inspector -> { + assertEquals(3, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class); + }); } @Test public void testDoNoRewriteWhenClassLoaderIsPhi() throws IOException, CompilationFailedException, ExecutionException { - Path path = temp.newFile("out.zip").toPath(); - CodeInspector inspector = - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) - .addKeepMainRule(LoadWhereClassLoaderIsPhi.class) - .enableInliningAnnotations() - .setMinApi(parameters) - .addKeepRules( - "-whyareyounotinlining class " - + ServiceLoader.class.getTypeName() - + " { *** load(...); }") - .enableExperimentalWhyAreYouNotInlining() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) - .allowDiagnosticInfoMessages() - .compile() - .assertAllInfosMatch( - DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) - .assertAtLeastOneInfoMessage() - .writeToZip(path) - .run(parameters.getRuntime(), LoadWhereClassLoaderIsPhi.class) - .assertSuccessWithOutputLines("Hello World!") - .inspector(); - - // Check that we have not rewritten the calls to ServiceLoader.load. - assertEquals(1, getServiceLoaderLoads(inspector)); - - // Check that we have not removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - ClassSubject serviceImpl = inspector.clazz(ServiceImpl.class); - assertTrue(serviceImpl.isPresent()); - assertNotNull(zip.getEntry("META-INF/services/" + serviceImpl.getFinalName())); + serviceLoaderTest(Service.class, ServiceImpl.class) + .addKeepMainRule(LoadWhereClassLoaderIsPhi.class) + .enableInliningAnnotations() + .allowDiagnosticInfoMessages() + .compile() + .assertAllInfosMatch( + DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) + .assertAtLeastOneInfoMessage() + .run(parameters.getRuntime(), LoadWhereClassLoaderIsPhi.class) + .assertSuccessWithOutputLines("Hello World!") + .inspect( + inspector -> { + assertEquals(1, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class); + }); } @Test @@ -362,40 +275,20 @@ assumeTrue( parameters.getRuntime().isCf() || !parameters.getRuntime().asDex().getVm().getVersion().equals(Version.V7_0_0)); - Path path = temp.newFile("out.zip").toPath(); - CodeInspector inspector = - testForR8(parameters.getBackend()) - .addInnerClasses(ServiceLoaderRewritingTest.class) - .addKeepMainRule(MainRunner.class) - .addKeepClassRules(Service.class) - .setMinApi(parameters) - .addKeepRules( - "-whyareyounotinlining class " - + ServiceLoader.class.getTypeName() - + " { *** load(...); }") - .enableExperimentalWhyAreYouNotInlining() - .addDataEntryResources( - DataEntryResource.fromBytes( - StringUtils.lines(ServiceImpl.class.getTypeName()).getBytes(), - "META-INF/services/" + Service.class.getTypeName(), - Origin.unknown())) - .allowDiagnosticInfoMessages() - .compile() - .assertAllInfosMatch( - DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) - .assertAtLeastOneInfoMessage() - .writeToZip(path) - .run(parameters.getRuntime(), MainRunner.class) - .assertSuccessWithOutput(EXPECTED_OUTPUT) - .inspector(); - - // Check that we have not rewritten the calls to ServiceLoader.load. - assertEquals(3, getServiceLoaderLoads(inspector)); - - // Check that we have not removed the service configuration from META-INF/services. - ZipFile zip = new ZipFile(path.toFile()); - ClassSubject service = inspector.clazz(Service.class); - assertTrue(service.isPresent()); - assertNotNull(zip.getEntry("META-INF/services/" + service.getFinalName())); + serviceLoaderTest(Service.class, ServiceImpl.class) + .addKeepMainRule(MainRunner.class) + .addKeepClassRules(Service.class) + .allowDiagnosticInfoMessages() + .compile() + .assertAllInfosMatch( + DiagnosticsMatcher.diagnosticType(ServiceLoaderRewriterDiagnostic.class)) + .assertAtLeastOneInfoMessage() + .run(parameters.getRuntime(), MainRunner.class) + .assertSuccessWithOutput(EXPECTED_OUTPUT) + .inspect( + inspector -> { + assertEquals(3, getServiceLoaderLoads(inspector)); + verifyServiceMetaInf(inspector, Service.class, ServiceImpl.class); + }); } }
diff --git a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java index 21276a2..cf1fc6f 100644 --- a/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java +++ b/src/test/java/com/android/tools/r8/optimize/serviceloader/ServiceLoaderTestBase.java
@@ -4,38 +4,43 @@ package com.android.tools.r8.optimize.serviceloader; import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethodWithName; -import static junit.framework.TestCase.assertTrue; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import com.android.tools.r8.DataEntryResource; +import com.android.tools.r8.R8FullTestBuilder; import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.graph.AppServices; +import com.android.tools.r8.origin.Origin; +import com.android.tools.r8.utils.DataResourceConsumerForTesting; import com.android.tools.r8.utils.codeinspector.ClassSubject; import com.android.tools.r8.utils.codeinspector.CodeInspector; import com.android.tools.r8.utils.codeinspector.InstructionSubject; -import com.android.tools.r8.utils.codeinspector.MethodSubject; +import com.google.common.collect.ImmutableMap; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.ServiceLoader; +import java.util.stream.Collectors; public class ServiceLoaderTestBase extends TestBase { + protected final TestParameters parameters; + protected DataResourceConsumerForTesting dataResourceConsumer; + + public ServiceLoaderTestBase(TestParameters parameters) { + this.parameters = parameters; + } public static long getServiceLoaderLoads(CodeInspector inspector) { - return inspector.allClasses().stream() - .mapToLong(ServiceLoaderTestBase::getServiceLoaderLoads) - .reduce(0, Long::sum); - } - - public static long getServiceLoaderLoads(CodeInspector inspector, Class<?> clazz) { - return getServiceLoaderLoads(inspector.clazz(clazz)); - } - - public static long getServiceLoaderLoads(ClassSubject classSubject) { - assertTrue(classSubject.isPresent()); - return classSubject.allMethods(MethodSubject::hasCode).stream() - .mapToLong( - method -> - method - .streamInstructions() - .filter(ServiceLoaderTestBase::isServiceLoaderLoad) - .count()) - .sum(); + return inspector + .streamInstructions() + .filter(ServiceLoaderTestBase::isServiceLoaderLoad) + .count(); } private static boolean isServiceLoaderLoad(InstructionSubject instruction) { @@ -43,22 +48,93 @@ && instruction.getMethod().qualifiedName().contains("ServiceLoader.load"); } - public void verifyNoClassLoaders(CodeInspector inspector) { - inspector.allClasses().forEach(this::verifyNoClassLoaders); + public static void verifyNoClassLoaders(CodeInspector inspector) { + inspector.allClasses().forEach(ServiceLoaderTestBase::verifyNoClassLoaders); } - public void verifyNoClassLoaders(ClassSubject classSubject) { + public static void verifyNoClassLoaders(ClassSubject classSubject) { assertTrue(classSubject.isPresent()); classSubject.forAllMethods( method -> assertThat(method, not(invokesMethodWithName("getClassLoader")))); } - public void verifyNoServiceLoaderLoads(CodeInspector inspector) { - inspector.allClasses().forEach(this::verifyNoServiceLoaderLoads); + public static void verifyNoServiceLoaderLoads(CodeInspector inspector) { + inspector.allClasses().forEach(ServiceLoaderTestBase::verifyNoServiceLoaderLoads); } - public void verifyNoServiceLoaderLoads(ClassSubject classSubject) { + public static void verifyNoServiceLoaderLoads(ClassSubject classSubject) { assertTrue(classSubject.isPresent()); classSubject.forAllMethods(method -> assertThat(method, not(invokesMethodWithName("load")))); } + + public Map<String, List<String>> getServiceMappings() { + return dataResourceConsumer.getAll().entrySet().stream() + .filter(e -> e.getKey().startsWith(AppServices.SERVICE_DIRECTORY_NAME)) + .collect( + Collectors.toMap( + e -> e.getKey().substring(AppServices.SERVICE_DIRECTORY_NAME.length()), + e -> e.getValue())); + } + + public void verifyServiceMetaInf( + CodeInspector inspector, Class<?> serviceClass, Class<?> serviceImplClass) { + // Account for renaming, and for the impl to be merged with the interface. + String finalServiceName = inspector.clazz(serviceClass).getFinalName(); + String finalImplName = inspector.clazz(serviceImplClass).getFinalName(); + if (finalServiceName == null) { + finalServiceName = finalImplName; + } + Map<String, List<String>> actual = getServiceMappings(); + Map<String, List<String>> expected = + ImmutableMap.of(finalServiceName, Collections.singletonList(finalImplName)); + assertEquals(expected, actual); + } + + public void verifyServiceMetaInf( + CodeInspector inspector, + Class<?> serviceClass, + Class<?> serviceImplClass1, + Class<?> serviceImplClass2) { + // Account for renaming. No class merging should happen. + String finalServiceName = inspector.clazz(serviceClass).getFinalName(); + String finalImplName1 = inspector.clazz(serviceImplClass1).getFinalName(); + String finalImplName2 = inspector.clazz(serviceImplClass2).getFinalName(); + Map<String, List<String>> actual = getServiceMappings(); + Map<String, List<String>> expected = + ImmutableMap.of(finalServiceName, Arrays.asList(finalImplName1, finalImplName2)); + assertEquals(expected, actual); + } + + protected R8FullTestBuilder serviceLoaderTest(Class<?> serviceClass, Class<?>... implClasses) + throws IOException { + return serviceLoaderTestNoClasses(serviceClass, implClasses).addInnerClasses(getClass()); + } + + protected R8FullTestBuilder serviceLoaderTestNoClasses( + Class<?> serviceClass, Class<?>... implClasses) throws IOException { + R8FullTestBuilder ret = + testForR8(parameters.getBackend()) + .setMinApi(parameters) + .addOptionsModification( + o -> { + dataResourceConsumer = new DataResourceConsumerForTesting(o.dataResourceConsumer); + o.dataResourceConsumer = dataResourceConsumer; + }) + // Enables ServiceLoader optimization failure diagnostics. + .enableExperimentalWhyAreYouNotInlining() + .addKeepRules( + "-whyareyounotinlining class java.util.ServiceLoader { *** load(...); }"); + if (implClasses.length > 0) { + String implLines = + Arrays.stream(implClasses) + .map(c -> c.getTypeName() + "\n") + .collect(Collectors.joining("")); + ret.addDataEntryResources( + DataEntryResource.fromBytes( + implLines.getBytes(), + AppServices.SERVICE_DIRECTORY_NAME + serviceClass.getTypeName(), + Origin.unknown())); + } + return ret; + } }
diff --git a/src/test/java/com/android/tools/r8/utils/DataResourceConsumerForTesting.java b/src/test/java/com/android/tools/r8/utils/DataResourceConsumerForTesting.java index 77274e6..dc2dfe8 100644 --- a/src/test/java/com/android/tools/r8/utils/DataResourceConsumerForTesting.java +++ b/src/test/java/com/android/tools/r8/utils/DataResourceConsumerForTesting.java
@@ -54,6 +54,10 @@ @Override public void finished(DiagnosticsHandler handler) {} + public Map<String, ImmutableList<String>> getAll() { + return resources; + } + public ImmutableList<String> get(String name) { return resources.get(name); }
diff --git a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java index 74bdc47..7c06d42 100644 --- a/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java +++ b/src/test/testbase/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java
@@ -67,6 +67,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; +import java.util.stream.Stream; public class CodeInspector { @@ -401,6 +402,12 @@ return builder.build(); } + public Stream<InstructionSubject> streamInstructions() { + return allClasses().stream() + .flatMap(cls -> cls.allMethods(MethodSubject::hasCode).stream()) + .flatMap(MethodSubject::streamInstructions); + } + public FieldSubject field(Field field) { return field(Reference.fieldFromField(field)); }