Skip to content

Commit

Permalink
fixes #875
Browse files Browse the repository at this point in the history
Fixes #875

In versions 4.7 through 4.11 setting up a Timeout @rule with a value of
0 would result in no timeout. In 4.12 FailOnTimeout switched to using
FutureTask.get(timeout). That method treats non-positive timeouts to
mean "timeout right now".

This fix checks that the timeout value is positive before using it,
and otherwise will wait indefinitely.
  • Loading branch information
Mike Drob committed Apr 15, 2014
1 parent e65558c commit c52397d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ public void evaluate() throws Throwable {
*/
private Throwable getResult(FutureTask<Throwable> task, Thread thread) {
try {
return task.get(fTimeout, fTimeUnit);
if (fTimeout > 0) {
return task.get(fTimeout, fTimeUnit);
} else {
return task.get();
}
} catch (InterruptedException e) {
return e; // caller will re-throw; no need to call Thread.interrupt()
} catch (ExecutionException e) {
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/junit/rules/Timeout.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
* the test completes, its execution is interrupted via {@link Thread#interrupt()}.
* This happens in interruptable I/O and locks, and methods in {@link Object}
* and {@link Thread} throwing {@link InterruptedException}.
* <p>
* A specified timeout of 0 will be interpreted as not set, however tests will
* still launch from separate threads. This can be useful for disabling timeouts
* in environments where they are dynamically set based on some property.
*
* @since 4.7
*/
Expand Down
22 changes: 22 additions & 0 deletions src/test/java/org/junit/tests/running/methods/TimeoutTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -274,4 +274,26 @@ public void makeSureAfterIsCalledAfterATimeout() {
JUnitCore.runClasses(WillTimeOut.class);
assertThat(WillTimeOut.afterWasCalled, is(true));
}

public static class TimeOutZero {
@Rule
public Timeout timeout = Timeout.seconds(0);

@Test
public void test() {
try {
Thread.sleep(200); // long enough to suspend thread execution
} catch (InterruptedException e) {
// Don't care
}
}
}

@Test
public void testZeroTimeoutIsIgnored() {
JUnitCore core = new JUnitCore();
Result result = core.run(TimeOutZero.class);
assertEquals("Should run the test", 1, result.getRunCount());
assertEquals("Test should not have failed", 0, result.getFailureCount());
}
}

0 comments on commit c52397d

Please sign in to comment.