Skip to content

Throw TestFailedOnTimeoutException instead of plain Exception on timeout. Fixes #771 #777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 14 commits into from
Closed

Throw TestFailedOnTimeoutException instead of plain Exception on timeout. Fixes #771 #777

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
dcc298c
Throw TestFailedOnTimeoutException instead of plain Exception on
Nov 28, 2013
619f4ff
Improvements after code review. Move Exception to non internal package
Nov 29, 2013
b645d2b
Use TestFailedOnTimeoutException when test times out
Nov 29, 2013
bf2f922
Add second test verifying the time unit and timeout value fields of the
Dec 2, 2013
6a302a6
minor code review changes
Dec 2, 2013
66bfb24
Refactor test, replace matcher with try catch
Dec 4, 2013
9c5a2f2
Merge remote-tracking branch 'upstream/master'
Dec 25, 2013
26c25f3
Rename TestFailedOnTimeoutException -> TestTimedOutException
Dec 25, 2013
a4f088d
minor formatting change
Jan 7, 2014
f03bc79
minor javadoc change
Jan 7, 2014
44e2428
Merge remote-tracking branch 'upstream/master'
Jan 20, 2014
9bd737e
Revert previous commit
Jan 24, 2014
848f6b7
Merge branch 'master' of https://github.com/askoog/junit.git
Jan 24, 2014
1e650b2
remove duplicate JunitCoreTest declaration
Jan 24, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/junit/internal/runners/statements/FailOnTimeout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private Throwable getResult(FutureTask<Throwable> task, Thread thread) {
private Exception createTimeoutException(Thread thread) {
StackTraceElement[] stackTrace = thread.getStackTrace();
final Thread stuckThread = fLookForStuckThread ? getStuckThread(thread) : null;
Exception currThreadException = new Exception(String.format(
Exception currThreadException = new TestFailedOnTimeoutException(String.format(
"test timed out after %d %s", fTimeout, fTimeUnit.name().toLowerCase()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are making a custom exception, then perhaps we should pass these values into the constructor and provide methods in the exception to get these values so people don't have to parse the exception message

if (stackTrace != null) {
currThreadException.setStackTrace(stackTrace);
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/junit/internal/runners/statements/TestFailedOnTimeoutException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.junit.internal.runners.statements;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to reference this class in your own code, we should probably not have it under "internal"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Any ideas for a better package?


public class TestFailedOnTimeoutException extends Exception {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add Javadoc?


private static final long serialVersionUID = 31935685163547539L;

public TestFailedOnTimeoutException(String message) {
super(message);
}

}
4 changes: 3 additions & 1 deletion src/test/java/org/junit/tests/AllTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.junit.tests.experimental.theories.runner.WithAutoGeneratedDataPoints;
import org.junit.tests.experimental.theories.runner.WithDataPointMethod;
import org.junit.tests.experimental.theories.runner.WithNamedDataPoints;
import org.junit.tests.internal.runners.statements.FailOnTimeoutTest;
import org.junit.tests.junit3compatibility.AllTestsTest;
import org.junit.tests.junit3compatibility.ClassRequestTest;
import org.junit.tests.junit3compatibility.ForwardCompatibilityTest;
Expand Down Expand Up @@ -196,7 +197,8 @@
JUnitCommandLineParseResultTest.class,
FilterFactoriesTest.class,
CategoryFilterFactoryTest.class,
JUnitCoreTest.class
JUnitCoreTest.class,
FailOnTimeoutTest.class
})
public class AllTests {
public static Test suite() {
Expand Down
8 changes: 8 additions & 0 deletions src/test/java/org/junit/tests/internal/runners/statements/FailOnTimeoutTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.internal.runners.statements.FailOnTimeout;
import org.junit.internal.runners.statements.TestFailedOnTimeoutException;
import org.junit.rules.ExpectedException;
import org.junit.runners.model.Statement;

Expand Down Expand Up @@ -61,6 +62,13 @@ public void throwTimeoutExceptionOnSecondCallAlthoughFirstCallThrowsException()
evaluateWithWaitDuration(TIMEOUT + 50);
}

@Test
public void throwsTestFailedWithTimeoutException()
throws Throwable {
thrown.expect(TestFailedOnTimeoutException.class);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the expectations to check for the contents of the message? Perhaps a regex match for "test timed out after 5 seconds" (replacing "5" and "second" for the actual values, of course).

evaluateWithWaitDuration(TIMEOUT + 50);
}

private void evaluateWithException(Exception exception) throws Throwable {
statement.nextException = exception;
statement.waitDuration = 0;
Expand Down