From 43730b3f0fed7a4d62e09ec24a2f7d7f837aad52 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 9 Oct 2024 16:24:44 +0900 Subject: [PATCH] Add connectivity diagnostics on test failure When CtsNetTestCases tests fail, or when a tryTest statement fails, collect connectivity diagnostics and output them as a test artifact. tryTest must be followed by cleanup blocks, which often change the configuration, so having diagnostics when the tryTest fails in addition to after the test failure is useful. The collected diagnostics will be available as individual test artifacts. They should make it easier to investigate test failures, and could be collected across test runs in an automated way. Output files are currently limited at 20 per run, to avoid collecting too much data in case everything is failing in the run. Test: atest Change-Id: Iaa48e519028dc61c03d387beeade7d41fca2ef48 --- staticlibs/testutils/Android.bp | 1 + .../ConnectivityDiagnosticsCollector.kt | 137 ++++++++++++++++++ .../com/android/testutils/Cleanup.kt | 13 ++ tests/cts/net/AndroidTestTemplate.xml | 6 + 4 files changed, 157 insertions(+) create mode 100644 staticlibs/testutils/devicetests/com/android/testutils/ConnectivityDiagnosticsCollector.kt diff --git a/staticlibs/testutils/Android.bp b/staticlibs/testutils/Android.bp index 8c71a91ea8..13e1dc06f9 100644 --- a/staticlibs/testutils/Android.bp +++ b/staticlibs/testutils/Android.bp @@ -33,6 +33,7 @@ java_library { ], static_libs: [ "androidx.test.ext.junit", + "collector-device-lib", "kotlin-reflect", "libnanohttpd", "net-tests-utils-host-device-common", diff --git a/staticlibs/testutils/devicetests/com/android/testutils/ConnectivityDiagnosticsCollector.kt b/staticlibs/testutils/devicetests/com/android/testutils/ConnectivityDiagnosticsCollector.kt new file mode 100644 index 0000000000..0cb5c340e8 --- /dev/null +++ b/staticlibs/testutils/devicetests/com/android/testutils/ConnectivityDiagnosticsCollector.kt @@ -0,0 +1,137 @@ +/* + * Copyright (C) 2024 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 com.android.testutils + +import android.device.collectors.BaseMetricListener +import android.device.collectors.DataRecord +import android.os.ParcelFileDescriptor +import android.util.Log +import androidx.test.platform.app.InstrumentationRegistry +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.PrintWriter +import java.time.ZonedDateTime +import kotlin.test.assertNull +import org.junit.runner.Description +import org.junit.runner.notification.Failure + +class ConnectivityDiagnosticsCollector : BaseMetricListener() { + companion object { + private const val COLLECTOR_DIR = "run_listeners/connectivity_diagnostics" + private const val FILENAME_SUFFIX = "_conndiag.txt" + private const val MAX_DUMPS = 20 + + private val TAG = ConnectivityDiagnosticsCollector::class.simpleName + var instance: ConnectivityDiagnosticsCollector? = null + } + + private val buffer = ByteArrayOutputStream() + private val collectorDir: File by lazy { + createAndEmptyDirectory(COLLECTOR_DIR) + } + private val outputFiles = mutableSetOf() + + override fun onSetUp() { + assertNull(instance, "ConnectivityDiagnosticsCollectors were set up multiple times") + instance = this + TryTestConfig.setDiagnosticsCollector { throwable -> + collectTestFailureDiagnostics(throwable) + } + } + + override fun onCleanUp() { + instance = null + } + + override fun onTestFail(testData: DataRecord, description: Description, failure: Failure) { + // TODO: find a way to disable this behavior only on local runs, to avoid slowing them down + // when iterating on failing tests. + if (outputFiles.size >= MAX_DUMPS) return + collectTestFailureDiagnostics(failure.exception) + + val baseFilename = "${description.className}#${description.methodName}_failure" + flushBufferToFileMetric(testData, baseFilename) + } + + override fun onTestEnd(testData: DataRecord, description: Description) { + // Tests may call methods like collectDumpsysConnectivity to collect diagnostics at any time + // during the run, for example to observe state at various points to investigate a flake + // and compare passing/failing cases. + // Flush the contents of the buffer to a file when the test ends, even when successful. + if (buffer.size() == 0) return + if (outputFiles.size >= MAX_DUMPS) return + + // Flush any data that the test added to the buffer for dumping + val baseFilename = "${description.className}#${description.methodName}_testdump" + flushBufferToFileMetric(testData, baseFilename) + } + + private fun flushBufferToFileMetric(testData: DataRecord, baseFilename: String) { + var filename = baseFilename + // In case a method was run multiple times (typically retries), append a number + var i = 2 + while (outputFiles.contains(filename)) { + filename = baseFilename + "_$i" + i++ + } + val outFile = File(collectorDir, filename + FILENAME_SUFFIX) + outputFiles.add(filename) + outFile.writeBytes(buffer.toByteArray()) + buffer.reset() + val fileKey = "${ConnectivityDiagnosticsCollector::class.qualifiedName}_$filename" + testData.addFileMetric(fileKey, outFile) + } + + /** + * Add connectivity diagnostics to the test data dump. + * + *

This collects a set of diagnostics that are relevant to connectivity test failures. + *

The dump will be collected immediately, and exported to a test artifact file when the + * test ends. + * @param exceptionContext An exception to write a stacktrace to the dump for context. + */ + fun collectTestFailureDiagnostics(exceptionContext: Throwable? = null) { + collectDumpsysConnectivity(exceptionContext) + } + + /** + * Add dumpsys connectivity to the test data dump. + * + *

The dump will be collected immediately, and exported to a test artifact file when the + * test ends. + * @param exceptionContext An exception to write a stacktrace to the dump for context. + */ + fun collectDumpsysConnectivity(exceptionContext: Throwable? = null) { + Log.i(TAG, "Collecting dumpsys connectivity for test artifacts") + PrintWriter(buffer).let { + it.println("--- Dumpsys connectivity at ${ZonedDateTime.now()} ---") + maybeWriteExceptionContext(it, exceptionContext) + it.flush() + } + ParcelFileDescriptor.AutoCloseInputStream( + InstrumentationRegistry.getInstrumentation().uiAutomation.executeShellCommand( + "dumpsys connectivity --dump-priority HIGH")).use { + it.copyTo(buffer) + } + } + + private fun maybeWriteExceptionContext(writer: PrintWriter, exceptionContext: Throwable?) { + if (exceptionContext == null) return + writer.println("At: ") + exceptionContext.printStackTrace(writer) + } +} \ No newline at end of file diff --git a/staticlibs/testutils/hostdevice/com/android/testutils/Cleanup.kt b/staticlibs/testutils/hostdevice/com/android/testutils/Cleanup.kt index 9f282347f5..dcd422cfbb 100644 --- a/staticlibs/testutils/hostdevice/com/android/testutils/Cleanup.kt +++ b/staticlibs/testutils/hostdevice/com/android/testutils/Cleanup.kt @@ -20,6 +20,7 @@ package com.android.testutils import com.android.testutils.FunctionalUtils.ThrowingRunnable import com.android.testutils.FunctionalUtils.ThrowingSupplier +import java.util.function.Consumer import javax.annotation.CheckReturnValue /** @@ -73,11 +74,23 @@ import javax.annotation.CheckReturnValue * }); */ +object TryTestConfig { + internal var diagnosticsCollector: Consumer? = null + + /** + * Set the diagnostics collector to be used in case of failure in [tryTest]. + */ + fun setDiagnosticsCollector(collector: Consumer) { + diagnosticsCollector = collector + } +} + @CheckReturnValue fun tryTest(block: () -> T) = TryExpr( try { Result.success(block()) } catch (e: Throwable) { + TryTestConfig.diagnosticsCollector?.accept(e) Result.failure(e) }) diff --git a/tests/cts/net/AndroidTestTemplate.xml b/tests/cts/net/AndroidTestTemplate.xml index 24431a628e..a65316f54b 100644 --- a/tests/cts/net/AndroidTestTemplate.xml +++ b/tests/cts/net/AndroidTestTemplate.xml @@ -56,7 +56,13 @@ the runner will only run the tests annotated with that annotation, but if it does not, the runner will run all the tests. -->