diff --git a/parser/ValidTestList.txt b/parser/ValidTestList.txt index 9a4e790..1df95f9 100644 --- a/parser/ValidTestList.txt +++ b/parser/ValidTestList.txt @@ -11,8 +11,10 @@ com.linkedin.parser.test.junit4.java.BasicJUnit4#basicJUnit4 com.linkedin.parser.test.junit4.java.BasicJUnit4#basicJUnit4Second com.linkedin.parser.test.junit4.java.BasicJUnit4#concreteTest com.linkedin.parser.test.junit4.java.BasicJUnit4#customAnnotationTest +com.linkedin.parser.test.junit4.java.BasicJUnit4#nonOverriddenConcreteTest com.linkedin.parser.test.junit4.java.ConcreteTest#abstractTest com.linkedin.parser.test.junit4.java.ConcreteTest#concreteTest +com.linkedin.parser.test.junit4.java.ConcreteTest#nonOverriddenConcreteTest com.linkedin.parser.test.junit4.java.JUnit4ClassInsideInterface$InnerClass#innerClassTest com.linkedin.parser.test.junit4.java.JUnit4TestInsideStaticInnerClass$InnerClass#innerClassTest com.linkedin.parser.test.junit4.kotlin.DefaultInterfaceImplementation#testMethodShouldNotBeReported diff --git a/parser/build.gradle b/parser/build.gradle index 7e1e874..07e1dea 100644 --- a/parser/build.gradle +++ b/parser/build.gradle @@ -42,7 +42,7 @@ task testParsing(dependsOn: ':test-app:assembleDebugAndroidTest', type: JavaExec def parsedTests = file("$buildDir/AllTests.txt").readLines() if (validTests != parsedTests) { - throw new GradleException("Parsed tests do not match expected tests") + throw new GradleException("Parsed tests do not match expected tests: " + parsedTests) } } } diff --git a/parser/src/main/kotlin/com/linkedin/dex/parser/JUnit4Extensions.kt b/parser/src/main/kotlin/com/linkedin/dex/parser/JUnit4Extensions.kt index 4ae1261..49618fb 100644 --- a/parser/src/main/kotlin/com/linkedin/dex/parser/JUnit4Extensions.kt +++ b/parser/src/main/kotlin/com/linkedin/dex/parser/JUnit4Extensions.kt @@ -119,21 +119,45 @@ private fun createAllTestMethods( val childClassAnnotations = dexFile.getClassAnnotationValues(directory) val childClassAnnotationNames = childClassAnnotations.map { it.name } + // We need to differentiate cases where a method is overridden from the superclass + // vs when they are not, as it will impact whether we should include all annotations + // from the superclass method or just ones with the inherited property + // So, we exclude any that have overridden versions val adaptedSuperMethods = superTestMethods + .filterNot { superMethod -> parsingResult.testMethods.any { + it.testNameWithoutClass == superMethod.testNameWithoutClass + } } .map { method -> val onlyParentAnnotations = method .annotations .filterNot { childClassAnnotationNames.contains(it.name) } - .filter { it.inherited } TestMethod( testName = className + method.testNameWithoutClass, - annotations = onlyParentAnnotations + childClassAnnotations + annotations = (onlyParentAnnotations + childClassAnnotations).toMutableList() ) } .toSet() - return adaptedSuperMethods union parsingResult.testMethods + // alter the existing test methods to include super annotations if they're inherited + val alteredMethods = parsingResult.testMethods.filter { method -> superTestMethods.any { + it.testNameWithoutClass == method.testNameWithoutClass + } } + .map { method -> + val superMethod = superTestMethods.find { it.testNameWithoutClass == method.testNameWithoutClass } + val onlyParentAnnotations = superMethod?.annotations ?: emptySet() + val inheritedAnnotations = onlyParentAnnotations + .filterNot { childClassAnnotationNames.contains(it.name) } + .filter { it.inherited } + + TestMethod(method.testName, inheritedAnnotations + method.annotations) + } + + val originalTestMethods = parsingResult.testMethods.filterNot { method -> superTestMethods.any { + it.testNameWithoutClass == method.testNameWithoutClass + } } + + return adaptedSuperMethods union alteredMethods union originalTestMethods } private val TestMethod.testNameWithoutClass diff --git a/parser/src/main/kotlin/com/linkedin/dex/parser/TestMethod.kt b/parser/src/main/kotlin/com/linkedin/dex/parser/TestMethod.kt index 6134543..72f7a67 100644 --- a/parser/src/main/kotlin/com/linkedin/dex/parser/TestMethod.kt +++ b/parser/src/main/kotlin/com/linkedin/dex/parser/TestMethod.kt @@ -52,7 +52,7 @@ private fun DexFile.createTestMethod(methodId: MethodIdItem, classAnnotations: List): TestMethod { val methodAnnotationDescriptors = getMethodAnnotationValues(methodId, directory) - val annotations = classAnnotations.plus(methodAnnotationDescriptors) + val annotations = classAnnotations.plus(methodAnnotationDescriptors).toMutableList() val className = formatClassName(classDef) val methodName = ParseUtils.parseMethodName(byteBuffer, stringIds, methodId) diff --git a/parser/src/test/kotlin/com/linkedin/dex/DexParserShould.kt b/parser/src/test/kotlin/com/linkedin/dex/DexParserShould.kt index f4bc67f..3842bd8 100644 --- a/parser/src/test/kotlin/com/linkedin/dex/DexParserShould.kt +++ b/parser/src/test/kotlin/com/linkedin/dex/DexParserShould.kt @@ -19,14 +19,14 @@ class DexParserShould { fun parseCorrectNumberOfTestMethods() { val testMethods = DexParser.findTestNames(APK_PATH, listOf("")) - assertEquals(21, testMethods.size) + assertEquals(23, testMethods.size) } @Test fun parseMethodWithMultipleMethodAnnotations() { val testMethods = DexParser.findTestMethods(APK_PATH, listOf("")).filter { it.annotations.filter { it.name.contains("TestValueAnnotation") }.isNotEmpty() } - assertEquals(4, testMethods.size) + assertEquals(5, testMethods.size) val method = testMethods[1] assertEquals(method.testName, "com.linkedin.parser.test.junit4.java.BasicJUnit4#basicJUnit4") @@ -40,7 +40,7 @@ class DexParserShould { val method = testMethods[0] assertEquals("com.linkedin.parser.test.junit4.java.BasicJUnit4#abstractTest", method.testName) - assertEquals(method.annotations[1].values["stringValue"], DecodedValue.DecodedString("Hello world!")) + assertEquals(method.annotations[2].values["stringValue"], DecodedValue.DecodedString("Hello world!")) } @Test @@ -56,7 +56,8 @@ class DexParserShould { fun parsNonInheritedMethodAnnotation() { val testMethods = DexParser.findTestMethods(APK_PATH, listOf("")).filter { it.annotations.filter { it.name.contains("InheritedAnnotation") }.isNotEmpty() } - val method = testMethods[0] + val method = testMethods.find { it.testName.contains("BasicJUnit4#concreteTest") } + requireNotNull(method) assertEquals("com.linkedin.parser.test.junit4.java.BasicJUnit4#concreteTest", method.testName) assertFalse(method.annotations.any { it.name.contains("NonInheritedAnnotation") }) } @@ -221,6 +222,15 @@ class DexParserShould { assertMatches(methodAnnotation.values["typeValue"], "Lorg/junit/Test;") } + @Test + fun parseAbstractClassTestMethodCorrectly() { + val method = getTestMethodFromAbstractClass() + val annotations = method.annotations + + assertEquals(annotations.size, 2) + assertTrue(annotations.any { it.name == "org.junit.Test" }) + } + private fun getBasicJunit4TestMethod(): TestMethod { val testMethods = DexParser.findTestMethods(APK_PATH, listOf("")).filter { it.annotations.filter { it.name.contains("TestValueAnnotation") }.isNotEmpty() }.filter { it.testName.equals("com.linkedin.parser.test.junit4.java.BasicJUnit4#basicJUnit4") } @@ -243,6 +253,17 @@ class DexParserShould { return method } + private fun getTestMethodFromAbstractClass(): TestMethod { + val testMethods = DexParser.findTestMethods(APK_PATH, listOf("")).filter { it.testName.equals("com.linkedin.parser.test.junit4.java.ConcreteTest#abstractTest") } + + assertEquals(1, testMethods.size) + + val method = testMethods.first() + assertEquals(method.testName, "com.linkedin.parser.test.junit4.java.ConcreteTest#abstractTest") + + return method + } + // region value type matchers private fun assertMatches(value: DecodedValue?, string: String) { if (value is DecodedValue.DecodedString) { diff --git a/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/BasicJUnit4.java b/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/BasicJUnit4.java index 266c15c..f35d589 100644 --- a/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/BasicJUnit4.java +++ b/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/BasicJUnit4.java @@ -31,6 +31,12 @@ private void privateTestShouldNotBeReported() { assertTrue(true); } + @Override + @Test + public void concreteTest() { + super.concreteTest(); + } + @NonInheritedAnnotation public void customAnnotationTest() { diff --git a/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/ConcreteTest.java b/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/ConcreteTest.java index e788aae..e0a0f15 100644 --- a/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/ConcreteTest.java +++ b/test-app/src/androidTest/java/com/linkedin/parser/test/junit4/java/ConcreteTest.java @@ -13,4 +13,11 @@ public class ConcreteTest extends AbstractTest { public void concreteTest() { assertTrue(true); } + + @NonInheritedAnnotation + @InheritedAnnotation + @Test + public void nonOverriddenConcreteTest() { + assertTrue(true); + } }