From 2c9f475d0ddd41b2a2d212e96b12f94c1a6c232a Mon Sep 17 00:00:00 2001 From: AndroidX Test Team Date: Mon, 19 Aug 2024 11:50:07 -0700 Subject: [PATCH] Hardening AndroidJUnitRunner against exceptions that can be thrown when there are problems in the underlying test libraries. PiperOrigin-RevId: 664908801 --- runner/android_junit_runner/CHANGELOG.md | 1 + .../androidx/test/runner/AndroidJUnitRunner.java | 13 ++++++++++++- services/CHANGELOG.md | 1 + .../test/services/events/internal/StackTrimmer.java | 9 ++++++++- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/runner/android_junit_runner/CHANGELOG.md b/runner/android_junit_runner/CHANGELOG.md index e5344973d..071a4657e 100644 --- a/runner/android_junit_runner/CHANGELOG.md +++ b/runner/android_junit_runner/CHANGELOG.md @@ -7,6 +7,7 @@ **Bug Fixes** * Exceptions during `@AfterClass` were not being reported via `InstrumentationResultPrinter`. +* Exceptions arising in AndroidJUnitRunner.buildRequest are now handled. **New Features** diff --git a/runner/android_junit_runner/java/androidx/test/runner/AndroidJUnitRunner.java b/runner/android_junit_runner/java/androidx/test/runner/AndroidJUnitRunner.java index 2bb05d230..1648bdd11 100644 --- a/runner/android_junit_runner/java/androidx/test/runner/AndroidJUnitRunner.java +++ b/runner/android_junit_runner/java/androidx/test/runner/AndroidJUnitRunner.java @@ -434,7 +434,12 @@ public void onStart() { try { setJsBridgeClassName("androidx.test.espresso.web.bridge.JavaScriptBridge"); super.onStart(); - Request testRequest = buildRequest(runnerArgs, getArguments()); + Request testRequest = null; + try { + testRequest = buildRequest(runnerArgs, getArguments()); + } catch (Throwable t) { + onException(this, t); + } if (runnerArgs.remoteMethod != null) { try { @@ -623,6 +628,12 @@ public boolean onException(Object obj, Throwable e) { final StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskWrites(); try { instResultPrinter.reportProcessCrash(e); + } catch (Throwable t) { + // It is possible for the test infrastructure to get sufficiently messed up that even this + // can fail. Don't let that get in the way of sending events reporting the problems, since + // that can make the difference between "test failed with a useful error message" and "test + // mysteriously timed out and the developer has to go spelunking in the system logcat". + Log.e(LOG_TAG, "Failed to report process crash.", t); } finally { StrictMode.setThreadPolicy(oldPolicy); } diff --git a/services/CHANGELOG.md b/services/CHANGELOG.md index 2b4bdc296..9d62f882d 100644 --- a/services/CHANGELOG.md +++ b/services/CHANGELOG.md @@ -7,6 +7,7 @@ **Bug Fixes** * TestStorage: Use input directory location for internal files +* StackTrimmer: harden against exceptions coming from Failure.getMessage(). **New Features** diff --git a/services/events/java/androidx/test/services/events/internal/StackTrimmer.java b/services/events/java/androidx/test/services/events/internal/StackTrimmer.java index 906edae39..8e0b30cf0 100644 --- a/services/events/java/androidx/test/services/events/internal/StackTrimmer.java +++ b/services/events/java/androidx/test/services/events/internal/StackTrimmer.java @@ -47,7 +47,14 @@ public static String getTrimmedStackTrace(Failure failure) { } public static String getTrimmedMessage(Failure failure) { - String message = failure.getMessage(); + String message = null; + try { + // Even this can fail! If so, don't let it wreck error reporting. + message = failure.getMessage(); + } catch (Throwable t) { + Log.e(TAG, "Failed to get message from " + failure.toString(), t); + message = "Couldn't get message due to " + t.getMessage(); + } if (message != null && message.length() > MAX_TRACE_SIZE) { // Since AJUR needs to report failures back to AM via a binder IPC, we need to make sure that // we don't exceed the Binder transaction limit - which is 1MB per process.