diff --git a/src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java b/src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java index 93a5760326a3..6ce7bebe38fe 100644 --- a/src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java +++ b/src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java @@ -21,19 +21,94 @@ public class FailOnTimeout extends Statement { private final boolean lookForStuckThread; private volatile ThreadGroup threadGroup = null; - public FailOnTimeout(Statement originalStatement, long millis) { - this(originalStatement, millis, TimeUnit.MILLISECONDS); + /** + * Returns a new builder for building an instance. + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Creates an instance wrapping the given statement with the given timeout in milliseconds. + * + * @param statement the statement to wrap + * @param timeoutMillis the timeout in milliseconds + * @deprecated use {@link #builder()} instead. + */ + @Deprecated + public FailOnTimeout(Statement statement, long timeoutMillis) { + this(builder().withTimeout(timeoutMillis, TimeUnit.MILLISECONDS), statement); } - public FailOnTimeout(Statement originalStatement, long timeout, TimeUnit unit) { - this(originalStatement, timeout, unit, false); + private FailOnTimeout(Builder builder, Statement statement) { + originalStatement = statement; + timeout = builder.timeout; + timeUnit = builder.unit; + lookForStuckThread = builder.lookForStuckThread; } - public FailOnTimeout(Statement originalStatement, long timeout, TimeUnit unit, boolean lookForStuckThread) { - this.originalStatement = originalStatement; - this.timeout = timeout; - timeUnit = unit; - this.lookForStuckThread = lookForStuckThread; + /** + * Builder for {@link FailOnTimeout}. + */ + public static class Builder { + private boolean lookForStuckThread = false; + private long timeout = 0; + private TimeUnit unit = TimeUnit.SECONDS; + + private Builder() { + } + + /** + * Specifies the time to wait before timing out the test. + * + *

If this is not called, or is called with a {@code timeout} of + * {@code 0}, the returned {@code Statement} will wait forever for the + * test to complete, however the test will still launch from a separate + * thread. This can be useful for disabling timeouts in environments + * where they are dynamically set based on some property. + * + * @param timeout the maximum time to wait + * @param unit the time unit of the {@code timeout} argument + * @return {@code this} for method chaining. + */ + public Builder withTimeout(long timeout, TimeUnit unit) { + if (timeout < 0) { + throw new IllegalArgumentException("timeout must be non-negative"); + } + if (unit == null) { + throw new NullPointerException("TimeUnit cannot be null"); + } + this.timeout = timeout; + this.unit = unit; + return this; + } + + /** + * Specifies whether to look for a stuck thread. If a timeout occurs and this + * feature is enabled, the test will look for a thread that appears to be stuck + * and dump its backtrace. This feature is experimental. Behavior may change + * after the 4.12 release in response to feedback. + * + * @param enable {@code true} to enable the feature + * @return {@code this} for method chaining. + */ + public Builder withLookingForStuckThread(boolean enable) { + this.lookForStuckThread = enable; + return this; + } + + /** + * Builds a {@link FailOnTimeout} instance using the values in this builder, + * wrapping the given statement. + * + * @param statement + */ + public FailOnTimeout build(Statement statement) { + if (statement == null) { + throw new NullPointerException("statement cannot be null"); + } + return new FailOnTimeout(this, statement); + } } @Override @@ -86,8 +161,8 @@ private Exception createTimeoutException(Thread thread) { new Exception ("Appears to be stuck in thread " + stuckThread.getName()); stuckThreadException.setStackTrace(getStackTrace(stuckThread)); - return new MultipleFailureException - (Arrays.asList(currThreadException, stuckThreadException)); + return new MultipleFailureException( + Arrays.asList(currThreadException, stuckThreadException)); } else { return currThreadException; } @@ -117,12 +192,12 @@ private StackTraceElement[] getStackTrace(Thread thread) { * problem or if the thread cannot be determined. The return value is never equal * to {@code mainThread}. */ - private Thread getStuckThread (Thread mainThread) { - if (threadGroup == null){ + private Thread getStuckThread(Thread mainThread) { + if (threadGroup == null) { return null; } Thread[] threadsInGroup = getThreadArray(threadGroup); - if (threadsInGroup == null){ + if (threadsInGroup == null) { return null; } @@ -162,13 +237,16 @@ private Thread[] getThreadArray(ThreadGroup group) { while (true) { threads = new Thread[enumSize]; enumCount = group.enumerate(threads); - if (enumCount < enumSize) break; + if (enumCount < enumSize) { + break; + } // if there are too many threads to fit into the array, enumerate's result // is >= the array's length; therefore we can't trust that it returned all // the threads. Try again. enumSize += 100; - if (++loopCount >= 5) + if (++loopCount >= 5) { return null; + } // threads are proliferating too fast for us. Bail before we get into // trouble. } @@ -185,8 +263,9 @@ private Thread[] getThreadArray(ThreadGroup group) { private Thread[] copyThreads(Thread[] threads, int count) { int length = Math.min(count, threads.length); Thread[] result = new Thread[length]; - for (int i = 0; i < length; i++) + for (int i = 0; i < length; i++) { result[i] = threads[i]; + } return result; } diff --git a/src/main/java/org/junit/rules/Timeout.java b/src/main/java/org/junit/rules/Timeout.java index 2ec3b564467f..eade271c7268 100644 --- a/src/main/java/org/junit/rules/Timeout.java +++ b/src/main/java/org/junit/rules/Timeout.java @@ -118,6 +118,17 @@ public Timeout lookingForStuckThread(boolean enable) { } public Statement apply(Statement base, Description description) { - return new FailOnTimeout(base, timeout, timeUnit, lookForStuckThread); + try { + return FailOnTimeout.builder() + .withTimeout(timeout, timeUnit) + .withLookingForStuckThread(lookForStuckThread) + .build(base); + } catch (final Exception e) { + return new Statement() { + @Override public void evaluate() throws Throwable { + throw new RuntimeException("Invalid parameters for Timeout", e); + } + }; + } } } diff --git a/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java b/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java index 29b681e75096..d024d73cf1a9 100644 --- a/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java +++ b/src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.junit.After; import org.junit.Before; @@ -310,7 +311,12 @@ protected Statement possiblyExpectingExceptions(FrameworkMethod method, protected Statement withPotentialTimeout(FrameworkMethod method, Object test, Statement next) { long timeout = getTimeout(method.getAnnotation(Test.class)); - return timeout > 0 ? new FailOnTimeout(next, timeout) : next; + if (timeout <= 0) { + return next; + } + return FailOnTimeout.builder() + .withTimeout(timeout, TimeUnit.MILLISECONDS) + .build(next); } /** diff --git a/src/test/java/org/junit/tests/experimental/rules/TimeoutRuleTest.java b/src/test/java/org/junit/tests/experimental/rules/TimeoutRuleTest.java index d66441f4b4fd..288f1bf056f6 100644 --- a/src/test/java/org/junit/tests/experimental/rules/TimeoutRuleTest.java +++ b/src/test/java/org/junit/tests/experimental/rules/TimeoutRuleTest.java @@ -23,6 +23,7 @@ import org.junit.rules.Timeout; import org.junit.runner.JUnitCore; import org.junit.runner.Result; +import org.junit.runner.notification.Failure; public class TimeoutRuleTest { private static final ReentrantLock run1Lock = new ReentrantLock(); @@ -101,6 +102,16 @@ public static class HasGlobalTimeUnitTimeout extends AbstractTimeoutTest { @Rule public final TestRule globalTimeout = new Timeout(200, TimeUnit.MILLISECONDS); } + + public static class HasNullTimeUnit { + + @Rule + public final TestRule globalTimeout = new Timeout(200, null); + + @Test + public void wouldPass() { + } + } @Before public void before() { @@ -140,4 +151,15 @@ public void longTimeout() { assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run5")); assertThat(HasGlobalLongTimeout.logger.toString(), containsString("run6")); } + + @Test + public void nullTimeUnit() { + Result result = JUnitCore.runClasses(HasNullTimeUnit.class); + assertEquals(1, result.getFailureCount()); + Failure failure = result.getFailures().get(0); + assertThat(failure.getException().getMessage(), + containsString("Invalid parameters for Timeout")); + Throwable cause = failure.getException().getCause(); + assertThat(cause.getMessage(), containsString("TimeUnit cannot be null")); + } }