RFR: 8346324: javax/swing/JScrollBar/4865918/bug4865918.java fails in CI

[Alexey Ivanov]

On Tue, 17 Dec 2024 07:44:11 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> javax/swing/JScrollBar/4865918/bug4865918.java fails in CI citing
> 
> java.lang.RuntimeException: The scrollbar block increment is incorrect
> at bug4865918.main(bug4865918.java:60) 
> 
> Seems like scrollbar is not in focus when mouse is pressed..Used CountDownLatch to mage focus gain more determinisitic.
> Fix is passing in CI in several CI systems

In the description you say, “scrollbar is not in focus when mouse is pressed.” Yet there's nothing in the test which handles the focus… even in the updated test.

However, I agree that your update makes the test more reliable, the latch guarantees the synthetic mouse event is handled before analysing the result.

So, it looks good exception for a couple of nits.

test/jdk/javax/swing/JScrollBar/4865918/bug4865918.java line 50:

> 48: 
> 49:     public static void main(String[] argv) throws Exception {
> 50:         latch = new CountDownLatch(1);

Initialise the latch at its declaration.
Suggestion:

    private static final CountDownLatch latch = new CountDownLatch(1);

    public static void main(String[] argv) throws Exception {


`mousePressLatch` would be a more descriptive name.

test/jdk/javax/swing/JScrollBar/4865918/bug4865918.java line 59:

> 57: 
> 58:             SwingUtilities.invokeAndWait(() -> sbar.pressMouse());
> 59:             latch.await();

You should use timed `await` and throw an exception if the wait times out. This way the timeout would be handled more gracefully… and more quickly, a two-second timeout should be enough.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22783#pullrequestreview-2508835078
PR Review Comment: https://git.openjdk.org/jdk/pull/22783#discussion_r1888490126
PR Review Comment: https://git.openjdk.org/jdk/pull/22783#discussion_r1888496261