Skip to content

Commit bb9e487

Browse files
Hardening AndroidJUnitRunner against exceptions that can be thrown when there are problems in the underlying test libraries.
PiperOrigin-RevId: 665051629
1 parent dd2b59a commit bb9e487

File tree

4 files changed

+41
-23
lines changed

4 files changed

+41
-23
lines changed

runner/android_junit_runner/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
**Bug Fixes**
88

99
* Exceptions during `@AfterClass` were not being reported via `InstrumentationResultPrinter`.
10+
* Exceptions arising in AndroidJUnitRunner.buildRequest are now handled.
1011

1112
**New Features**
1213

runner/android_junit_runner/java/androidx/test/runner/AndroidJUnitRunner.java

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,21 @@ InstrumentationResultPrinter getInstrumentationResultPrinter() {
426426
return instrumentationResultPrinter;
427427
}
428428

429+
private void invokeRemoteMethod() {
430+
try {
431+
new ReflectiveMethod<Void>(
432+
runnerArgs.remoteMethod.testClassName, runnerArgs.remoteMethod.methodName)
433+
.invokeStatic();
434+
} catch (ReflectionException e) {
435+
Log.e(
436+
LOG_TAG,
437+
String.format(
438+
"Reflective call to remote method %s#%s failed",
439+
runnerArgs.remoteMethod.testClassName, runnerArgs.remoteMethod.methodName),
440+
e);
441+
}
442+
}
443+
429444
@Override
430445
public void onStart() {
431446
Log.d(LOG_TAG, "onStart is called.");
@@ -434,21 +449,11 @@ public void onStart() {
434449
try {
435450
setJsBridgeClassName("androidx.test.espresso.web.bridge.JavaScriptBridge");
436451
super.onStart();
452+
437453
Request testRequest = buildRequest(runnerArgs, getArguments());
438454

439455
if (runnerArgs.remoteMethod != null) {
440-
try {
441-
new ReflectiveMethod<Void>(
442-
runnerArgs.remoteMethod.testClassName, runnerArgs.remoteMethod.methodName)
443-
.invokeStatic();
444-
} catch (ReflectionException e) {
445-
Log.e(
446-
LOG_TAG,
447-
String.format(
448-
"Reflective call to remote method %s#%s failed",
449-
runnerArgs.remoteMethod.testClassName, runnerArgs.remoteMethod.methodName),
450-
e);
451-
}
456+
invokeRemoteMethod();
452457
}
453458

454459
// TODO(b/162075422): using deprecated isPrimaryInstrProcess(argsProcessName) method
@@ -457,16 +462,13 @@ public void onStart() {
457462
return;
458463
}
459464

460-
try {
461-
TestExecutor.Builder executorBuilder = new TestExecutor.Builder(this);
462-
addListeners(runnerArgs, executorBuilder);
463-
results = executorBuilder.build().execute(testRequest);
464-
} catch (Throwable t) {
465-
final String msg = "Fatal exception when running tests";
466-
Log.e(LOG_TAG, msg, t);
467-
onException(this, t);
468-
}
469-
465+
TestExecutor.Builder executorBuilder = new TestExecutor.Builder(this);
466+
addListeners(runnerArgs, executorBuilder);
467+
results = executorBuilder.build().execute(testRequest);
468+
} catch (Throwable t) {
469+
final String msg = "Fatal exception when running tests";
470+
Log.e(LOG_TAG, msg, t);
471+
onException(this, t);
470472
} finally {
471473
Trace.endSection();
472474
}
@@ -623,6 +625,13 @@ public boolean onException(Object obj, Throwable e) {
623625
final StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskWrites();
624626
try {
625627
instResultPrinter.reportProcessCrash(e);
628+
} catch (Throwable t) {
629+
// It is possible for the test infrastructure to get sufficiently messed up that even this
630+
// can fail (with a NoSuchMethodError from Failure.getTrace()!). Don't let that get in the
631+
// way of sending events reporting the problems, since that can make the difference between
632+
// "test failed with a useful error message" and "test mysteriously timed out and the
633+
// developer has to go spelunking in the system logcat".
634+
Log.e(LOG_TAG, "Failed to report process crash.", t);
626635
} finally {
627636
StrictMode.setThreadPolicy(oldPolicy);
628637
}

services/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
**Bug Fixes**
88

99
* TestStorage: Use input directory location for internal files
10+
* StackTrimmer: harden against exceptions coming from Failure.getMessage().
1011

1112
**New Features**
1213

services/events/java/androidx/test/services/events/internal/StackTrimmer.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,14 @@ public static String getTrimmedStackTrace(Failure failure) {
4747
}
4848

4949
public static String getTrimmedMessage(Failure failure) {
50-
String message = failure.getMessage();
50+
String message = null;
51+
try {
52+
// Even this can fail! If so, don't let it wreck error reporting.
53+
message = failure.getMessage();
54+
} catch (Throwable t) {
55+
Log.e(TAG, "Failed to get message from " + failure, t);
56+
message = "Couldn't get message due to " + t.getMessage();
57+
}
5158
if (message != null && message.length() > MAX_TRACE_SIZE) {
5259
// Since AJUR needs to report failures back to AM via a binder IPC, we need to make sure that
5360
// we don't exceed the Binder transaction limit - which is 1MB per process.

0 commit comments

Comments
 (0)