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

Detekt the api coverage in Integration tests #2300

Merged
merged 1 commit into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 16 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,22 @@ test-pyramid:legacy-integration-instrumented-median-api:
- !reference [.snippets, install-android-sdk]
- !reference [.snippets, run-legacy-integration-instrumented]

test-pyramid:detekt-api-coverage:
tags: [ "arch:amd64" ]
image: $CI_IMAGE_DOCKER
stage: test-pyramid
timeout: 1h
script:
- mkdir -p ./config/
- aws ssm get-parameter --region us-east-1 --name ci.dd-sdk-android.gradle-properties --with-decryption --query "Parameter.Value" --out text >> ./gradle.properties
- GRADLE_OPTS="-Xmx4096M" ./gradlew assembleLibrariesDebug --stacktrace --no-daemon
- GRADLE_OPTS="-Xmx4096M" ./gradlew printSdkDebugRuntimeClasspath --stacktrace --no-daemon
- GRADLE_OPTS="-Xmx4096M" ./gradlew :tools:detekt:jar --stacktrace --no-daemon
- curl -sSLO https://github.com/detekt/detekt/releases/download/v1.23.4/detekt-cli-1.23.4-all.jar
- java -jar detekt-cli-1.23.4-all.jar --config detekt_test_pyramid.yml --plugins tools/detekt/build/libs/detekt.jar -ex "**/*.kts" --jvm-target 11 -cp $(cat sdk_classpath)
# For now we just print the uncovered apis, eventually we will fail if it's not empty
- grep -v -f apiUsage.log apiSurface.log

test-pyramid:publish-e2e-synthetics:
tags: [ "arch:amd64" ]
image: $CI_IMAGE_DOCKER
Expand Down Expand Up @@ -424,7 +440,6 @@ test-pyramid:publish-webview-synthetics:
paths:
- sample/kotlin/build/outputs/apk/us1/release/kotlin-us1-release.apk


test-pyramid:publish-staging-synthetics:
tags: [ "arch:amd64" ]
image: $CI_IMAGE_DOCKER
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ fun Project.registerSubModuleAggregationTask(
) {
tasks.register(taskName) {
project.subprojects.forEach { subProject ->
println("SubProject ${subProject.name} / ${subProject.path}")
if (!exceptions.contains(subProject.name) &&
subProject.name.startsWith(subModuleNamePrefix) &&
subProject.path.startsWith(subModulePathPrefix)
Expand Down
26 changes: 22 additions & 4 deletions detekt_test_pyramid.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,27 @@ datadog-test-pyramid:
active: true
ApiUsage:
active: true
includes: ['**/reliability/**']
includes: [ '**/reliability/**' ]
internalPackagePrefix: 'com.datadog'
ApiSurface:
active: true
includes: ['**/dd-sdk-android-*/**']
excludes: ['**/build/**', '**/test/**', '**/testDebug/**','**/testRelease/**', '**/androidTest/**', '**/testFixtures/**', '**/buildSrc/**', '**/*.kts', '**/instrumented/**', '**/sample/**', '**/tools/**']

includes: [ '**/dd-sdk-android-*/**' ]
excludes:
- '**/build/**'
- '**/test/**'
- '**/testDebug/**'
- '**/testRelease/**'
- '**/androidTest/**'
- '**/testFixtures/**'
- '**/buildSrc/**'
- '**/*.kts'
- '**/instrumented/**'
- '**/sample/**'
- '**/tools/**'
- '**/dd-sdk-android-internal/**'
internalPackagePrefix: 'com.datadog'
ignoredAnnotations:
- "com.datadog.android.lint.InternalApi"
ignoredClasses:
- "com.datadog.android._InternalProxy"
- "com.datadog.android.rum._RumInternalProxy"
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.config
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import io.gitlab.arturbosch.detekt.rules.isOverride
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtClass
import org.jetbrains.kotlin.psi.KtFunctionType
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtNullableType
import org.jetbrains.kotlin.psi.KtObjectLiteralExpression
import org.jetbrains.kotlin.psi.KtObjectDeclaration
import org.jetbrains.kotlin.psi.KtParameter
import org.jetbrains.kotlin.psi.KtParameterList
import org.jetbrains.kotlin.psi.KtPrimaryConstructor
import org.jetbrains.kotlin.psi.KtTypeElement
import org.jetbrains.kotlin.psi.KtUserType
import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
import org.jetbrains.kotlin.psi.psiUtil.isExtensionDeclaration
import java.io.File

Expand All @@ -40,6 +40,9 @@ class ApiSurface(

private val outputFileName: String by config(defaultValue = "apiSurface.log")
private val outputFile: File by lazy { File(outputFileName) }
private val internalPackagePrefix: String by config(defaultValue = "")
private val ignoredAnnotations: List<String> by config(defaultValue = emptyList())
private val ignoredClasses: List<String> by config(defaultValue = emptyList())

// region Rule

Expand All @@ -50,20 +53,45 @@ class ApiSurface(
Debt.FIVE_MINS
)

override fun visitClass(klass: KtClass) {
if (klass.hasModifier(KtTokens.PRIVATE_KEYWORD) || klass.hasModifier(KtTokens.INTERNAL_KEYWORD)) {
override fun visitObjectDeclaration(declaration: KtObjectDeclaration) {
val hasIgnoredKeyword = ignoredKeywords.any { declaration.hasModifier(it) }
val isIgnoredClass = declaration.fqName?.asString() in ignoredClasses
val hasIgnoredAnnotation = declaration.annotationEntries.any {
it.shortName?.asString()?.resolveFullType() in ignoredAnnotations
}

@Suppress("ComplexCondition")
if (isIgnoredClass || hasIgnoredKeyword || hasIgnoredAnnotation) {
return
}
if (klass.isInterface()) {

super.visitObjectDeclaration(declaration)
}

override fun visitClass(klass: KtClass) {
val hasIgnoredKeyword = ignoredKeywords.any { klass.hasModifier(it) }
val isInterface = klass.isInterface()
val isEnum = klass.isEnum()
val isIgnoredClass = klass.fqName?.asString() in ignoredClasses
val hasIgnoredAnnotation = klass.annotationEntries.any {
it.shortName?.asString()?.resolveFullType() in ignoredAnnotations
}

@Suppress("ComplexCondition")
if (isIgnoredClass || hasIgnoredKeyword || isInterface || isEnum || hasIgnoredAnnotation) {
return
}

super.visitClass(klass)
}

override fun visitPrimaryConstructor(constructor: KtPrimaryConstructor) {
if (constructor.hasModifier(KtTokens.PRIVATE_KEYWORD) || constructor.hasModifier(KtTokens.INTERNAL_KEYWORD)) {
val hasIgnoredKeyword = ignoredKeywords.any { constructor.hasModifier(it) }

if (hasIgnoredKeyword) {
return
}

val parentName = constructor.containingClassOrObject?.fqName
?: constructor.containingKtFile.packageFqName
outputFile.appendText("$parentName.constructor(")
Expand All @@ -78,37 +106,47 @@ class ApiSurface(
outputFile.appendText(")\n")
}

@Suppress("ReturnCount")
override fun visitNamedFunction(function: KtNamedFunction) {
if (function.hasModifier(KtTokens.PRIVATE_KEYWORD) || function.hasModifier(KtTokens.INTERNAL_KEYWORD)) {
return
}
val parameterList = function.getChildrenOfType<KtParameterList>().firstOrNull()
if (function.name == "toString" && parameterList?.children.isNullOrEmpty()) {
return
val hasIgnoredKeyword = ignoredKeywords.any { function.hasModifier(it) }
val hasIgnoredName = function.name in ignoredFunctionNames
val hasIgnoredAnnotation = function.annotationEntries.any {
it.shortName?.asString()?.resolveFullType() in ignoredAnnotations
}
if (function.getStrictParentOfType<KtObjectLiteralExpression>() != null) {
// Function is overriding something in an anonymous object
// e.g.: val x = object : Interface { override fun foo() {} }
val isOverride = function.isOverride()

@Suppress("ComplexCondition")
if (hasIgnoredKeyword || hasIgnoredName || hasIgnoredAnnotation || isOverride) {
return
}
if (function.isExtensionDeclaration()) {
val target = function.receiverTypeReference?.nameForReceiverLabel()
val fqName = target?.resolveFullType()
outputFile.appendText("$fqName.${function.nameAsSafeName}(")

val containerFqName = if (function.isExtensionDeclaration()) {
val receiverType = function.receiverTypeReference?.typeElement
val target = if (receiverType is KtNullableType) {
receiverType.innerType?.fullType()
} else {
receiverType?.fullType()
}
val parentType = function.typeParameters.filter { it.name == target }
.map { it.extendsBound?.typeElement?.fullType() }
.firstOrNull()
parentType ?: target ?: "null"
} else {
val parentName = function.containingClassOrObject?.fqName
?: function.containingKtFile.packageFqName
outputFile.appendText("$parentName.${function.nameAsSafeName}(")
function.containingClassOrObject?.fqName?.asString()
?: function.containingKtFile.packageFqName.asString()
}

parameterList?.children?.filterIsInstance<KtParameter>()?.forEachIndexed { idx, p ->
val typeElement = p.typeReference?.typeElement
if (idx > 0) outputFile.appendText(", ")
outputFile.appendText(typeElement?.fullType() ?: "???")
}
if (internalPackagePrefix.isBlank() || containerFqName.startsWith(internalPackagePrefix)) {
outputFile.appendText("$containerFqName.${function.nameAsSafeName}(")

outputFile.appendText(")\n")
val parameterList = function.getChildrenOfType<KtParameterList>().firstOrNull()
parameterList?.children?.filterIsInstance<KtParameter>()?.forEachIndexed { idx, p ->
val typeElement = p.typeReference?.typeElement
if (idx > 0) outputFile.appendText(", ")
outputFile.appendText(typeElement?.fullType() ?: "???")
}

outputFile.appendText(")\n")
}
}

// endregion
Expand Down Expand Up @@ -137,4 +175,14 @@ class ApiSurface(
}

// endregion

companion object {
private val ignoredFunctionNames = setOf("toString", "equals", "hashCode")

private val ignoredKeywords = setOf(
KtTokens.PRIVATE_KEYWORD,
KtTokens.PROTECTED_KEYWORD,
KtTokens.INTERNAL_KEYWORD
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class ApiUsage(
private val outputFileName: String by config(defaultValue = "apiUsage.log")
private val outputFile: File by lazy { File(outputFileName) }

private val internalPackagePrefix: String by config(defaultValue = "")

private var visitingTestFunction = false

// region Rule
Expand Down Expand Up @@ -63,8 +65,10 @@ class ApiUsage(
expression: KtCallExpression,
resolvedCall: ResolvedFunCall
) {
outputFile.appendText(resolvedCall.call)
outputFile.appendText("\n")
if (internalPackagePrefix.isBlank() || resolvedCall.containerFqName.startsWith(internalPackagePrefix)) {
outputFile.appendText(resolvedCall.call)
outputFile.appendText("\n")
}
}

// endregion
Expand Down