Improve Exception handling in SandboxTestRunner

Don't let exceptions from `afterTest` and `finallyAfterTest` hide exceptions from the test and its setup.

This came up when a RuntimeException that was being thrown in a `@BeforeClass` block
was being hidden. Add a test case for this as well.

PiperOrigin-RevId: 627115792

2 files changed, 44 insertions, 8 deletions

diff --git a/junit/src/main/java/org/robolectric/internal/SandboxTestRunner.java b/junit/src/main/java/org/robolectric/internal/SandboxTestRunner.java
index 414b4108a..2821ff8bf 100644
--- a/junit/src/main/java/org/robolectric/internal/SandboxTestRunner.java
+++ b/junit/src/main/java/org/robolectric/internal/SandboxTestRunner.java
@@ -5,6 +5,7 @@ import static java.util.Arrays.stream;
 
 import com.google.common.base.Splitter;
 import java.lang.reflect.Method;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -12,6 +13,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Queue;
 import javax.annotation.Nonnull;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -274,6 +276,8 @@ public class SandboxTestRunner extends BlockJUnit4ClassRunner {
                 throw new RuntimeException(e);
               }
 
+              Queue<Throwable> thrown = new ArrayDeque<>();
+
               try {
                 // Only invoke @BeforeClass once per class
                 invokeBeforeClass(bootstrappedTestClass, sandbox);
@@ -284,20 +288,32 @@ public class SandboxTestRunner extends BlockJUnit4ClassRunner {
 
                 Statement statement =
                     helperTestRunner.methodBlock(new FrameworkMethod(bootstrappedMethod));
+                statement.evaluate();
+              } catch (Throwable throwable) {
+                thrown.add(throwable);
+              }
 
-                // todo: this try/finally probably isn't right -- should mimic RunAfters? [xw]
-                try {
-                  statement.evaluate();
-                } finally {
-                  afterTest(method, bootstrappedMethod);
-                }
+              try {
+                afterTest(method, bootstrappedMethod);
               } catch (Throwable throwable) {
-                throw Util.sneakyThrow(throwable);
-              } finally {
+                thrown.add(throwable);
+              }
+
+              try {
                 Thread.currentThread().setContextClassLoader(priorContextClassLoader);
                 finallyAfterTest(method);
                 reportPerfStats(perfStatsCollector);
                 perfStatsCollector.reset();
+              } catch (Throwable throwable) {
+                thrown.add(throwable);
+              }
+
+              Throwable first = thrown.poll();
+              if (first != null) {
+                while (!thrown.isEmpty()) {
+                  first.addSuppressed(thrown.remove());
+                }
+                throw Util.sneakyThrow(first);
               }
             });
       }
diff --git a/robolectric/src/test/java/org/robolectric/RobolectricTestRunnerTest.java b/robolectric/src/test/java/org/robolectric/RobolectricTestRunnerTest.java
index dfdb9d495..8bb56391a 100644
--- a/robolectric/src/test/java/org/robolectric/RobolectricTestRunnerTest.java
+++ b/robolectric/src/test/java/org/robolectric/RobolectricTestRunnerTest.java
@@ -31,6 +31,7 @@ import javax.inject.Named;
 import org.junit.After;
 import org.junit.AssumptionViolatedException;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.FixMethodOrder;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -513,4 +514,23 @@ public class RobolectricTestRunnerTest {
       events.add("failure: " + message);
     }
   }
+
+  @Test
+  public void shouldReportExceptionsInBeforeClass() throws Exception {
+    RobolectricTestRunner runner =
+        new SingleSdkRobolectricTestRunner(TestWithBeforeClassThatThrowsRuntimeException.class);
+    runner.run(notifier);
+    assertThat(events.get(1)).startsWith("failure: fail");
+  }
+
+  @Ignore
+  public static class TestWithBeforeClassThatThrowsRuntimeException {
+    @BeforeClass
+    public static void beforeClass() {
+      throw new RuntimeException("fail");
+    }
+
+    @Test
+    public void test() {}
+  }
 }