Debug tests for vertical class merger
When a class pkg.A is merged with a class pkg.B, and both classes have a member named m, then pkg.A.m will be renamed to pkg.B.m$pkg$A.
This CL extends the existing tests for vertical class merging by running the tests with a debugger and checking that we never encounter a method with the name pkg.B.m$pkg$A. In particular, the tests check that there are no methods containing the string "$classmerging$" (this suffices since all tests are in the package named "classmerging").
Change-Id: Ie200afce849080f1808336c79284738b4dbfd893
diff --git a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
index d9d62c3..6322840 100644
--- a/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/ClassMergingTest.java
@@ -3,9 +3,11 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.classmerging;
+import static com.android.tools.r8.ir.desugar.InterfaceMethodRewriter.COMPANION_CLASS_NAME_SUFFIX;
import static com.android.tools.r8.smali.SmaliBuilder.buildCode;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
@@ -22,6 +24,10 @@
import com.android.tools.r8.VmTestRunner.IgnoreForRangeOfVmVersions;
import com.android.tools.r8.code.Instruction;
import com.android.tools.r8.code.MoveException;
+import com.android.tools.r8.debug.DebugTestBase;
+import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.Command;
+import com.android.tools.r8.debug.DebugTestBase.JUnit3Wrapper.DebuggeeState;
+import com.android.tools.r8.debug.DexDebugTestConfig;
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.ir.optimize.Inliner.Reason;
import com.android.tools.r8.jasmin.JasminBuilder;
@@ -166,7 +172,9 @@
configure(options);
// Avoid that direct methods in B get inlined.
options.testing.validInliningReasons = ImmutableSet.of(Reason.FORCE);
- });
+ },
+ // Disable debug testing since the test has a method with "$classmerging$" in the name.
+ null);
ClassSubject clazzSubject = inspector.clazz("classmerging.ConflictInGeneratedNameTest$B");
assertThat(clazzSubject, isPresent());
@@ -1008,6 +1016,23 @@
String proguardConfig,
Consumer<InternalOptions> optionsConsumer)
throws Throwable {
+ return runTestOnInput(
+ main,
+ input,
+ preservedClassNames,
+ proguardConfig,
+ optionsConsumer,
+ new VerticalClassMergerDebugTest(main));
+ }
+
+ private CodeInspector runTestOnInput(
+ String main,
+ AndroidApp input,
+ Predicate<String> preservedClassNames,
+ String proguardConfig,
+ Consumer<InternalOptions> optionsConsumer,
+ VerticalClassMergerDebugTest debugTestRunner)
+ throws Throwable {
Path proguardMapPath = File.createTempFile("mapping", ".txt", temp.getRoot()).toPath();
R8Command.Builder commandBuilder =
ToolHelper.prepareR8CommandBuilder(input)
@@ -1034,6 +1059,11 @@
}
// Check that the R8-generated code produces the same result as D8-generated code.
assertEquals(runOnArt(compileWithD8(input), main), runOnArt(output, main));
+ // Check that we never come across a method that has a name with "$classmerging$" in it during
+ // debugging.
+ if (debugTestRunner != null) {
+ debugTestRunner.test(output, proguardMapPath);
+ }
return outputInspector;
}
@@ -1049,4 +1079,82 @@
}
return builder.toString();
}
+
+ private class VerticalClassMergerDebugTest extends DebugTestBase {
+
+ private final String main;
+ private DebugTestRunner runner = null;
+
+ public VerticalClassMergerDebugTest(String main) {
+ this.main = main;
+ }
+
+ public void test(AndroidApp app, Path proguardMapPath) throws Throwable {
+ Path appPath =
+ File.createTempFile("app", ".zip", ClassMergingTest.this.temp.getRoot()).toPath();
+ app.writeToZip(appPath, OutputMode.DexIndexed);
+
+ DexDebugTestConfig config = new DexDebugTestConfig(appPath);
+ config.allowUnprocessedCommands();
+ config.setProguardMap(proguardMapPath);
+
+ this.runner =
+ getDebugTestRunner(
+ config, main, breakpoint(main, "main"), run(), stepIntoUntilNoLongerInApp());
+ this.runner.runBare();
+ }
+
+ private void checkState(DebuggeeState state) {
+ // If a class pkg.A is merged into pkg.B, and a method pkg.A.m() needs to be renamed, then
+ // it will be renamed to pkg.B.m$pkg$A(). Since all tests are in the package "classmerging",
+ // we check that no methods in the debugging state (i.e., after the Proguard map has been
+ // applied) contain "$classmerging$.
+ String qualifiedMethodSignature =
+ state.getClassSignature() + "->" + state.getMethodName() + state.getMethodSignature();
+ boolean holderIsCompanionClass = state.getClassName().endsWith(COMPANION_CLASS_NAME_SUFFIX);
+ if (!holderIsCompanionClass) {
+ assertThat(qualifiedMethodSignature, not(containsString("$classmerging$")));
+ }
+ }
+
+ // Keeps stepping in until it is no longer in a class from the classmerging package.
+ // Then starts stepping out until it is again in the classmerging package.
+ private Command stepIntoUntilNoLongerInApp() {
+ return stepUntil(
+ StepKind.INTO,
+ StepLevel.INSTRUCTION,
+ state -> {
+ if (state.getClassSignature().contains("classmerging")) {
+ checkState(state);
+
+ // Continue stepping into.
+ return false;
+ }
+
+ // Stop stepping into.
+ runner.enqueueCommandFirst(stepOutUntilInApp());
+ return true;
+ });
+ }
+
+ // Keeps stepping out until it is in a class from the classmerging package.
+ // Then starts stepping in until it is no longer in the classmerging package.
+ private Command stepOutUntilInApp() {
+ return stepUntil(
+ StepKind.OUT,
+ StepLevel.INSTRUCTION,
+ state -> {
+ if (state.getClassSignature().contains("classmerging")) {
+ checkState(state);
+
+ // Stop stepping out.
+ runner.enqueueCommandFirst(stepIntoUntilNoLongerInApp());
+ return true;
+ }
+
+ // Continue stepping out.
+ return false;
+ });
+ }
+ }
}
diff --git a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
index 97f6820..df2c3e0 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -137,6 +137,18 @@
private void runInternal(
DebugTestConfig config, String debuggeeClass, List<JUnit3Wrapper.Command> commands)
throws Throwable {
+ getDebugTestRunner(config, debuggeeClass, commands).runBare();
+ }
+
+ protected DebugTestRunner getDebugTestRunner(
+ DebugTestConfig config, String debuggeeClass, JUnit3Wrapper.Command... commands)
+ throws Throwable {
+ return getDebugTestRunner(config, debuggeeClass, Arrays.asList(commands));
+ }
+
+ protected DebugTestRunner getDebugTestRunner(
+ DebugTestConfig config, String debuggeeClass, List<JUnit3Wrapper.Command> commands)
+ throws Throwable {
// Skip test due to unsupported runtime.
Assume.assumeTrue("Skipping test " + testName.getMethodName() + " because ART is not supported",
ToolHelper.artSupported());
@@ -152,7 +164,7 @@
? null
: ClassNameMapper.mapperFromFile(config.getProguardMap());
- new JUnit3Wrapper(config, debuggeeClass, commands, classNameMapper).runBare();
+ return new JUnit3Wrapper(config, debuggeeClass, commands, classNameMapper);
}
/**
@@ -495,8 +507,14 @@
return t -> inspector.accept(t.debuggeeState.getLocalValues().get(localName));
}
+ protected interface DebugTestRunner {
+ void enqueueCommandFirst(JUnit3Wrapper.Command command);
+
+ void runBare() throws Throwable;
+ }
+
@Ignore("Prevents Gradle from running the wrapper as a test.")
- static class JUnit3Wrapper extends JDWPTestCase {
+ public static class JUnit3Wrapper extends JDWPTestCase implements DebugTestRunner {
private final String debuggeeClassName;
@@ -801,8 +819,10 @@
// Continue stepping until mainLoopStep exits with false.
}
- assertTrue(
- "All commands have NOT been processed for config: " + config, commandsQueue.isEmpty());
+ if (config.mustProcessAllCommands()) {
+ assertTrue(
+ "All commands have NOT been processed for config: " + config, commandsQueue.isEmpty());
+ }
logWriter.println("Finish loop");
}
diff --git a/src/test/java/com/android/tools/r8/debug/DebugTestConfig.java b/src/test/java/com/android/tools/r8/debug/DebugTestConfig.java
index 556e17f..519b580 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestConfig.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestConfig.java
@@ -17,6 +17,7 @@
DEX
}
+ private boolean mustProcessAllCommands = true;
private List<Path> paths = new ArrayList<>();
private Path proguardMap = null;
@@ -47,6 +48,14 @@
return this;
}
+ public boolean mustProcessAllCommands() {
+ return mustProcessAllCommands;
+ }
+
+ public void allowUnprocessedCommands() {
+ mustProcessAllCommands = false;
+ }
+
/** Proguard map that the debuggee has been translated according to, null if not present. */
public Path getProguardMap() {
return proguardMap;
