> This PR adds a default timeout of 120 seconds for JUnit 5 tests that do not 
> have an explicit `@Timeout` on either the methods or the class, and a default 
> timeout of 20 seconds for lifecycle methods (e.g., `@BeforeEach`, 
> `@BeforeAll`, `@AfterEach`, `@AfterAll`).
> 
> JUnit 5 effects its timeout by interrupting the test thread. We have several 
> tests that catch and ignore InterruptedException for sleep or await, so I 
> fixed these tests to throw an error if InterruptedException is caught (they 
> may or may not be a problem, but if any of them can happen in a loop, it 
> would block the timeout from killing the test).
> 
> While testing this, I discovered that by itself, this JUnit 5 timeout is 
> ineffective if the deadlock described in 
> [JDK-8238505](https://bugs.openjdk.org/browse/JDK-8238505)  occurs. The 
> reason for this is that the deadlock almost always occurs when called from a 
> shutdown hook (called by `System.exit`), and that prevents the process from 
> exiting even when interrupted by the test runner after the timeout. A 
> separate PR is out for that bug fix.
> 
> ### Testing instructions
> 
> I tested this in connection with PR #1574 / 
> [JDK-8340405](https://bugs.openjdk.org/browse/JDK-8340405) using my 
> [test-timeout-and-shutdown-hang](https://github.com/kevinrushforth/jfx/tree/test-timeout-and-shutdown-hang)
>  branch. That branch has the fixes from both this PR and PR #1574 as well as 
> tests that will hang indefinitely without the fixes and fail due to the 
> expected timeout with the fixes.
> 
> The default timeout values for test and lifecycle methods can be modified 
> using a gradle property:
> 
> * `JUNIT_TEST_TIMEOUT` : default = 120s
> * `JUNIT_LIFECYCLE_TIMEOUT` : default = 20s
> 
> For example, to set a timeout of 60 seconds for tests and 15 seconds for 
> lifecycle methods:
> 
> 
> gradle -PJUNIT_TEST_TIMEOUT=60s -PJUNIT_LIFECYCLE_TIMEOUT=15s test

Kevin Rushforth has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains five additional 
commits since the last revision:

 - throw new AssertionError(ex) --> fail(ex)
 - Merge remote-tracking branch 'upstream/master' into 8328629-junit-timeout
 - Remove unused sleep method from BehaviorRobotTestBase.
 - Fix typo (extra spaces) in system property name
 - 8328629: JUnit test without a timeout value can hang indefinitely

-------------

Changes:
  - all: https://git.openjdk.org/jfx/pull/1575/files
  - new: https://git.openjdk.org/jfx/pull/1575/files/8a4078fe..de443509

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx&pr=1575&range=02
 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1575&range=01-02

  Stats: 38648 lines in 1261 files changed: 15843 ins; 5497 del; 17308 mod
  Patch: https://git.openjdk.org/jfx/pull/1575.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1575/head:pull/1575

PR: https://git.openjdk.org/jfx/pull/1575

Reply via email to