Skip to content

Commit 2db6394

Browse files
stefanbirknerstefanbirkner
committed
Tidy up FailOnTimeoutTest
Improve readability and reusability. When I started to read the test it took me some time to understand it. The reason was the configurable statement TestStatement and the evaluateWith... methods. The refactored test uses fixed Statements when possible and uses smaller methods for wrapping FailOnTimeout with a ThrowingRunnable and for wrapping an arbitrary statement with a FailOnTimeout. It also calls FailOnTimeout#evaluate directly when possible.
1 parent 64634e1 commit 2db6394

File tree

1 file changed

+135
-132
lines changed

1 file changed

+135
-132
lines changed

‎src/test/java/org/junit/internal/runners/statements/FailOnTimeoutTest.java

+135-132
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,24 @@
88
import static java.util.concurrent.TimeUnit.MILLISECONDS;
99
import static org.junit.Assert.assertEquals;
1010
import static org.junit.Assert.assertFalse;
11+
import static org.junit.Assert.assertNotNull;
1112
import static org.junit.Assert.assertNotSame;
1213
import static org.junit.Assert.assertSame;
1314
import static org.junit.Assert.assertThrows;
1415
import static org.junit.Assert.assertTrue;
15-
import static org.junit.Assert.fail;
1616
import static org.junit.Assume.assumeFalse;
1717
import static org.junit.Assume.assumeTrue;
1818

1919
import java.util.Arrays;
20-
import java.util.concurrent.TimeUnit;
20+
import java.util.concurrent.atomic.AtomicBoolean;
2121
import java.util.concurrent.atomic.AtomicReference;
2222

2323
import org.junit.Test;
2424
import org.junit.function.ThrowingRunnable;
25-
import org.junit.internal.runners.statements.Fail;
2625
import org.junit.runner.RunWith;
2726
import org.junit.runners.Parameterized;
27+
import org.junit.runners.Parameterized.Parameter;
28+
import org.junit.runners.Parameterized.Parameters;
2829
import org.junit.runners.model.Statement;
2930
import org.junit.runners.model.TestTimedOutException;
3031

@@ -34,176 +35,129 @@
3435
*/
3536
@RunWith(Parameterized.class)
3637
public class FailOnTimeoutTest {
37-
private static final long TIMEOUT = 100;
38-
private static final long DURATION_THAT_EXCEEDS_TIMEOUT = 60 * 60 * 1000; //1 hour
3938

40-
private final TestStatement statement = new TestStatement();
41-
42-
private final boolean lookingForStuckThread;
43-
private final FailOnTimeout failOnTimeout;
44-
45-
@Parameterized.Parameters(name = "lookingForStuckThread = {0}")
39+
@Parameters(name = "lookingForStuckThread = {0}")
4640
public static Iterable<Boolean> getParameters() {
4741
return Arrays.asList(Boolean.TRUE, Boolean.FALSE);
4842
}
4943

50-
public FailOnTimeoutTest(Boolean lookingForStuckThread) {
51-
this.lookingForStuckThread = lookingForStuckThread;
52-
this.failOnTimeout = builder().withTimeout(TIMEOUT, MILLISECONDS).build(statement);
53-
}
44+
@Parameter
45+
public boolean lookingForStuckThread;
46+
47+
@Test
48+
public void noExceptionIsThrownWhenWrappedStatementFinishesBeforeTimeoutWithoutThrowingException()
49+
throws Throwable {
50+
FailOnTimeout failOnTimeout = failAfter50Ms(new FastStatement());
5451

55-
private FailOnTimeout.Builder builder() {
56-
return FailOnTimeout.builder().withLookingForStuckThread(lookingForStuckThread);
52+
failOnTimeout.evaluate();
53+
54+
// test is successful when no exception is thrown
5755
}
5856

5957
@Test
6058
public void throwsTestTimedOutException() {
6159
assertThrows(
6260
TestTimedOutException.class,
63-
evaluateWithWaitDuration(DURATION_THAT_EXCEEDS_TIMEOUT));
61+
run(failAfter50Ms(new InfiniteLoop())));
6462
}
6563

6664
@Test
6765
public void throwExceptionWithNiceMessageOnTimeout() {
68-
TestTimedOutException e = assertThrows(
69-
TestTimedOutException.class,
70-
evaluateWithWaitDuration(DURATION_THAT_EXCEEDS_TIMEOUT));
71-
assertEquals("test timed out after 100 milliseconds", e.getMessage());
66+
Exception e = assertThrows(
67+
Exception.class,
68+
run(failAfter50Ms(new InfiniteLoop())));
69+
assertEquals("test timed out after 50 milliseconds", e.getMessage());
7270
}
7371

7472
@Test
7573
public void sendUpExceptionThrownByStatement() {
76-
RuntimeException exception = new RuntimeException();
77-
RuntimeException e = assertThrows(
78-
RuntimeException.class,
79-
evaluateWithException(exception));
74+
Exception exception = new RuntimeException();
75+
Exception e = assertThrows(
76+
Exception.class,
77+
run(failAfter50Ms(new Fail(exception))));
8078
assertSame(exception, e);
8179
}
8280

8381
@Test
8482
public void throwExceptionIfTheSecondCallToEvaluateNeedsTooMuchTime()
8583
throws Throwable {
86-
evaluateWithWaitDuration(0).run();
84+
DelegateStatement statement = new DelegateStatement();
85+
FailOnTimeout failOnTimeout = failAfter50Ms(statement);
86+
87+
statement.delegate = new FastStatement();
88+
failOnTimeout.evaluate();
89+
90+
statement.delegate = new InfiniteLoop();
8791
assertThrows(
8892
TestTimedOutException.class,
89-
evaluateWithWaitDuration(DURATION_THAT_EXCEEDS_TIMEOUT));
93+
run(failOnTimeout));
9094
}
9195

9296
@Test
9397
public void throwTimeoutExceptionOnSecondCallAlthoughFirstCallThrowsException() {
94-
try {
95-
evaluateWithException(new RuntimeException()).run();
96-
} catch (Throwable expected) {
97-
}
98+
DelegateStatement statement = new DelegateStatement();
99+
FailOnTimeout failOnTimeout = failAfter50Ms(statement);
98100

99-
TestTimedOutException e = assertThrows(
101+
statement.delegate = new Fail(new AssertionError("first execution failed"));
102+
assertThrows(
103+
AssertionError.class,
104+
run(failOnTimeout)
105+
);
106+
107+
statement.delegate = new InfiniteLoop();
108+
assertThrows(
100109
TestTimedOutException.class,
101-
evaluateWithWaitDuration(DURATION_THAT_EXCEEDS_TIMEOUT));
102-
assertEquals("test timed out after 100 milliseconds", e.getMessage());
110+
run(failOnTimeout));
103111
}
104112

105113
@Test
106114
public void throwsExceptionWithTimeoutValueAndTimeUnitSet() {
107115
TestTimedOutException e = assertThrows(
108116
TestTimedOutException.class,
109-
evaluateWithWaitDuration(DURATION_THAT_EXCEEDS_TIMEOUT));
110-
assertEquals(TIMEOUT, e.getTimeout());
111-
assertEquals(TimeUnit.MILLISECONDS, e.getTimeUnit());
112-
}
113-
114-
private ThrowingRunnable evaluateWithDelegate(final Statement delegate) {
115-
return new ThrowingRunnable() {
116-
public void run() throws Throwable {
117-
statement.nextStatement = delegate;
118-
statement.waitDuration = 0;
119-
failOnTimeout.evaluate();
120-
}
121-
};
122-
}
123-
124-
private ThrowingRunnable evaluateWithException(Exception exception) {
125-
return evaluateWithDelegate(new Fail(exception));
126-
}
127-
128-
private ThrowingRunnable evaluateWithWaitDuration(final long waitDuration) {
129-
return new ThrowingRunnable() {
130-
public void run() throws Throwable {
131-
statement.nextStatement = null;
132-
statement.waitDuration = waitDuration;
133-
failOnTimeout.evaluate();
134-
}
135-
};
136-
}
137-
138-
private static final class TestStatement extends Statement {
139-
long waitDuration;
140-
141-
Statement nextStatement;
142-
143-
@Override
144-
public void evaluate() throws Throwable {
145-
sleep(waitDuration);
146-
if (nextStatement != null) {
147-
nextStatement.evaluate();
148-
}
149-
}
117+
run(failAfter50Ms(new InfiniteLoop())));
118+
assertEquals(50, e.getTimeout());
119+
assertEquals(MILLISECONDS, e.getTimeUnit());
150120
}
151121

152122
@Test
153123
public void stopEndlessStatement() throws Throwable {
154-
InfiniteLoopStatement infiniteLoop = new InfiniteLoopStatement();
155-
FailOnTimeout infiniteLoopTimeout = builder().withTimeout(TIMEOUT, MILLISECONDS).build(infiniteLoop);
156-
try {
157-
infiniteLoopTimeout.evaluate();
158-
} catch (Exception timeoutException) {
159-
sleep(20); // time to interrupt the thread
160-
int firstCount = InfiniteLoopStatement.COUNT;
161-
sleep(20); // time to increment the count
162-
assertTrue("Thread has not been stopped.",
163-
firstCount == InfiniteLoopStatement.COUNT);
164-
}
165-
}
166-
167-
private static final class InfiniteLoopStatement extends Statement {
168-
private static int COUNT = 0;
169-
170-
@Override
171-
public void evaluate() throws Throwable {
172-
while (true) {
173-
sleep(10); // sleep in order to enable interrupting thread
174-
++COUNT;
175-
}
176-
}
124+
InfiniteLoop infiniteLoop = new InfiniteLoop();
125+
assertThrows(
126+
TestTimedOutException.class,
127+
run(failAfter50Ms(infiniteLoop)));
128+
129+
sleep(20); // time to interrupt the thread
130+
infiniteLoop.stillExecuting.set(false);
131+
sleep(20); // time to increment the count
132+
assertFalse(
133+
"Thread has not been stopped.",
134+
infiniteLoop.stillExecuting.get());
177135
}
178136

179137
@Test
180-
public void stackTraceContainsRealCauseOfTimeout() throws Throwable {
181-
StuckStatement stuck = new StuckStatement();
182-
FailOnTimeout stuckTimeout = builder().withTimeout(TIMEOUT, MILLISECONDS).build(stuck);
183-
try {
184-
stuckTimeout.evaluate();
185-
// We must not get here, we expect a timeout exception
186-
fail("Expected timeout exception");
187-
} catch (Exception timeoutException) {
188-
StackTraceElement[] stackTrace = timeoutException.getStackTrace();
189-
boolean stackTraceContainsTheRealCauseOfTheTimeout = false;
190-
boolean stackTraceContainsOtherThanTheRealCauseOfTheTimeout = false;
191-
for (StackTraceElement element : stackTrace) {
192-
String methodName = element.getMethodName();
193-
if ("theRealCauseOfTheTimeout".equals(methodName)) {
194-
stackTraceContainsTheRealCauseOfTheTimeout = true;
195-
}
196-
if ("notTheRealCauseOfTheTimeout".equals(methodName)) {
197-
stackTraceContainsOtherThanTheRealCauseOfTheTimeout = true;
198-
}
138+
public void stackTraceContainsRealCauseOfTimeout() {
139+
TestTimedOutException timedOutException = assertThrows(
140+
TestTimedOutException.class,
141+
run(failAfter50Ms(new StuckStatement())));
142+
143+
StackTraceElement[] stackTrace = timedOutException.getStackTrace();
144+
boolean stackTraceContainsTheRealCauseOfTheTimeout = false;
145+
boolean stackTraceContainsOtherThanTheRealCauseOfTheTimeout = false;
146+
for (StackTraceElement element : stackTrace) {
147+
String methodName = element.getMethodName();
148+
if ("theRealCauseOfTheTimeout".equals(methodName)) {
149+
stackTraceContainsTheRealCauseOfTheTimeout = true;
150+
}
151+
if ("notTheRealCauseOfTheTimeout".equals(methodName)) {
152+
stackTraceContainsOtherThanTheRealCauseOfTheTimeout = true;
199153
}
200-
assertTrue(
201-
"Stack trace does not contain the real cause of the timeout",
202-
stackTraceContainsTheRealCauseOfTheTimeout);
203-
assertFalse(
204-
"Stack trace contains other than the real cause of the timeout, which can be very misleading",
205-
stackTraceContainsOtherThanTheRealCauseOfTheTimeout);
206154
}
155+
assertTrue(
156+
"Stack trace does not contain the real cause of the timeout",
157+
stackTraceContainsTheRealCauseOfTheTimeout);
158+
assertFalse(
159+
"Stack trace contains other than the real cause of the timeout, which can be very misleading",
160+
stackTraceContainsOtherThanTheRealCauseOfTheTimeout);
207161
}
208162

209163
private static final class StuckStatement extends Statement {
@@ -238,36 +192,85 @@ public void lookingForStuckThread_threadGroupNotLeaked() throws Throwable {
238192
final AtomicReference<ThreadGroup> innerThreadGroup = new AtomicReference<ThreadGroup>();
239193
final AtomicReference<Thread> innerThread = new AtomicReference<Thread>();
240194
final ThreadGroup outerThreadGroup = currentThread().getThreadGroup();
241-
ThrowingRunnable runnable = evaluateWithDelegate(new Statement() {
195+
FailOnTimeout failOnTimeout = failAfter50Ms(new Statement() {
242196
@Override
243197
public void evaluate() {
244198
innerThread.set(currentThread());
245199
ThreadGroup group = currentThread().getThreadGroup();
246-
assertNotSame("inner thread should use a different thread group", outerThreadGroup, group);
200+
assertNotSame("inner thread should use a different thread group",
201+
outerThreadGroup, group);
247202
innerThreadGroup.set(group);
248-
assertTrue("the 'FailOnTimeoutGroup' thread group should be a daemon thread group", group.isDaemon());
203+
assertTrue("the 'FailOnTimeoutGroup' thread group should be a daemon thread group",
204+
group.isDaemon());
249205
}
250206
});
251207

252-
runnable.run();
208+
failOnTimeout.evaluate();
253209

254-
assertTrue("the Statement was never run", innerThread.get() != null);
210+
assertNotNull("the Statement was never run", innerThread.get());
255211
innerThread.get().join();
256-
assertTrue("the 'FailOnTimeoutGroup' thread group should be destroyed after running the test", innerThreadGroup.get().isDestroyed());
212+
assertTrue("the 'FailOnTimeoutGroup' thread group should be destroyed after running the test",
213+
innerThreadGroup.get().isDestroyed());
257214
}
258215

259216
@Test
260217
public void notLookingForStuckThread_usesSameThreadGroup() throws Throwable {
261218
assumeFalse(lookingForStuckThread);
219+
final AtomicBoolean statementWasExecuted = new AtomicBoolean();
262220
final ThreadGroup outerThreadGroup = currentThread().getThreadGroup();
263-
ThrowingRunnable runnable = evaluateWithDelegate(new Statement() {
221+
FailOnTimeout failOnTimeout = failAfter50Ms(new Statement() {
264222
@Override
265223
public void evaluate() {
224+
statementWasExecuted.set(true);
266225
ThreadGroup group = currentThread().getThreadGroup();
267226
assertSame("inner thread should use the same thread group", outerThreadGroup, group);
268227
}
269228
});
270229

271-
runnable.run();
230+
failOnTimeout.evaluate();
231+
232+
assertTrue("the Statement was never run", statementWasExecuted.get());
233+
}
234+
235+
private FailOnTimeout failAfter50Ms(Statement statement) {
236+
return FailOnTimeout.builder()
237+
.withTimeout(50, MILLISECONDS)
238+
.withLookingForStuckThread(lookingForStuckThread)
239+
.build(statement);
240+
}
241+
242+
private ThrowingRunnable run(final FailOnTimeout failOnTimeout) {
243+
return new ThrowingRunnable() {
244+
public void run() throws Throwable {
245+
failOnTimeout.evaluate();
246+
}
247+
};
248+
}
249+
250+
private static class DelegateStatement extends Statement {
251+
Statement delegate;
252+
253+
@Override
254+
public void evaluate() throws Throwable {
255+
delegate.evaluate();
256+
}
257+
}
258+
259+
private static class FastStatement extends Statement {
260+
@Override
261+
public void evaluate() throws Throwable {
262+
}
263+
}
264+
265+
private static final class InfiniteLoop extends Statement {
266+
final AtomicBoolean stillExecuting = new AtomicBoolean();
267+
268+
@Override
269+
public void evaluate() throws Throwable {
270+
while (true) {
271+
sleep(10); // sleep in order to enable interrupting thread
272+
stillExecuting.set(true);
273+
}
274+
}
272275
}
273276
}

0 commit comments

Comments
 (0)