From d5e1b331e2b8ae14bbcb3d9dd0e554ff9ea7b383 Mon Sep 17 00:00:00 2001 From: Mikhail Date: Tue, 25 Feb 2020 17:01:48 +0300 Subject: [PATCH] PER-1548 Add possibility to give individual thresholds for tests (#208) --- .../performance/PerformanceTestReporter.kt | 27 ++++++++++--- .../stats/comparison/ComparedTest.kt | 6 +-- .../performance/PerformanceComparatorTest.kt | 27 +++++++------ .../avito/performance/stats/StatsApiTest.kt | 3 +- .../comparison/ComparedTestComparisonTest.kt | 40 ++++++++++++++++++- 5 files changed, 79 insertions(+), 24 deletions(-) diff --git a/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/PerformanceTestReporter.kt b/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/PerformanceTestReporter.kt index 0e1f233ab3..ff669465b8 100644 --- a/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/PerformanceTestReporter.kt +++ b/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/PerformanceTestReporter.kt @@ -62,7 +62,7 @@ internal enum class Metric( FPS( "FPS", shouldBeLess = false, - diffThreshold = 3.0, + diffThreshold = 2.0, isBlocker = true ), PROPER_FRAMES_PERCENT( @@ -74,7 +74,7 @@ internal enum class Metric( ACTIVITY_STARTUP_TIME( "ActivityStartupTime", shouldBeLess = true, - diffThreshold = 150.0, + diffThreshold = 200.0, isBlocker = true ), JUNKY_FRAME_MAX_DURATION( @@ -113,10 +113,7 @@ internal fun ComparedTest.Comparison.failed() = } else { val greaterThanExpected = it.greaterThanExpected(metric) val lessThanExpected = it.lessThanExpected(metric) - val significantThreshold = significantThreshold( - value = it.value.meanDiff, - threshold = metric.diffThreshold - ) + val significantThreshold = isSignificant(it.value, metric) val blocker = metric.isBlocker ((lessThanExpected || greaterThanExpected) && significantThreshold @@ -124,6 +121,23 @@ internal fun ComparedTest.Comparison.failed() = } } +private fun isSignificant( + series: ComparedTest.Series, + metric: Metric +): Boolean { + return if (abs(series.thresholdStatic) < EPSILON) { + significantThreshold( + value = series.meanDiff, + threshold = metric.diffThreshold + ) + } else { + significantThreshold( + value = series.meanDiff, + threshold = series.thresholdStatic + ) + } +} + internal fun ComparedTest.Comparison.performedMuchBetterThanUsual() = this.series .filter { @@ -146,3 +160,4 @@ internal fun ComparedTest.Comparison.performedMuchBetterThanUsual() = private const val NEW_LINE = "\n" private const val PERFORMER = "Performer" +private const val EPSILON = 1e-9 diff --git a/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/stats/comparison/ComparedTest.kt b/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/stats/comparison/ComparedTest.kt index cc48558cd7..7c919709ec 100644 --- a/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/stats/comparison/ComparedTest.kt +++ b/subprojects/gradle/performance/src/main/kotlin/com/avito/performance/stats/comparison/ComparedTest.kt @@ -14,9 +14,9 @@ internal interface ComparedTest { val currentSampleIs: State, val statistic: Double, val pValue: Double, - //current-previous - val meanDiff: Double, - val threshold: Double + val meanDiff: Double, //current-previous + val threshold: Double, + val thresholdStatic: Double //overrides threshold for particular tests in mongo ) enum class State { diff --git a/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/PerformanceComparatorTest.kt b/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/PerformanceComparatorTest.kt index f5275f05a9..6d7835d148 100644 --- a/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/PerformanceComparatorTest.kt +++ b/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/PerformanceComparatorTest.kt @@ -37,14 +37,14 @@ internal class PerformanceComparatorTest { Assertions.assertTrue(outputFile.exists()) { "current-runs.txt should exist!" } Truth.assertThat(outputFile.readText()).contains( - ("""[{"testName":"testname0\"","id":"0","series":{"testname0\"_fps":{"significance":0.0, - |"currentSampleIs":"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0}}}, - |{"testName":"testname1\"","id":"1","series":{"testname1\"_fps":{"significance":0.0, - |"currentSampleIs":"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0}}}, - |{"testName":"testname2\"","id":"2","series":{"testname2\"_fps":{"significance":0.0, - |"currentSampleIs":"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0}}}, - |{"testName":"testname3\"","id":"3","series":{"testname3\"_fps":{"significance":0.0, - |"currentSampleIs":"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0}}}]""" + ("""[{"testName":"testname0\"","id":"0","series":{"testname0\"_fps":{"significance":0.0,"currentSampleIs": + |"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0,"thresholdStatic":0.0}}}, + |{"testName":"testname1\"","id":"1","series":{"testname1\"_fps":{"significance":0.0,"currentSampleIs": + |"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0,"thresholdStatic":0.0}}}, + |{"testName":"testname2\"","id":"2","series":{"testname2\"_fps":{"significance":0.0,"currentSampleIs": + |"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0,"thresholdStatic":0.0}}}, + |{"testName":"testname3\"","id":"3","series":{"testname3\"_fps":{"significance":0.0,"currentSampleIs": + |"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0,"thresholdStatic":0.0}}}]""" .trimMargin()) .trimIndent() .replace("\n", "") @@ -61,10 +61,10 @@ internal class PerformanceComparatorTest { Assertions.assertTrue(outputFile.exists()) { "current-runs.txt should exist" } Truth.assertThat(outputFile.readText()).contains( - ("""[{"testName":"testname0\"","id":"0","series":{"testname0\"_fps":{"significance":0.0, - |"currentSampleIs":"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0}}}, - |{"testName":"testname1\"","id":"1","series":{"testname1\"_fps":{"significance":0.0, - |"currentSampleIs":"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0}}}]""" + ("""[{"testName":"testname0\"","id":"0","series":{"testname0\"_fps":{"significance":0.0,"currentSampleIs" + |:"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0,"thresholdStatic":0.0}}}, + |{"testName":"testname1\"","id":"1","series":{"testname1\"_fps":{"significance":0.0,"currentSampleIs": + |"same","statistic":0.0,"pValue":1.0,"meanDiff":2.0,"threshold":100.0,"thresholdStatic":0.0}}}]""" .trimMargin()) .trimIndent() .replace("\n", "") @@ -112,7 +112,8 @@ internal class PerformanceComparatorTest { 0.0, 1.0, 2.0, - threshold = 100.0 + threshold = 100.0, + thresholdStatic = 0.0 ) }.toMap() ) diff --git a/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/StatsApiTest.kt b/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/StatsApiTest.kt index a2fc625936..d40b9d0e98 100644 --- a/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/StatsApiTest.kt +++ b/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/StatsApiTest.kt @@ -48,7 +48,8 @@ internal class StatsApiTest { statistic = 0.0, pValue = 0.2, meanDiff = 2.0, - threshold = 100.0 + threshold = 100.0, + thresholdStatic = 0.0 ) ) ) diff --git a/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/comparison/ComparedTestComparisonTest.kt b/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/comparison/ComparedTestComparisonTest.kt index 731b74bdb6..f6ace6692b 100644 --- a/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/comparison/ComparedTestComparisonTest.kt +++ b/subprojects/gradle/performance/src/test/kotlin/com/avito/performance/stats/comparison/ComparedTestComparisonTest.kt @@ -157,6 +157,42 @@ internal class ComparedTestComparisonTest { .hasSize(1) } + @Test + fun `failed - returns 0 - if less than zero and should be greater and less than static threshold`() { + assertThat( + givenTestComparison( + currentSampleIs = LESS, + meanDiff = -1.0, + metric = "FPS", + thresholdStatic = 2.0 + ).failed()) + .hasSize(0) + } + + @Test + fun `failed - returns 0 - if greater than zero and should be greater and greater than static threshold`() { + assertThat( + givenTestComparison( + currentSampleIs = GREATER, + meanDiff = 1.0, + metric = "FPS", + thresholdStatic = 0.5 + ).failed()) + .hasSize(0) + } + + @Test + fun `failed - returns 1 - if less than zero and should be greater and greater than static threshold`() { + assertThat( + givenTestComparison( + currentSampleIs = LESS, + meanDiff = -1.0, + metric = "FPS", + thresholdStatic = 0.5 + ).failed()) + .hasSize(1) + } + @Test fun `failed - returns 1 - if less than zero and should be greater and greater than threshold`() { assertThat( @@ -187,6 +223,7 @@ internal class ComparedTestComparisonTest { //current-previous meanDiff: Double = 0.0, threshold: Double = 0.0, + thresholdStatic: Double = 0.0, metric: String = "FPS" ): Comparison { return Comparison( @@ -198,7 +235,8 @@ internal class ComparedTestComparisonTest { statistic, pValue, meanDiff, - threshold + threshold, + thresholdStatic ) ) )