Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Differentiate overridden vs inherited super test methods #71

Merged
merged 3 commits into from
Aug 30, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions parser/ValidTestList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion parser/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
23 changes: 21 additions & 2 deletions parser/src/main/kotlin/com/linkedin/dex/parser/JUnit4Extensions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,39 @@ 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.equals(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()

// alter the existing test methods to include super annotations if they're inherited
parsingResult.testMethods.filter { method -> superTestMethods.any {
it.testNameWithoutClass.equals(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 }

method.annotations += inheritedAnnotations

}

return adaptedSuperMethods union parsingResult.testMethods
}

Expand Down
4 changes: 2 additions & 2 deletions parser/src/main/kotlin/com/linkedin/dex/parser/TestMethod.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import com.linkedin.dex.spec.ClassDefItem
import com.linkedin.dex.spec.DexFile
import com.linkedin.dex.spec.MethodIdItem

data class TestMethod(val testName: String, val annotations: List<TestAnnotation>) : Comparable<TestMethod> {
data class TestMethod(val testName: String, val annotations: MutableList<TestAnnotation>) : Comparable<TestMethod> {
override fun compareTo(other: TestMethod): Int = testName.compareTo(other.testName)
}

Expand Down Expand Up @@ -52,7 +52,7 @@ private fun DexFile.createTestMethod(methodId: MethodIdItem,
classAnnotations: List<TestAnnotation>): 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)
Expand Down
31 changes: 26 additions & 5 deletions parser/src/test/kotlin/com/linkedin/dex/DexParserShould.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -49,14 +49,15 @@ class DexParserShould {

val method = testMethods[0]
assertEquals("com.linkedin.parser.test.junit4.java.BasicJUnit4#concreteTest", method.testName)
assertEquals(method.annotations[2].values["stringValue"], DecodedValue.DecodedString("Hello world!"))
assertEquals(method.annotations[0].values["stringValue"], DecodedValue.DecodedString("Hello world!"))
}

@Test
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") })
}
Expand Down Expand Up @@ -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") }

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ private void privateTestShouldNotBeReported() {
assertTrue(true);
}

@Override
@Test
public void concreteTest() {
super.concreteTest();
}

@NonInheritedAnnotation
public void customAnnotationTest() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,11 @@ public class ConcreteTest extends AbstractTest {
public void concreteTest() {
assertTrue(true);
}

@NonInheritedAnnotation
@InheritedAnnotation
@Test
public void nonOverriddenConcreteTest() {
assertTrue(true);
}
}