diff --git a/libraries/src/main/kotlin/com/avito/android/ExternalLibrariesExtension.kt b/libraries/src/main/kotlin/com/avito/android/ExternalLibrariesExtension.kt index 7ed41edc9d..5cdbb1ff93 100644 --- a/libraries/src/main/kotlin/com/avito/android/ExternalLibrariesExtension.kt +++ b/libraries/src/main/kotlin/com/avito/android/ExternalLibrariesExtension.kt @@ -94,7 +94,7 @@ abstract class ExternalLibrariesExtension @Inject constructor(private val provid val kotlinPlugin = "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" val androidGradlePlugin = "com.android.tools.build:gradle:$androidGradlePluginVersion" val nebulaIntegTest = "com.netflix.nebula:nebula-project-plugin:$nebulaIntegTestVersion" - val gradleTestRetryPlugin = "org.gradle:test-retry-gradle-plugin:1.2.1" + val gradleTestRetryPlugin = "org.gradle:test-retry-gradle-plugin:1.3.1" val androidXTestRunner = "androidx.test:runner:${Versions.androidXTest}" val testOrchestrator = "androidx.test:orchestrator:${Versions.androidXTest}" diff --git a/subprojects/common/junit-utils/src/main/java/com/avito/test/Flaky.kt b/subprojects/common/junit-utils/src/main/java/com/avito/test/Flaky.kt index 913f9954a2..9b22f8addd 100644 --- a/subprojects/common/junit-utils/src/main/java/com/avito/test/Flaky.kt +++ b/subprojects/common/junit-utils/src/main/java/com/avito/test/Flaky.kt @@ -4,6 +4,7 @@ package com.avito.test * Unstable test marker. * It's used for retry logic. */ +@Retention(AnnotationRetention.BINARY) annotation class Flaky( val reason: String ) diff --git a/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheMetricsTest.kt b/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheMetricsTest.kt index 29e543877f..d937a3f311 100644 --- a/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheMetricsTest.kt +++ b/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheMetricsTest.kt @@ -1,11 +1,11 @@ package com.avito.android.plugin.build_metrics.cache -import com.avito.android.plugin.build_metrics.assertHasMetric import com.avito.android.plugin.build_metrics.assertNoMetric +import com.avito.android.plugin.build_metrics.statsdMetrics import com.avito.android.stats.CountMetric import com.avito.android.stats.StatsMetric import com.avito.test.gradle.TestResult -import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.assertWithMessage import org.gradle.testkit.runner.TaskOutcome import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.TestFactory @@ -45,7 +45,6 @@ internal class HttpBuildCacheMetricsTest : HttpBuildCacheTestFixture() { private class TestCase( val name: String, - val enabled: Boolean = true, val loadStatus: Int, val storeStatus: Int, val assertion: (result: TestResult) -> Unit @@ -60,63 +59,43 @@ internal class HttpBuildCacheMetricsTest : HttpBuildCacheTestFixture() { result.assertNoMetric(".build.cache.errors.") } ), - // TODO: this case is flaky - // If it's failed, please add info to MBS-11302 TestCase( name = "load error - 500 response", - enabled = false, loadStatus = 500, storeStatus = 200, assertion = { result -> - result.assertHasMetric(".build.cache.errors.load.500").also { - assertThat(it.delta).isEqualTo(1) - } + result.assertHasEvents(".build.cache.errors.load.500") } ), - // TODO: this case is flaky - // If it's failed, please add info to MBS-11302 TestCase( name = "store error - 500 response", - enabled = false, loadStatus = 404, storeStatus = 500, assertion = { result -> - result.assertHasMetric(".build.cache.errors.store.500").also { - assertThat(it.delta).isEqualTo(1) - } + result.assertHasEvents(".build.cache.errors.store.500") } ), - // TODO: this case is flaky - // If it's failed, please add info to MBS-11302 TestCase( name = "store error - unknown error", - enabled = false, loadStatus = 404, storeStatus = invalidHttpStatus, assertion = { result -> - result.assertHasMetric(".build.cache.errors.store.unknown").also { - assertThat(it.delta).isEqualTo(1) - } + result.assertHasEvents(".build.cache.errors.store.unknown") } ), - // TODO: this case is flaky - // If it's failed, please add info to MBS-11302 TestCase( name = "load error - unknown error", - enabled = false, loadStatus = invalidHttpStatus, storeStatus = 200, assertion = { result -> - result.assertHasMetric(".build.cache.errors.load.unknown").also { - assertThat(it.delta).isEqualTo(1) - } + result.assertHasEvents(".build.cache.errors.load.unknown") } ), ) @TestFactory fun `remote cache errors`(): List { - return cases.filter { it.enabled }.map { case -> + return cases.map { case -> DynamicTest.dynamicTest(case.name) { setup() @@ -136,6 +115,18 @@ internal class HttpBuildCacheMetricsTest : HttpBuildCacheTestFixture() { } } } + + private fun TestResult.assertHasEvents(path: String) { + val metrics = statsdMetrics() + val filtered = metrics + .filterIsInstance(CountMetric::class.java) + .filter { + it.name.toString().contains(path) + } + + assertWithMessage("Expected metrics ($path) in $metrics. Logs: $output") + .that(filtered.size).isAtLeast(1) + } } private const val invalidHttpStatus = -1 diff --git a/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheTestFixture.kt b/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheTestFixture.kt index b077ac8f56..ac71a7d3f2 100644 --- a/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheTestFixture.kt +++ b/subprojects/gradle/build-metrics/src/gradleTest/kotlin/com/avito/android/plugin/build_metrics/cache/HttpBuildCacheTestFixture.kt @@ -75,6 +75,12 @@ internal abstract class HttpBuildCacheTestFixture { protected fun build(vararg args: String) = BuildMetricsRunner(projectDir) .build( - args.toList().plus("--build-cache") + args.toList().plus(listOf( + "--build-cache", + // Cache errors can happen for Kotlin DSL scripts. + // It disables cache before applying the plugin for metrics + // This is why we have to intercept all errors in tests + "-Dorg.gradle.unsafe.build-cache.remote-continue-on-error=true" + )) ) } diff --git a/subprojects/gradle/critical-path/critical-path/src/gradleTest/kotlin/com/avito/android/critical_path/CriticalPathTest.kt b/subprojects/gradle/critical-path/critical-path/src/gradleTest/kotlin/com/avito/android/critical_path/CriticalPathTest.kt index ad003bc1c8..d29bc46266 100644 --- a/subprojects/gradle/critical-path/critical-path/src/gradleTest/kotlin/com/avito/android/critical_path/CriticalPathTest.kt +++ b/subprojects/gradle/critical-path/critical-path/src/gradleTest/kotlin/com/avito/android/critical_path/CriticalPathTest.kt @@ -1,7 +1,6 @@ package com.avito.android.critical_path import com.avito.android.critical_path.internal.CriticalPathReport -import com.avito.test.Flaky import com.avito.test.gradle.TestResult import com.avito.test.gradle.gradlew import com.google.common.truth.Truth.assertThat @@ -12,7 +11,9 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir import java.io.File -@Flaky(reason = "Will be stabilized in MBS-11302") +/** + * These tests check Gradle integration scenarios but not critical path logic itself. + */ internal class CriticalPathTest { private lateinit var projectDir: File @@ -71,11 +72,11 @@ internal class CriticalPathTest { fun `dependsOn dependent task - path has a dependent task`() { setupTasks( """ - ${delayTaskDeclaration()} + ${customTaskDeclaration()} - tasks.register("first", DelayTask::class.java) + tasks.register("first", CustomTask::class.java) - tasks.register("second", DelayTask::class.java) { + tasks.register("second", CustomTask::class.java) { dependsOn(":first") } """.trimIndent() @@ -90,11 +91,11 @@ internal class CriticalPathTest { fun `mustRunAfter dependent task - path has a dependent task`() { setupTasks( """ - ${delayTaskDeclaration()} + ${customTaskDeclaration()} - tasks.register("first", DelayTask::class.java) + tasks.register("first", CustomTask::class.java) - tasks.register("second", DelayTask::class.java) { + tasks.register("second", CustomTask::class.java) { shouldRunAfter(":first") } """.trimIndent() @@ -111,72 +112,14 @@ internal class CriticalPathTest { // no op } - @Disabled("Fails constantly") - @Test - fun `independent routes - path has the longest one`() { - setupTasks( - """ - ${delayTaskDeclaration()} - - tasks.register("short_first", DelayTask::class.java) { - durationMs.set(0) - } - tasks.register("short_last", DelayTask::class.java) { - durationMs.set(0) - dependsOn(":short_first") - } - - tasks.register("long_first", DelayTask::class.java) { - durationMs.set(100) - } - tasks.register("long_last", DelayTask::class.java) { - durationMs.set(100) - dependsOn(":long_first") - } - """.trimIndent() - ) - - // TODO: remove verbose logging after stabilizing tests MBS-11302 - val tasks = calculatePath(":short_last", ":long_last", args = listOf("--info")) - - assertThat(tasks).containsExactly(":long_first", ":long_last") - } - - @Test - fun `parallel routes - path has the longest task`() { - setupTasks( - """ - ${delayTaskDeclaration()} - - tasks.register("first", DelayTask::class.java) - - tasks.register("intermediate_1", DelayTask::class.java){ - durationMs.set(1) - dependsOn(":first") - } - tasks.register("intermediate_100", DelayTask::class.java){ - durationMs.set(100) - dependsOn(":first") - } - - tasks.register("last", DelayTask::class.java) { - dependsOn(":intermediate_1", ":intermediate_100") - } - """.trimIndent() - ) - val tasks = calculatePath(":last") - - assertThat(tasks).containsExactly(":first", ":intermediate_100", ":last") - } - @Test fun `disabled plugin - no report`() { setupTasks( enabledPlugin = false, buildScript = """ - ${delayTaskDeclaration()} + ${customTaskDeclaration()} - tasks.register("work", DelayTask::class.java) + tasks.register("work", CustomTask::class.java) """.trimIndent() ) @@ -207,17 +150,16 @@ internal class CriticalPathTest { ) } - private fun delayTaskDeclaration(): String { + private fun customTaskDeclaration(): String { return """ - abstract class DelayTask @Inject constructor(objects: ObjectFactory) : DefaultTask() { + abstract class CustomTask @Inject constructor(objects: ObjectFactory) : DefaultTask() { - @Input - var durationMs = objects.property().convention(1) + init { + outputs.upToDateWhen { false } + } @TaskAction - fun createFile() { - Thread.sleep(durationMs.get()) - } + fun action() { } } """.trimIndent() }