JUnit5 JenkinsRule supports timeout#979
JUnit5 JenkinsRule supports timeout#979strangelookingnerd wants to merge 1 commit intojenkinsci:masterfrom
Conversation
cdcfa94 to
3307464
Compare
| class JenkinsRuleTimeout5Test extends JenkinsRuleTimeoutTestBase { | ||
|
|
||
| // this is the only way to make JenkinsRule#timeout work here | ||
| private static final String TIMEOUT = System.setProperty("jenkins.test.timeout", "30"); |
There was a problem hiding this comment.
there a reason you aren't just going the way of setting the default junit test timeout?
There was a problem hiding this comment.
I was trying to keep JenkinsRule#timeout and @WithTimeout functional as before. Which kind of works I think, but the outcome - as you can see - is a mess.
If we can simply ignore the above as a requirement, then yes, setting the default timeout is the easiest solution.
It could be based on jenkins.test.timeout property to keep that functional as well.
There was a problem hiding this comment.
I think the built-in would be fine and a javadoc update to say not supported on JUnit 5 and to point to the new way instead
There was a problem hiding this comment.
I gave it another go an hit a wall. Just pasting my findings here.
It appears that the global timeout defined via junit.jupiter.execution.timeout.default does handle lifecyle methods and tests, however this does not include anything happening in an Extension.
This is unfortunate as it will not allow us to cover JenkinsRule#before or JenkinsRule#after by these timeouts. This could be by-passed by wrapping these calls with assertTimeout or assertTmeoutPreemptively. Yet this comes with a caveat:
By default timeouts are not handled preemptively in JUnit5 (see https://junit.org/junit5/docs/current/user-guide/#writing-tests-declarative-timeouts-thread-mode). This means a test will report that it timed-out only if it at some point finishes. If execution hangs indefinitly for what ever reason, tests will do so as well.
I tried to by-pass that by setting the thread-mode as suggested in the documentation. This will then interrupt hanging threads, but also comes with side-effects for the behavior due to ThreadLocal usages as suggested by the documentation.
Not really what I was hoping for, yet there still may be some things I have not yet tried out.
Trying to fix #977.
Draft for now, implementation is very rough.
Testing done
Automated testing will be an issue as it is tricky to make tests pass that have failed with a timeout 😄
Submitter checklist