Skip to content

Commit 0004274

Browse files
Refactor timeout handling in AndroidJUnit4ClassRunner.
This change replaces the use of JUnit's FailOnTimeout with a custom implementation that ensures @before, @test, and @after methods run on the same thread. The previous FailOnTimeout would execute only the @test method in a separate thread, potentially causing issues with thread-local state. The new implementation wraps the entire test block (including Before/After) in a separate thread when a timeout is active, preserving thread locality within the test lifecycle. PiperOrigin-RevId: 903547390
1 parent 1144878 commit 0004274

3 files changed

Lines changed: 154 additions & 11 deletions

File tree

runner/android_junit_runner/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
**Bug Fixes**
88

9+
* Ensure @Before and @Test run on the same thread in AndroidJUnit4ClassRunner.
10+
911
**New Features**
1012

1113
* Make perfetto trace sections for tests more identifiable by prefixing with "test:" and using fully qualified class name. (b/204992764)

runner/android_junit_runner/java/androidx/test/internal/runner/junit4/AndroidJUnit4ClassRunner.java

Lines changed: 111 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,26 @@
1616
package androidx.test.internal.runner.junit4;
1717

1818
import static androidx.test.platform.app.InstrumentationRegistry.getArguments;
19+
import static java.util.concurrent.TimeUnit.MILLISECONDS;
1920

2021
import androidx.test.internal.runner.RunnerArgs;
2122
import androidx.test.internal.runner.junit4.statement.RunAfters;
2223
import androidx.test.internal.runner.junit4.statement.RunBefores;
2324
import androidx.test.internal.runner.junit4.statement.UiThreadStatement;
2425
import androidx.test.internal.util.AndroidRunnerParams;
2526
import java.util.List;
27+
import java.util.concurrent.CountDownLatch;
28+
import java.util.concurrent.atomic.AtomicReference;
2629
import org.junit.After;
2730
import org.junit.Before;
2831
import org.junit.Test;
29-
import org.junit.internal.runners.statements.FailOnTimeout;
32+
import org.junit.internal.runners.model.ReflectiveCallable;
33+
import org.junit.internal.runners.statements.Fail;
3034
import org.junit.runners.BlockJUnit4ClassRunner;
3135
import org.junit.runners.model.FrameworkMethod;
3236
import org.junit.runners.model.InitializationError;
3337
import org.junit.runners.model.Statement;
38+
import org.junit.runners.model.TestTimedOutException;
3439

3540
/** A specialized {@link BlockJUnit4ClassRunner} that can handle timeouts */
3641
public class AndroidJUnit4ClassRunner extends BlockJUnit4ClassRunner {
@@ -55,13 +60,35 @@ public AndroidJUnit4ClassRunner(Class<?> klass) throws InitializationError {
5560
this(klass, RunnerArgs.parseTestTimeout(getArguments()));
5661
}
5762

63+
private static final ThreadLocal<CountDownLatch> currentTestStartedLatch = new ThreadLocal<>();
64+
private static final ThreadLocal<CountDownLatch> currentTestFinishedLatch = new ThreadLocal<>();
65+
5866
/** Returns a {@link Statement} that invokes {@code method} on {@code test} */
5967
@Override
6068
protected Statement methodInvoker(FrameworkMethod method, Object test) {
69+
final Statement invoker;
6170
if (UiThreadStatement.shouldRunOnUiThread(method)) {
62-
return new UiThreadStatement(super.methodInvoker(method, test), true);
71+
invoker = new UiThreadStatement(super.methodInvoker(method, test), true);
72+
} else {
73+
invoker = super.methodInvoker(method, test);
6374
}
64-
return super.methodInvoker(method, test);
75+
return new Statement() {
76+
@Override
77+
public void evaluate() throws Throwable {
78+
CountDownLatch startLatch = currentTestStartedLatch.get();
79+
if (startLatch != null) {
80+
startLatch.countDown();
81+
}
82+
try {
83+
invoker.evaluate();
84+
} finally {
85+
CountDownLatch finishLatch = currentTestFinishedLatch.get();
86+
if (finishLatch != null) {
87+
finishLatch.countDown();
88+
}
89+
}
90+
}
91+
};
6592
}
6693

6794
@Override
@@ -76,28 +103,101 @@ protected Statement withAfters(FrameworkMethod method, Object target, Statement
76103
return afters.isEmpty() ? statement : new RunAfters(method, statement, afters, target);
77104
}
78105

106+
@Override
107+
protected Statement methodBlock(FrameworkMethod method) {
108+
Object test;
109+
try {
110+
test =
111+
new ReflectiveCallable() {
112+
@Override
113+
protected Object runReflectiveCall() throws Throwable {
114+
return createTest();
115+
}
116+
}.run();
117+
} catch (Throwable e) {
118+
return new Fail(e);
119+
}
120+
121+
Statement statement = methodInvoker(method, test);
122+
statement = possiblyExpectingExceptions(method, test, statement);
123+
statement = withBefores(method, test, statement);
124+
statement = withAfters(method, test, statement);
125+
statement = withPotentialTimeout(method, test, statement);
126+
try {
127+
java.lang.reflect.Method withRulesMethod =
128+
BlockJUnit4ClassRunner.class.getDeclaredMethod(
129+
"withRules", FrameworkMethod.class, Object.class, Statement.class);
130+
withRulesMethod.setAccessible(true);
131+
statement = (Statement) withRulesMethod.invoke(this, method, test, statement);
132+
} catch (Exception e) {
133+
throw new RuntimeException(e);
134+
}
135+
return statement;
136+
}
137+
79138
/**
80139
* Default to {@link org.junit.Test#timeout()} level timeout if set. Otherwise, set the timeout
81140
* that was passed to the instrumentation via argument.
82141
*/
83142
@Override
84143
protected Statement withPotentialTimeout(FrameworkMethod method, Object test, Statement next) {
85-
// test level timeout i.e @Test(timeout = 123)
86144
long timeout = getTimeout(method.getAnnotation(Test.class));
87-
88-
// use runner arg timeout if test level timeout is not present
89145
if (timeout <= 0 && perTestTimeout > 0) {
90146
timeout = perTestTimeout;
91147
}
148+
final long finalTimeout = timeout;
92149

93-
if (timeout <= 0) {
94-
// no timeout was set
150+
if (finalTimeout <= 0 || UiThreadStatement.shouldRunOnUiThread(method)) {
95151
return next;
96152
}
97153

98-
// Cannot switch to use builder as that is not supported in JUnit 4.10 which is what is
99-
// available in AOSP.
100-
return new FailOnTimeout(next, timeout);
154+
return new Statement() {
155+
@Override
156+
@SuppressWarnings("Interruption") // We want to interrupt the thread to stop the test.
157+
public void evaluate() throws Throwable {
158+
final AtomicReference<Throwable> failure = new AtomicReference<>();
159+
final CountDownLatch testStartedLatch = new CountDownLatch(1);
160+
final CountDownLatch testFinishedLatch = new CountDownLatch(1);
161+
final CountDownLatch doneLatch = new CountDownLatch(1);
162+
163+
Thread thread =
164+
new Thread(
165+
new Runnable() {
166+
@Override
167+
public void run() {
168+
currentTestStartedLatch.set(testStartedLatch);
169+
currentTestFinishedLatch.set(testFinishedLatch);
170+
try {
171+
next.evaluate();
172+
} catch (Throwable t) {
173+
failure.set(t);
174+
} finally {
175+
testStartedLatch.countDown();
176+
testFinishedLatch.countDown();
177+
doneLatch.countDown();
178+
currentTestStartedLatch.remove();
179+
currentTestFinishedLatch.remove();
180+
}
181+
}
182+
},
183+
"Time-limited test");
184+
thread.setDaemon(true);
185+
thread.start();
186+
187+
testStartedLatch.await();
188+
boolean finishedInTime = testFinishedLatch.await(finalTimeout, MILLISECONDS);
189+
190+
if (!finishedInTime) {
191+
thread.interrupt();
192+
throw new TestTimedOutException(finalTimeout, MILLISECONDS);
193+
}
194+
195+
doneLatch.await();
196+
if (failure.get() != null) {
197+
throw failure.get();
198+
}
199+
}
200+
};
101201
}
102202

103203
private long getTimeout(Test annotation) {

runner/android_junit_runner/javatests/androidx/test/internal/runner/junit4/AndroidAnnotatedBuilderTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,17 @@
2626
import java.lang.reflect.InvocationTargetException;
2727
import java.util.Arrays;
2828
import java.util.Collection;
29+
import org.junit.After;
2930
import org.junit.Assert;
3031
import org.junit.Before;
3132
import org.junit.Test;
33+
import org.junit.runner.JUnitCore;
34+
import org.junit.runner.Result;
3235
import org.junit.runner.RunWith;
3336
import org.junit.runner.Runner;
3437
import org.junit.runners.JUnit4;
3538
import org.junit.runners.Parameterized;
39+
import org.junit.runners.model.InitializationError;
3640
import org.junit.runners.model.RunnerBuilder;
3741
import org.mockito.Mock;
3842
import org.mockito.MockitoAnnotations;
@@ -133,4 +137,41 @@ public Runner buildAndroidRunner(Class<? extends Runner> runnerClass, Class<?> t
133137
// attempt to create a runner for a class with no @RunWith annotation
134138
ab.runnerForClass(NoRunWithClass.class);
135139
}
140+
141+
@SuppressWarnings("NonFinalStaticField") // Static fields are needed to check thread assignment.
142+
public static class TimeoutTestClass {
143+
static Thread beforeThread;
144+
static Thread testThread;
145+
static Thread afterThread;
146+
147+
@Before
148+
public void before() {
149+
beforeThread = Thread.currentThread();
150+
}
151+
152+
@Test(timeout = 5000)
153+
public void testWithTimeout() {
154+
testThread = Thread.currentThread();
155+
}
156+
157+
@After
158+
public void after() {
159+
afterThread = Thread.currentThread();
160+
}
161+
}
162+
163+
@Test
164+
public void testThreadsSameWithTimeout() throws InitializationError {
165+
TimeoutTestClass.beforeThread = null;
166+
TimeoutTestClass.testThread = null;
167+
TimeoutTestClass.afterThread = null;
168+
169+
AndroidJUnit4ClassRunner runner = new AndroidJUnit4ClassRunner(TimeoutTestClass.class, 0);
170+
Result result = new JUnitCore().run(runner);
171+
172+
assertEquals(0, result.getFailureCount());
173+
Assert.assertNotNull(TimeoutTestClass.beforeThread);
174+
assertEquals(TimeoutTestClass.beforeThread, TimeoutTestClass.testThread);
175+
assertEquals(TimeoutTestClass.testThread, TimeoutTestClass.afterThread);
176+
}
136177
}

0 commit comments

Comments
 (0)