From 71e285f594d328344ce883154eec5930cb50e383 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Thu, 8 Feb 2024 14:55:45 -0500 Subject: [PATCH] Add functional test for both NDK and Mapping upload Fix other issues from code review. --- .../gradle/plugin/DdAndroidGradlePlugin.kt | 2 +- .../datadog/gradle/plugin/DdFileUploadTask.kt | 31 ++--- .../plugin/DdNdkSymbolFileUploadTask.kt | 47 ++++---- .../plugin/internal/GitRepositoryDetector.kt | 1 - .../DdAndroidGradlePluginFunctionalTest.kt | 107 +++++++++++++++--- .../plugin/DdAndroidGradlePluginTest.kt | 4 +- .../build_with_native_and_datadog_dep.gradle | 7 -- ...ith_proguard_native_and_datadog_dep.gradle | 74 ++++++++++++ 8 files changed, 205 insertions(+), 68 deletions(-) create mode 100644 dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_proguard_native_and_datadog_dep.gradle diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt index 0d025bf8..1528f71e 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePlugin.kt @@ -120,7 +120,7 @@ class DdAndroidGradlePlugin @Inject constructor( target: Project, extension: DdExtension, variant: ApplicationVariant, - buildIdTask: TaskProvider, + buildIdTask: TaskProvider ): TaskProvider? { val apiKey = resolveApiKey(target) val extensionConfiguration = resolveExtensionConfiguration(extension, variant) diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdFileUploadTask.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdFileUploadTask.kt index 0ebe142a..7a027e1d 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdFileUploadTask.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdFileUploadTask.kt @@ -146,19 +146,17 @@ abstract class DdFileUploadTask @Inject constructor( val mappingFiles = getFilesList() if (mappingFiles.isEmpty()) return - var repositories = emptyList() - repositoryDetector?.let { - repositories = it.detectRepositories( - sourceSetRoots, - remoteRepositoryUrl - ) - } + val repositories = repositoryDetector?.detectRepositories( + sourceSetRoots, + remoteRepositoryUrl + ).orEmpty() + if (repositories.isNotEmpty()) { generateRepositoryFile(repositories) } val site = DatadogSite.valueOf(site) - var lastError: Exception? = null + var caughtErrors = mutableListOf() for (mappingFile in mappingFiles) { LOGGER.info("Uploading ${mappingFile.fileType} file: ${mappingFile.file.absolutePath}") try { @@ -178,13 +176,20 @@ abstract class DdFileUploadTask @Inject constructor( !disableGzipOption.isPresent ) } catch (e: Exception) { - LOGGER.error("Failed to upload ${mappingFile.fileType} file", e) - lastError = e + caughtErrors.add(e) } } - // If any of these errored, throw the last error that occured - if (lastError != null) { - throw lastError + // If any errors occurred, throw them as a single exception + if (caughtErrors.isNotEmpty()) { + if (caughtErrors.count() == 1) { + throw caughtErrors.first() + } else { + val consolidatedError = Exception("Multiple errors occurred during upload") + caughtErrors.forEach { + consolidatedError.addSuppressed(it) + } + throw consolidatedError + } } } diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdNdkSymbolFileUploadTask.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdNdkSymbolFileUploadTask.kt index e7c55343..6997dc33 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdNdkSymbolFileUploadTask.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdNdkSymbolFileUploadTask.kt @@ -39,7 +39,6 @@ internal abstract class DdNdkSymbolFileUploadTask @Inject constructor( searchDirectories .flatMap(this::findSoFiles) - .toSortedSet(compareBy { it.absolutePath }) .toSet() .forEach { file -> val arch = file.parentFile.name @@ -72,7 +71,7 @@ internal abstract class DdNdkSymbolFileUploadTask @Inject constructor( } companion object { - internal const val TASK_NAME = "ddUploadNdkSymbolFiles" + internal const val TASK_NAME = "uploadNdkSymbolFiles" internal const val KEY_NDK_SYMBOL_FILE = "ndk_symbol_file" internal const val TYPE_NDK_SYMBOL_FILE = "ndk_symbol_file" internal const val ENCODING = "application/octet-stream" @@ -104,30 +103,29 @@ internal abstract class DdNdkSymbolFileUploadTask @Inject constructor( } @Suppress("MagicNumber", "ReturnCount") - private fun isAgp7OrAbove(): Boolean { + private fun isAgpAbove(major: Int, minor: Int, patch: Int): Boolean { val version = Version.ANDROID_GRADLE_PLUGIN_VERSION val groups = version.split(".") if (groups.size < 3) return false - val major = groups[0].toIntOrNull() - val minor = groups[1].toIntOrNull() - val patch = groups[2].substringBefore("-").toIntOrNull() - if (major == null || minor == null || patch == null) return false - return major >= 7 && minor >= 0 && patch >= 0 + val currentMajor = groups[0].toIntOrNull() + val currentMinor = groups[1].toIntOrNull() + val currentPatch = groups[2].substringBefore("-").toIntOrNull() + if (currentMajor == null || currentMinor == null || currentPatch == null) return false + return currentMajor >= major && currentMinor >= minor && currentPatch >= patch } - private fun fixNativeOutputPath(taskFolder: File): File { - return taskFolder.parentFile.parentFile.takeIf { it.parentFile.name == "cxx" } - ?: taskFolder.parentFile.parentFile.parentFile.takeIf { it.parentFile.name == "cxx" } - ?: taskFolder - } - - private fun getSearchDir(buildTask: TaskProvider, propertyName: String, providerFactory: ProviderFactory): Provider { + private fun getSearchDirs(buildTask: TaskProvider, providerFactory: ProviderFactory): Provider { return buildTask.flatMap { task -> - val soFolder = ExternalNativeBuildTask::class.memberProperties.find { it.name == propertyName }?.get(task) - when (soFolder) { - is File -> providerFactory.provider { fixNativeOutputPath(soFolder) } - is DirectoryProperty -> soFolder.map { fixNativeOutputPath(it.asFile) } - else -> providerFactory.provider { File("") } + // var soFolder: Provider + if (isAgpAbove(8, 0, 0)) { + task.soFolder.map { it.asFile } + } else { + val soFolder = ExternalNativeBuildTask::class.memberProperties.find { it.name == "objFolder" }?.get(task) + when (soFolder) { + is File -> providerFactory.provider { soFolder } + is DirectoryProperty -> soFolder.map { it.asFile } + else -> providerFactory.provider { null } + } } } } @@ -144,7 +142,7 @@ internal abstract class DdNdkSymbolFileUploadTask @Inject constructor( ): TaskProvider? { val nativeBuildProviders = variant.externalNativeBuildProviders if (nativeBuildProviders.isEmpty()) { - LOGGER.info("No native build tasks found for variant ${variant.name}, skipping symbol file upload.") + LOGGER.info("No native build tasks found for variant ${variant.name}, skipping NDK symbol file upload.") return null } @@ -155,17 +153,14 @@ internal abstract class DdNdkSymbolFileUploadTask @Inject constructor( val roots = mutableListOf() variant.sourceSets.forEach { it -> roots.addAll(it.javaDirectories) - if (isAgp7OrAbove()) { + if (isAgpAbove(7, 0, 0)) { roots.addAll(it.kotlinDirectories) } } task.sourceSetRoots = roots nativeBuildProviders.forEach { buildTask -> - val searchFiles = arrayOf( - getSearchDir(buildTask, "soFolder", providerFactory), - getSearchDir(buildTask, "objFolder", providerFactory) - ) + val searchFiles = getSearchDirs(buildTask, providerFactory) task.searchDirectories.from(searchFiles) task.dependsOn(buildTask) diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/GitRepositoryDetector.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/GitRepositoryDetector.kt index d4d50c9e..2e540349 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/GitRepositoryDetector.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/GitRepositoryDetector.kt @@ -65,7 +65,6 @@ internal class GitRepositoryDetector( sourceSetRoots: List ): List { val files = mutableListOf() - LOGGER.info("Listing ${sourceSetRoots}") sourceSetRoots.forEach { sourceSetRoot -> LOGGER.info("Listing ${sourceSetRoot.absolutePath}") if (sourceSetRoot.exists() && sourceSetRoot.isDirectory) { diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt index 8b13aa3f..e252f095 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt @@ -35,7 +35,7 @@ import kotlin.io.path.Path @Extensions( ExtendWith(ForgeExtension::class) ) -@ForgeConfiguration(Configurator::class) +@ForgeConfiguration(value = Configurator::class) internal class DdAndroidGradlePluginFunctionalTest { @TempDir lateinit var testProjectDir: File @@ -63,7 +63,6 @@ internal class DdAndroidGradlePluginFunctionalTest { private var cmakeFile: File? = null private var cppPlaceholderFile: File? = null - @StringForgery(regex = "http[s]?://github\\.com:[1-9]{2}/[a-z]+/repository\\.git") lateinit var fakeRemoteUrl: String private val colors = listOf("Blue", "Green") @@ -945,7 +944,7 @@ internal class DdAndroidGradlePluginFunctionalTest { // endregion - // region Symbol Upload + // region NDK Symbol Upload @Test fun `M not try to upload the symbol file W no cmake dependencies { using a fake API_KEY }`(forge: Forge) { @@ -976,7 +975,7 @@ internal class DdAndroidGradlePluginFunctionalTest { val version = forge.anElementFrom(versions) val variantVersionName = version.lowercase() val variant = "${version.lowercase()}$color" - val taskName = resolveSymbolUploadTask(variant) + val taskName = resolveNdkSymbolUploadTask(variant) gradleRunner { withArguments("--info", ":samples:app:assembleRelease") } .build() @@ -996,15 +995,15 @@ internal class DdAndroidGradlePluginFunctionalTest { assertThat(result).containsInOutput("Creating request with GZIP encoding.") - assertThat(result).containsInOutput("Uploading ndk_symbol_file file: "); + assertThat(result).containsInOutput("Uploading ndk_symbol_file file: ") assertThat(result).containsInOutput( "Uploading file libplaceholder.so with tags " + - "`service:com.example.variants.$variantVersionName`, " + - "`version:1.0-$variantVersionName`, " + - "`version_code:1`, " + - "`variant:$variant`, " + - "`build_id:$buildIdInOriginFile` (site=datadoghq.com):" + "`service:com.example.variants.$variantVersionName`, " + + "`version:1.0-$variantVersionName`, " + + "`version_code:1`, " + + "`variant:$variant`, " + + "`build_id:$buildIdInOriginFile` (site=datadoghq.com):" ) for (arch in SUPPORTED_ABIS) { @@ -1024,7 +1023,7 @@ internal class DdAndroidGradlePluginFunctionalTest { val version = forge.anElementFrom(versions) val variantVersionName = version.lowercase() val variant = "${version.lowercase()}$color" - val taskName = resolveSymbolUploadTask(variant) + val taskName = resolveNdkSymbolUploadTask(variant) gradleRunner { withArguments("--info", ":samples:app:assembleRelease") } .build() @@ -1044,15 +1043,15 @@ internal class DdAndroidGradlePluginFunctionalTest { assertThat(result).containsInOutput("Creating request without GZIP encoding.") - assertThat(result).containsInOutput("Uploading ndk_symbol_file file: "); + assertThat(result).containsInOutput("Uploading ndk_symbol_file file: ") assertThat(result).containsInOutput( "Uploading file libplaceholder.so with tags " + - "`service:com.example.variants.$variantVersionName`, " + - "`version:1.0-$variantVersionName`, " + - "`version_code:1`, " + - "`variant:$variant`, " + - "`build_id:$buildIdInOriginFile` (site=datadoghq.com):" + "`service:com.example.variants.$variantVersionName`, " + + "`version:1.0-$variantVersionName`, " + + "`version_code:1`, " + + "`variant:$variant`, " + + "`build_id:$buildIdInOriginFile` (site=datadoghq.com):" ) for (arch in SUPPORTED_ABIS) { @@ -1062,10 +1061,82 @@ internal class DdAndroidGradlePluginFunctionalTest { // endregion + @Test + fun `M try to upload the symbol file and mapping file W upload { using a fake API_KEY }`(forge: Forge) { + // Given + stubGradleBuildFromResourceFile( + "native_gradle_files/build_with_proguard_native_and_datadog_dep.gradle", + appBuildGradleFile + ) + stubNativeFiles() + + val color = forge.anElementFrom(colors) + val version = forge.anElementFrom(versions) + val variantVersionName = version.lowercase() + val variant = "${version.lowercase()}$color" + val ndkSymbolUploadTaskName = resolveNdkSymbolUploadTask(variant) + val mappingUploadTaskName = resolveMappingUploadTask(variant) + + // When + gradleRunner { withArguments("--info", ":samples:app:assembleRelease") } + .build() + + val nativeResult = gradleRunner { + withArguments( + ndkSymbolUploadTaskName, + "--info", + "--stacktrace", + "-PDD_API_KEY=fakekey" + ) + } + .buildAndFail() + + val mappingResult = gradleRunner { + withArguments( + mappingUploadTaskName, + "--info", + "--stacktrace", + "-PDD_API_KEY=fakekey" + ) + } + .buildAndFail() + + // Then + val buildIdInOriginFile = testProjectDir.findBuildIdInOriginFile(variant) + + assertThat(nativeResult).containsInOutput("Creating request with GZIP encoding.") + + assertThat(nativeResult).containsInOutput("Uploading ndk_symbol_file file: ") + + assertThat(nativeResult).containsInOutput( + "Uploading file libplaceholder.so with tags " + + "`service:com.example.variants.$variantVersionName`, " + + "`version:1.0-$variantVersionName`, " + + "`version_code:1`, " + + "`variant:$variant`, " + + "`build_id:$buildIdInOriginFile` (site=datadoghq.com):" + ) + + for (arch in SUPPORTED_ABIS) { + assertThat(nativeResult).containsInOutput("extra attributes: {arch=$arch}") + } + + assertThat(mappingResult).containsInOutput("Creating request with GZIP encoding.") + + assertThat(mappingResult).containsInOutput( + "Uploading file jvm_mapping with tags " + + "`service:com.example.variants.$variantVersionName`, " + + "`version:1.0-$variantVersionName`, " + + "`version_code:1`, " + + "`variant:$variant`, " + + "`build_id:$buildIdInOriginFile` (site=datadoghq.com):" + ) + } + // region Internal private fun resolveMappingUploadTask(variantName: String) = "uploadMapping${variantName}Release" - private fun resolveSymbolUploadTask(variantName: String) = "ddUploadNdkSymbolFiles${variantName}Release" + private fun resolveNdkSymbolUploadTask(variantName: String) = "uploadNdkSymbolFiles${variantName}Release" private fun stubFile(destination: File, content: String) { with(destination.outputStream()) { diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt index 7854bc8e..ac7d544b 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginTest.kt @@ -495,7 +495,7 @@ internal class DdAndroidGradlePluginTest { // Then val allTasks = fakeProject.tasks.map { it.name } - assertThat(allTasks).contains("ddUploadNdkSymbolFiles${variantName.replaceFirstChar { capitalizeChar(it) }}") + assertThat(allTasks).contains("uploadNdkSymbolFiles${variantName.replaceFirstChar { capitalizeChar(it) }}") } @Test @@ -531,7 +531,7 @@ internal class DdAndroidGradlePluginTest { // Then val allTasks = fakeProject.tasks.map { it.name } - assertThat(allTasks).allMatch { !it.startsWith("ddUploadNdkSymbolFiles") } + assertThat(allTasks).allMatch { !it.startsWith("uploadNdkSymbolFiles") } } @Test diff --git a/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_native_and_datadog_dep.gradle b/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_native_and_datadog_dep.gradle index 0c6be7b5..12ad73d0 100644 --- a/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_native_and_datadog_dep.gradle +++ b/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_native_and_datadog_dep.gradle @@ -22,13 +22,6 @@ android { multiDexEnabled = true } - buildTypes { - release { - minifyEnabled true - proguardFiles getDefaultProguardFile ('proguard-android-optimize.txt'), 'proguard-rules.pro' - } - } - namespace = "com.example.variants" compileOptions { diff --git a/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_proguard_native_and_datadog_dep.gradle b/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_proguard_native_and_datadog_dep.gradle new file mode 100644 index 00000000..404321ab --- /dev/null +++ b/dd-sdk-android-gradle-plugin/src/test/resources/native_gradle_files/build_with_proguard_native_and_datadog_dep.gradle @@ -0,0 +1,74 @@ +plugins { + id("com.android.application") + id("kotlin-android") + id("com.datadoghq.dd-sdk-android-gradle-plugin") +} + +repositories { + google() + mavenCentral() +} + +android { + compileSdkVersion = rootProject.ext.targetSdkVersion + buildToolsVersion = rootProject.ext.buildToolsVersion + + defaultConfig { + applicationId "com.example.variants" + minSdkVersion 21 + targetSdkVersion rootProject.ext.targetSdkVersion + versionCode 1 + versionName "1.0" + multiDexEnabled = true + } + + buildTypes { + release { + minifyEnabled true + proguardFiles getDefaultProguardFile ('proguard-android-optimize.txt'), 'proguard-rules.pro' + } + } + + namespace = "com.example.variants" + + compileOptions { + sourceCompatibility = rootProject.ext.jvmTarget + targetCompatibility = rootProject.ext.jvmTarget + } + + kotlinOptions { + jvmTarget = rootProject.ext.jvmTarget + } + + externalNativeBuild { + cmake { + path file('src/main/cpp/CMakeLists.txt') + version '3.22.1' + } + } + + flavorDimensions("version", "colour") + productFlavors { + demo { + dimension "version" + applicationIdSuffix ".demo" + versionNameSuffix "-demo" + } + full { + dimension "version" + applicationIdSuffix ".full" + versionNameSuffix "-full" + } + + green { + dimension "colour" + } + blue { + dimension "colour" + } + } +} + +dependencies { + implementation(rootProject.ext.datadogSdkDependency) +}