Merge pull request #986 from kcooney/Timeout-rule-builder

Add builder for Timeout rule. Make Timeout rule designed for extension.

doc/ReleaseNotes4.12.md

@@ -136,9 +136,24 @@ If a custom failure message is not provided, a default message is used.
 When a test times out, a `org.junit.runners.model.TestTimedOutException` is now thrown instead of a plain `java.lang.Exception`.
 
 
-### [Pull request #742:](https://github.com/junit-team/junit/pull/742) `Timeout` exceptions now include stack trace from stuck thread (experimental)
+### [Pull request #742:](https://github.com/junit-team/junit/pull/742) [Pull request #986:](https://github.com/junit-team/junit/pull/986) `Timeout` exceptions now include stack trace from stuck thread (experimental)
 
-`Timeout` exceptions try to determine if there is a child thread causing the problem, and if so its stack trace is included in the exception in addition to the one of the main thread. This feature must be enabled with a rule such as `new Timeout(100, TimeUnit.MILLISECONDS).lookingForStuckThread(true)`.
+`Timeout` exceptions try to determine if there is a child thread causing the problem, and if so its stack trace is included in the exception in addition to the one of the main thread. This feature must be enabled with the timeout rule by creating it through the new `Timeout.builder()` method:
+
+```java
+public class HasGlobalTimeout {
+    @Rule public final TestRule timeout = Timeout.builder()
+            .withTimeout(10, TimeUnit.SECONDS)
+            .withLookingForStuckThread(true)
+            .build();
+
+    @Test
+    public void testInfiniteLoop() {
+        for (;;) {
+        }
+    }
+}
+```
 
 
 ### [Pull request #544:](https://github.com/junit-team/junit/pull/544) New constructor and factories in `Timeout`
@@ -147,17 +162,21 @@ When a test times out, a `org.junit.runners.model.TestTimedOutException` is now
 A new constructor is available: `Timeout(long timeout, TimeUnit unit)`. It enables you to use different granularities of time units like `NANOSECONDS`, `MICROSECONDS`, `MILLISECONDS`, and `SECONDS`. Examples:
 
 ```java
-    @Rule public final TestRule globalTimeout = new Timeout(50, TimeUnit.MILLISECONDS);
+@Rule public final TestRule globalTimeout = new Timeout(50, TimeUnit.MILLISECONDS);
+```
 
-    @Rule public final TestRule globalTimeout = new Timeout(10, TimeUnit.SECONDS);
+```java
+@Rule public final TestRule globalTimeout = new Timeout(10, TimeUnit.SECONDS);
 ```
 
 and factory methods in `Timeout`:
 
 ```java
-    @Rule public final TestRule globalTimeout = Timeout.millis(50);
+@Rule public final TestRule globalTimeout = Timeout.millis(50);
+```
 
-    @Rule public final TestRule globalTimeout = Timeout.seconds(10);`
+```java
+@Rule public final TestRule globalTimeout = Timeout.seconds(10);
 ```
 
 This usage avoids the truncation, which was the problem in the deprecated constructor `Timeout(int millis)` when casting `long` to `int`.

src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java

@@ -23,6 +23,8 @@ public class FailOnTimeout extends Statement {
 
     /**
      * Returns a new builder for building an instance.
+     *
+     * @since 4.12
      */
     public static Builder builder() {
         return new Builder();
@@ -49,6 +51,8 @@ private FailOnTimeout(Builder builder, Statement statement) {
 
     /**
      * Builder for {@link FailOnTimeout}.
+     *
+     * @since 4.12
      */
     public static class Builder {
         private boolean lookForStuckThread = false;

src/main/java/org/junit/rules/Timeout.java

@@ -42,6 +42,15 @@ public class Timeout implements TestRule {
     private final TimeUnit timeUnit;
     private final boolean lookForStuckThread;
 
+    /**
+     * Returns a new builder for building an instance.
+     *
+     * @since 4.12
+     */
+    public static Builder builder() {
+        return new Builder();
+    }
+
     /**
      * Create a {@code Timeout} instance with the timeout specified
      * in milliseconds.
@@ -75,54 +84,75 @@ public Timeout(long timeout, TimeUnit timeUnit) {
     }
 
     /**
-     * Create a {@code Timeout} instance with the same fields as {@code t}
-     * except for {@code lookForStuckThread}.
+     * Create a {@code Timeout} instance initialized with values form
+     * a builder.
      *
-     * @param t the {@code Timeout} instance to copy
-     * @param lookForStuckThread whether to look for a stuck thread
      * @since 4.12
      */
-    protected Timeout(Timeout t, boolean lookForStuckThread) {
-        timeout = t.timeout;
-        timeUnit = t.timeUnit;
-        this.lookForStuckThread = lookForStuckThread;
+    protected Timeout(Builder builder) {
+        timeout = builder.getTimeout();
+        timeUnit = builder.getTimeUnit();
+        lookForStuckThread = builder.getLookingForStuckThread();
     }
 
     /**
-     * @param millis the timeout in milliseconds
+     * Creates a {@link Timeout} that will timeout a test after the
+     * given duration, in milliseconds.
+     *
      * @since 4.12
      */
     public static Timeout millis(long millis) {
         return new Timeout(millis, TimeUnit.MILLISECONDS);
     }
 
     /**
-     * @param seconds the timeout in seconds
+     * Creates a {@link Timeout} that will timeout a test after the
+     * given duration, in seconds.
+     *
      * @since 4.12
      */
     public static Timeout seconds(long seconds) {
         return new Timeout(seconds, TimeUnit.SECONDS);
     }
 
     /**
-     * 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 This object
+     * Gets the timeout configured for this rule, in the given units.
+     *
+     * @since 4.12
+     */
+    protected final long getTimeout(TimeUnit unit) {
+        return unit.convert(timeout, timeUnit);
+    }
+
+    /**
+     * Gets whether this {@code Timeout} will look for a stuck thread
+     * when the test times out.
+     *
+     * @since 4.12
+     */
+    protected final boolean getLookingForStuckThread() {
+        return lookForStuckThread;
+    }
+
+    /**
+     * Creates a {@link Statement} that will run the given
+     * {@code statement}, and timeout the operation based
+     * on the values configured in this rule. Subclasses
+     * can override this method for different behavior.
+     *
      * @since 4.12
      */
-    public Timeout lookingForStuckThread(boolean enable) {
-        return new Timeout(this, enable);
+    protected Statement createFailOnTimeoutStatement(
+            Statement statement) throws Exception {
+        return FailOnTimeout.builder()
+            .withTimeout(timeout, timeUnit)
+            .withLookingForStuckThread(lookForStuckThread)
+            .build(statement);
     }
 
     public Statement apply(Statement base, Description description) {
         try {
-            return FailOnTimeout.builder()
-                .withTimeout(timeout, timeUnit)
-                .withLookingForStuckThread(lookForStuckThread)
-                .build(base);
+            return createFailOnTimeoutStatement(base);
         } catch (final Exception e) {
             return new Statement() {
                 @Override public void evaluate() throws Throwable {
@@ -131,4 +161,73 @@ public Statement apply(Statement base, Description description) {
             };
         }
     }
+
+    /**
+     * Builder for {@link Timeout}.
+     *
+     * @since 4.12
+     */
+    public static class Builder {
+        private boolean lookForStuckThread = false;
+        private long timeout = 0;
+        private TimeUnit timeUnit = TimeUnit.SECONDS;
+
+        protected Builder() {
+        }
+
+        /**
+         * Specifies the time to wait before timing out the test.
+         *
+         * <p>If this is not called, or is called with a
+         * {@code timeout} of {@code 0}, the returned {@code Timeout}
+         * rule instance will cause the tests to wait forever to
+         * complete, however the tests 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) {
+            this.timeout = timeout;
+            this.timeUnit = unit;
+            return this;
+        }
+
+        protected long getTimeout() {
+            return timeout;
+        }
+
+        protected TimeUnit getTimeUnit()  {
+            return timeUnit;
+        }
+
+        /**
+         * Specifies whether to look for a stuck thread.  If a timeout occurs and this
+         * feature is enabled, the rule 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;
+        }
+
+        protected boolean getLookingForStuckThread() {
+            return lookForStuckThread;
+        }
+
+
+        /**
+         * Builds a {@link Timeout} instance using the values in this builder.,
+         */
+        public Timeout build() {
+            return new Timeout(this);
+        }
+    }
 }

src/test/java/org/junit/tests/running/methods/TimeoutTest.java

@@ -199,7 +199,10 @@ public void failure(boolean mainThreadStalls) throws Exception {
 
     public static class InfiniteLoopWithStuckThreadTest {
         @Rule
-        public TestRule globalTimeout = new Timeout(100, TimeUnit.MILLISECONDS).lookingForStuckThread(true);
+        public TestRule globalTimeout = Timeout.builder()
+            .withTimeout(100, TimeUnit.MILLISECONDS)
+            .withLookingForStuckThread(true)
+            .build();
 
         @Test
         public void failure() throws Exception {
@@ -209,7 +212,10 @@ public void failure() throws Exception {
 
     public static class InfiniteLoopStuckInMainThreadTest {
         @Rule
-        public TestRule globalTimeout = new Timeout(100, TimeUnit.MILLISECONDS).lookingForStuckThread(true);
+        public TestRule globalTimeout = Timeout.builder()
+            .withTimeout(100, TimeUnit.MILLISECONDS)
+            .withLookingForStuckThread(true)
+            .build();
 
         @Test
         public void failure() throws Exception {
@@ -296,4 +302,33 @@ public void testZeroTimeoutIsIgnored() {
         assertEquals("Should run the test", 1, result.getRunCount());
         assertEquals("Test should not have failed", 0, result.getFailureCount());
     }
+
+    private static class TimeoutSubclass extends Timeout {
+
+        public TimeoutSubclass(long timeout, TimeUnit timeUnit) {
+            super(timeout, timeUnit);
+        }
+
+        public long getTimeoutFromSuperclass(TimeUnit unit) {
+            return super.getTimeout(unit);
+        }
+    }
+
+    public static class TimeOutOneSecond {
+        @Rule
+        public TimeoutSubclass timeout = new TimeoutSubclass(1, TimeUnit.SECONDS);
+
+        @Test
+        public void test() {
+            assertEquals(1000, timeout.getTimeoutFromSuperclass(TimeUnit.MILLISECONDS));
+        }
+    }
+
+    @Test
+    public void testGetTimeout() {
+        JUnitCore core = new JUnitCore();
+        Result result = core.run(TimeOutOneSecond.class);
+        assertEquals("Should run the test", 1, result.getRunCount());
+        assertEquals("Test should not have failed", 0, result.getFailureCount());
+    }
 }