-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix stop watch thread safety #606
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package cucumber.runtime; | ||
|
||
import static org.junit.Assert.assertNull; | ||
|
||
import org.junit.Test; | ||
|
||
public class StopWatchTest { | ||
private final StopWatch stopWatch = StopWatch.SYSTEM; | ||
private Throwable exception; | ||
|
||
@Test | ||
public void should_be_thread_safe() { | ||
try { | ||
Thread timerThreadOne = new TimerThread(500L); | ||
Thread timerThreadTwo = new TimerThread(750L); | ||
|
||
timerThreadOne.start(); | ||
timerThreadTwo.start(); | ||
|
||
timerThreadOne.join(); | ||
timerThreadTwo.join(); | ||
|
||
assertNull("null_pointer_exception", exception); | ||
} catch (InterruptedException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try-catch block can be removed at all if the InterruptedException is moved upper in the throws section of the test method declaration. |
||
Thread.currentThread().interrupt(); | ||
} | ||
} | ||
|
||
class TimerThread extends Thread { | ||
private final long timeoutMillis; | ||
|
||
public TimerThread(long timeoutMillis) { | ||
this.timeoutMillis = timeoutMillis; | ||
} | ||
|
||
@Override | ||
public void run() { | ||
try { | ||
stopWatch.start(); | ||
Thread.sleep(timeoutMillis); | ||
stopWatch.stop(); | ||
} catch (NullPointerException e) { | ||
exception = e; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The e can be re-thrown instead of being saved in the field. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can it though? The point is to verify that no exceptions occurred on line 23. |
||
} catch (InterruptedException e) { | ||
Thread.currentThread().interrupt(); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be the
start.remove()
is more appropriate.