Support to allow the Timeout @Rule to become disabled when debugging without modification to the test class #904
Conversation
| public Statement apply(Statement base, Description description) { | ||
| if (isDebugging() && !fEnableWhenDebugging) { |
There was a problem hiding this comment.
could you reverse the ordering of this expression? Most of the time, fEnableWhenDebugging should be true.
|
Thanks. Not so sure how we would test this. If we put tests in the same package as the code, we could have a package-scope constructor to inject in the JVM input arguments. |
|
Very true, it's one of the reasons I didn't submit any tests with the origin request. I think what you suggest would work well (or we could even inject a RuntimeMXBean) but in order for it to work nicely with the builder-pattern the Timeout class uses, it would end up being passed around everywhere. I've added a test that adds assertions around the control flow in apply(). It does not cover how isDebugging() works, or the builder-pattern aspects of the codebase. Let me know your thoughts. |
| * the test ran has a debugger attached the test will not timeout to allow | ||
| * the user time to debug. | ||
| * | ||
| * Timeouts or time sensitive logic in the code under test is not handled |
There was a problem hiding this comment.
Please put a
tag in front of every new paragraph
|
@kay Sorry for taking so long to get back to you |
|
Absolutely no problem. Thanks very much for finding the time. |
|
I don't have a strong feeling about putting tests in the same class. I think that sometimes wanting to create an only-for-testing API indicates a missing public API waiting to be carefully designed. On the other hand, sometimes one doesn't have time for careful design, and the carelessly designed API ends up public forever. So won't push hard for either solution. |
been launched in debug mode. Allowing users to debug their tests without the timeout firing. This feature may be disabled by calling Timeout.whenDebugging(true) which forces the timeout to fire even during debugging. Timeouts or time sensitive logic in the code under test is not handled by this feature and may make this unuseful in some circumstances. The important benefit of this feature is that you can suspend timeouts without any making any modifications to your test class to remove them during debugging. Anecdotally removing timeouts for debug purposes often results in developers forgetting to restore them.
|
I've resolved recent merge conflicts, squashed and rebased this branch. |
Feature to disable the Timeout @rule when the test class being ran has been launched in debug mode. Allowing users to debug their tests without the timeout firing. This feature may be disabled by calling Timeout.whenDebugging(true) which forces the timeout to fire even during debugging.
Timeouts or time sensitive logic in the code under test is not handled by this feature and may make this unuseful in some circumstances. The important benefit of this feature is that you can suspend timeouts without any making any modifications to your test class to remove them during debugging. Anecdotally removing timeouts for debug purposes often results in developers forgetting to restore them.
In this pull-request I've enabled the feature by default. There is perhaps an argument to disable it by default and allow users to opt-in. I'm very open to discussion on this.
This feature somewhat relates to this #738
I've tried to keep as close to the JUnit projects conventions but please let me know if there is anything you'd prefer to be changed.