RFR: 8328629: JUnit test without a timeout value can hang indefinitely

[Kevin Rushforth]

On Fri, 20 Sep 2024 12:14:36 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> tests/system/src/test/java/test/robot/javafx/scene/control/behavior/BehaviorRobotTestBase.java line 329:
>> 
>>> 327:     protected void sleep(int ms) {
>>> 328:         try {
>>> 329:             // KCR: this should be sleep(ms) not a hard-coded 1
>> 
>> good thing it's never called, but it is my fault.
>> this method was later replaced with `waitForIdle`
>> 
>> I can either delete the method or fix it as part of #1569
>> what do you think?
>
> If it isn't being used, then I like your idea of deleting it. Especially since this seems redundant given `Util.sleep(ms)`. I'll revert my changes and let you delete it as part of #1569.

On closer inspection, I don't think this should be removed as part of PR #1569, since that file is already using JUnit 5 and is not touched by your PR.

Either I can remove the method with this PR or we just leave it for a follow-up. Removing it with this PR seems easiest, since I am already modifying that method as part of this PR (along with other methods that catch and ignore InterruptedException from sleep).

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1575#discussion_r1768826711