Change benchmark warnings to be true errors, which fail test by default
Fixes: 137653596
Test: ./gradlew benchmark:cC
They can be suppressed by instrumentation arg, but outputs will still
be mangled.
This makes it much less likely to have to deal with bad data in CI.
Benchmark tests suppress DEBUGGABLE, CODE-COVERAGE, EMULATOR,
LOW-BATTERY, and UNLOCKED since any of those is fine for correctness
tests (and all occur in correctness test CI).
They do this from code though, as instrumentation args defined in
gradle aren't currently picked up by our CI.
Change-Id: I4308e565c08da413b4c5e5a890145802937554dc
diff --git a/benchmark/api/1.0.0-alpha04.txt b/benchmark/api/1.0.0-alpha04.txt
index 268140e..96d4bc8 100644
--- a/benchmark/api/1.0.0-alpha04.txt
+++ b/benchmark/api/1.0.0-alpha04.txt
@@ -5,6 +5,10 @@
ctor public AndroidBenchmarkRunner();
}
+ public final class ArgumentsKt {
+ ctor public ArgumentsKt();
+ }
+
public final class BenchmarkRule implements org.junit.rules.TestRule {
ctor public BenchmarkRule();
method public org.junit.runners.model.Statement apply(org.junit.runners.model.Statement base, org.junit.runner.Description description);
diff --git a/benchmark/api/current.txt b/benchmark/api/current.txt
index 268140e..96d4bc8 100644
--- a/benchmark/api/current.txt
+++ b/benchmark/api/current.txt
@@ -5,6 +5,10 @@
ctor public AndroidBenchmarkRunner();
}
+ public final class ArgumentsKt {
+ ctor public ArgumentsKt();
+ }
+
public final class BenchmarkRule implements org.junit.rules.TestRule {
ctor public BenchmarkRule();
method public org.junit.runners.model.Statement apply(org.junit.runners.model.Statement base, org.junit.runner.Description description);
diff --git a/benchmark/api/restricted_1.0.0-alpha04.txt b/benchmark/api/restricted_1.0.0-alpha04.txt
index a4c1fd3..35c0a19 100644
--- a/benchmark/api/restricted_1.0.0-alpha04.txt
+++ b/benchmark/api/restricted_1.0.0-alpha04.txt
@@ -5,6 +5,10 @@
ctor public AndroidBenchmarkRunner();
}
+ public final class ArgumentsKt {
+ ctor public ArgumentsKt();
+ }
+
public final class BenchmarkRule implements org.junit.rules.TestRule {
ctor public BenchmarkRule();
method public org.junit.runners.model.Statement apply(org.junit.runners.model.Statement base, org.junit.runner.Description description);
diff --git a/benchmark/api/restricted_current.txt b/benchmark/api/restricted_current.txt
index a4c1fd3..35c0a19 100644
--- a/benchmark/api/restricted_current.txt
+++ b/benchmark/api/restricted_current.txt
@@ -5,6 +5,10 @@
ctor public AndroidBenchmarkRunner();
}
+ public final class ArgumentsKt {
+ ctor public ArgumentsKt();
+ }
+
public final class BenchmarkRule implements org.junit.rules.TestRule {
ctor public BenchmarkRule();
method public org.junit.runners.model.Statement apply(org.junit.runners.model.Statement base, org.junit.runner.Description description);
diff --git a/benchmark/src/androidTest/AndroidManifest.xml b/benchmark/src/androidTest/AndroidManifest.xml
index ccafe73..f5ec776 100644
--- a/benchmark/src/androidTest/AndroidManifest.xml
+++ b/benchmark/src/androidTest/AndroidManifest.xml
@@ -19,7 +19,8 @@
xmlns:tools="http://schemas.android.com/tools"
package="androidx.benchmark.test">
- <application>
+ <application
+ android:name="androidx.benchmark.ArgumentInjectingApplication">
<activity android:name="android.app.Activity"/>
</application>
</manifest>
\ No newline at end of file
diff --git a/benchmark/src/androidTest/java/androidx/benchmark/ArgumentInjectingApplication.kt b/benchmark/src/androidTest/java/androidx/benchmark/ArgumentInjectingApplication.kt
new file mode 100644
index 0000000..3f6f804
--- /dev/null
+++ b/benchmark/src/androidTest/java/androidx/benchmark/ArgumentInjectingApplication.kt
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.benchmark
+
+import android.app.Application
+import android.os.Bundle
+
+/**
+ * Hack to enable overriding benchmark arguments (since we can't easily do this in CI, per apk)
+ *
+ * The *correct* way to do this would be to put the following in benchmark/build.gradle:
+ *
+ * ```
+ * android {
+ * defaultConfig {
+ * testInstrumentationRunnerArgument 'androidx.benchmark.suppressErrors',
+ * 'CODE-COVERAGE,DEBUGGABLE,EMULATOR,LOW-BATTERY,UNLOCKED'
+ * }
+ * }
+ * ```
+ */
+class ArgumentInjectingApplication : Application() {
+ override fun onCreate() {
+ super.onCreate()
+
+ ArgumentSource = Bundle().apply {
+ putString("androidx.benchmark.output.enable", "true")
+
+ // Since these benchmark correctness tests run as part of the regular
+ // (non-performance-test) suite, they will have debuggable=true, won't be clock-locked,
+ // can run with low-battery or on an emulator, and code coverage enabled.
+ putString(
+ "androidx.benchmark.suppressErrors",
+ "CODE-COVERAGE,DEBUGGABLE,EMULATOR,LOW-BATTERY,UNLOCKED"
+ )
+ }
+ }
+}
\ No newline at end of file
diff --git a/benchmark/src/androidTest/java/androidx/benchmark/BenchmarkStateTest.kt b/benchmark/src/androidTest/java/androidx/benchmark/BenchmarkStateTest.kt
index 4113d41..f5f7710 100644
--- a/benchmark/src/androidTest/java/androidx/benchmark/BenchmarkStateTest.kt
+++ b/benchmark/src/androidTest/java/androidx/benchmark/BenchmarkStateTest.kt
@@ -94,7 +94,7 @@
)
// check attribute presence and naming
- val prefix = WarningState.WARNING_PREFIX
+ val prefix = Errors.WARNING_PREFIX
assertNotNull(bundle.get("${prefix}min"))
assertNotNull(bundle.get("${prefix}mean"))
assertNotNull(bundle.get("${prefix}count"))
diff --git a/benchmark/src/main/java/androidx/benchmark/AndroidBenchmarkRunner.kt b/benchmark/src/main/java/androidx/benchmark/AndroidBenchmarkRunner.kt
index 1df88f2..592be43 100644
--- a/benchmark/src/main/java/androidx/benchmark/AndroidBenchmarkRunner.kt
+++ b/benchmark/src/main/java/androidx/benchmark/AndroidBenchmarkRunner.kt
@@ -74,8 +74,8 @@
override fun onCreate(arguments: Bundle?) {
super.onCreate(arguments)
- // Because these values are used by WarningState, it's important to set this flag as early
- // as possible, before WarningState gets lazily initialized. Otherwise we may print false
+ // Because these values are used by Errors, it's important to set this flag as early
+ // as possible, before Errors gets lazily initialized. Otherwise we may print false
// warnings about needing the runner, when the runner simply hasn't initialized yet.
runnerInUse = true
sustainedPerformanceModeInUse = !CpuInfo.locked && isSustainedPerformanceModeSupported()
diff --git a/benchmark/src/main/java/androidx/benchmark/Arguments.kt b/benchmark/src/main/java/androidx/benchmark/Arguments.kt
new file mode 100644
index 0000000..5221e26
--- /dev/null
+++ b/benchmark/src/main/java/androidx/benchmark/Arguments.kt
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.benchmark
+
+import android.os.Bundle
+import androidx.test.platform.app.InstrumentationRegistry
+
+// This allows tests to override arguments from code
+internal var ArgumentSource: Bundle? = null
+
+internal object Arguments {
+ val outputEnable: Boolean
+ val suppressedErrors: Set<String>
+
+ init {
+ val prefix = "androidx.benchmark"
+ val arguments = ArgumentSource ?: InstrumentationRegistry.getArguments()
+ outputEnable = arguments.getString("$prefix.output.enable")?.toBoolean() ?: false
+
+ // Transform comma-delimited list into set of suppressed errors
+ // E.g. "DEBUGGABLE, UNLOCKED" -> setOf("DEBUGGABLE", "UNLOCKED")
+ suppressedErrors = arguments.getString("androidx.benchmark.suppressErrors", "")
+ .split(',')
+ .map { it.trim() }
+ .filter { it.isNotEmpty() }
+ .toSet()
+ }
+}
\ No newline at end of file
diff --git a/benchmark/src/main/java/androidx/benchmark/BenchmarkRule.kt b/benchmark/src/main/java/androidx/benchmark/BenchmarkRule.kt
index f185133..3b3875a 100644
--- a/benchmark/src/main/java/androidx/benchmark/BenchmarkRule.kt
+++ b/benchmark/src/main/java/androidx/benchmark/BenchmarkRule.kt
@@ -19,7 +19,7 @@
import android.Manifest
import android.util.Log
import androidx.annotation.RestrictTo
-import androidx.benchmark.WarningState.WARNING_PREFIX
+import androidx.benchmark.Errors.WARNING_PREFIX
import androidx.test.rule.GrantPermissionRule
import org.junit.Assert.assertTrue
import org.junit.rules.RuleChain
diff --git a/benchmark/src/main/java/androidx/benchmark/BenchmarkState.kt b/benchmark/src/main/java/androidx/benchmark/BenchmarkState.kt
index 6a4a34d..66843be7 100644
--- a/benchmark/src/main/java/androidx/benchmark/BenchmarkState.kt
+++ b/benchmark/src/main/java/androidx/benchmark/BenchmarkState.kt
@@ -24,6 +24,7 @@
import androidx.annotation.IntRange
import androidx.annotation.VisibleForTesting
import androidx.test.platform.app.InstrumentationRegistry
+import org.junit.Assert
import java.io.File
import java.text.NumberFormat
import java.util.ArrayList
@@ -288,6 +289,10 @@
internal fun keepRunningInternal(): Boolean {
when (state) {
NOT_STARTED -> {
+ if (Errors.UNSUPPRESSED_WARNING_MESSAGE != null) {
+ Assert.fail(Errors.UNSUPPRESSED_WARNING_MESSAGE)
+ }
+
if (totalRunTimeStartNs == 0L) {
// This is the beginning of the benchmark, we remember it.
totalRunTimeStartNs = System.nanoTime()
@@ -295,7 +300,7 @@
if (performThrottleChecks &&
!CpuInfo.locked &&
!AndroidBenchmarkRunner.sustainedPerformanceModeInUse &&
- !WarningState.isEmulator
+ !Errors.isEmulator
) {
ThrottleDetector.computeThrottleBaseline()
}
@@ -375,7 +380,7 @@
Log.i(TAG, key + summaryLine())
val status = Bundle()
- val prefix = WarningState.WARNING_PREFIX
+ val prefix = Errors.WARNING_PREFIX
status.putLong("${prefix}median", stats.median)
status.putLong("${prefix}mean", stats.mean.toLong())
status.putLong("${prefix}min", stats.min)
@@ -463,7 +468,7 @@
// Report value to Studio console
val bundle = Bundle()
- val fullTestName = WarningState.WARNING_PREFIX +
+ val fullTestName = Errors.WARNING_PREFIX +
if (className.isNotEmpty()) "$className.$testName" else testName
bundle.putIdeSummaryLine(fullTestName, report.stats.min)
InstrumentationRegistry.getInstrumentation().sendStatus(Activity.RESULT_OK, bundle)
@@ -474,7 +479,7 @@
internal fun ideSummaryLineWrapped(key: String, nanos: Long): String {
val warningLines =
- WarningState.acquireWarningStringForLogging()?.split("\n") ?: listOf()
+ Errors.acquireWarningStringForLogging()?.split("\n") ?: listOf()
return (warningLines + ideSummaryLine(key, nanos))
// remove first line if empty
.filterIndexed { index, it -> index != 0 || !it.isEmpty() }
diff --git a/benchmark/src/main/java/androidx/benchmark/WarningState.kt b/benchmark/src/main/java/androidx/benchmark/Errors.kt
similarity index 84%
rename from benchmark/src/main/java/androidx/benchmark/WarningState.kt
rename to benchmark/src/main/java/androidx/benchmark/Errors.kt
index 6b155e2..41c4e90e 100644
--- a/benchmark/src/main/java/androidx/benchmark/WarningState.kt
+++ b/benchmark/src/main/java/androidx/benchmark/Errors.kt
@@ -28,10 +28,24 @@
/**
* Lazy-initialized test-suite global state for warnings around measurement inaccuracy.
*/
-internal object WarningState {
+internal object Errors {
+ /**
+ * Same as trimMargins, but add newlines on either side.
+ */
+ @Suppress("MemberVisibilityCanBePrivate")
+ internal fun String.trimMarginWrapNewlines(): String {
+ return "\n" + trimMargin() + " \n"
+ }
+
+ @Suppress("MemberVisibilityCanBePrivate")
+ internal fun Set<String>.toDisplayString(): String {
+ return toList().sorted().joinToString(" ")
+ }
+
private const val TAG = "Benchmark"
val WARNING_PREFIX: String
+ val UNSUPPRESSED_WARNING_MESSAGE: String?
private var warningString: String? = null
/**
@@ -176,16 +190,35 @@
}
WARNING_PREFIX = warningPrefix
- if (!warningString.isEmpty()) {
+ if (warningString.isNotEmpty()) {
this.warningString = warningString
warningString.split("\n").map { Log.w(TAG, it) }
}
- }
- /**
- * Same as trimMargins, but add newlines on either side.
- */
- private fun String.trimMarginWrapNewlines(): String {
- return "\n" + trimMargin() + " \n"
+ val warningSet = WARNING_PREFIX
+ .split('_')
+ .filter { it.isNotEmpty() }
+ .toSet()
+ val unsuppressedWarningSet = warningSet - Arguments.suppressedErrors
+ UNSUPPRESSED_WARNING_MESSAGE = if (unsuppressedWarningSet.isNotEmpty()) {
+ """
+ |ERRORS (not suppressed): ${unsuppressedWarningSet.toDisplayString()}
+ |(Suppressed errors: ${Arguments.suppressedErrors.toDisplayString()})
+ |$warningString
+ |While you can suppress these errors (turning them into warnings)
+ |PLEASE NOTE THAT EACH SUPPRESSED ERROR COMPROMISES ACCURACY
+ |
+ |// Sample suppression, in a benchmark module's build.gradle:
+ |android {
+ | defaultConfig {
+ | // Enable measuring on an emulator, or devices with low battery
+ | testInstrumentationRunnerArgument
+ | 'androidx.benchmark.suppressErrors', 'EMULATOR,LOW_BATTERY'
+ | }
+ |}
+ """.trimMargin()
+ } else {
+ null
+ }
}
}
\ No newline at end of file
diff --git a/benchmark/src/main/java/androidx/benchmark/ResultWriter.kt b/benchmark/src/main/java/androidx/benchmark/ResultWriter.kt
index 879a053..06dfb50 100644
--- a/benchmark/src/main/java/androidx/benchmark/ResultWriter.kt
+++ b/benchmark/src/main/java/androidx/benchmark/ResultWriter.kt
@@ -31,8 +31,7 @@
fun appendReport(report: BenchmarkState.Report) {
reports.add(report)
- val arguments = InstrumentationRegistry.getArguments()
- if (arguments.getString("androidx.benchmark.output.enable")?.toLowerCase() == "true") {
+ if (Arguments.outputEnable) {
// Currently, we just overwrite the whole file
// Ideally, append for efficiency
val packageName =
