RFR: 8328402: Implement pausing functionality for the PassFailJFrame

[Alexey Ivanov]

On Tue, 19 Mar 2024 01:14:29 GMT, Alexander Zvegintsev <azvegint at openjdk.org> wrote:

>> we need to add next to Pass/Fail a "Pause Timer" button, that
> (a) stops the count down
> (b) changes the Pause Timer to "Resume Timer"
> ~~(c) disables Pass/Fail until the timer is resumed~~
> the test will not have to pause or be aware - only the PassFailJFrame machinery.
> ~~So the tester can do anything they want except exit the test in the paused mode.~~
> 
> This PR implements everything except the (c).
> I see nothing wrong with completing the test from the paused state, and it does not add an extra click.  
> 
> 
> Example screenshots:
> 
> ![image](https://github.com/openjdk/jdk/assets/77687766/65d34bf8-1e46-4e68-b39e-a77f257ec3c0)
> ![image](https://github.com/openjdk/jdk/assets/77687766/3a43e059-2d48-46f3-9d1b-c2ab84fcce37)

Looks good overall.

However, the _Pause Timer_ is quite large, it's larger than Pass or Fail button, and it attracts more attention.

I would place the Pause button into the timeout panel at the top. Yet the button is larger than the label. The font may be reduced, a symbol '⏸' (U+23F8) and '⏵' (U+23F5) could be used (see [Media control symbols](https://en.wikipedia.org/wiki/Media_control_symbols)), or an image icon with a tooltip.

The timeout and its UI elements — the label and pause button — may be encapsulated into `TimeoutHandler` and return a `JPanel`.

Yet the above improvements could be handled separately. I agree there should be a way to pause the test. I agree that Pass and Fail button can remain enabled, the tester could proceed with the test even when the timer isn't running.

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 647:

> 645:         private static final String PAUSE_BUTTON_LABEL = "Pause Timer";
> 646:         private static final String RESUME_BUTTON_LABEL = "Resume Timer";
> 647:         private long endTime;

Suggestion:

        private static final String RESUME_BUTTON_LABEL = "Resume Timer";

        private long endTime;

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 648:

> 646:         private static final String RESUME_BUTTON_LABEL = "Resume Timer";
> 647:         private long endTime;
> 648:         private long pauseTimeLeft = 0L;

Suggestion:

        private long pauseTimeLeft;

Zero is the default value; you never use `pauseTimeLeft` before assigning it a value when the Pause button is pressed.

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 704:

> 702:             } else {
> 703:                 label.setText(label.getText().replace(PAUSED_LABEL_SUFFIX, ""));
> 704:                 endTime = System.currentTimeMillis() + pauseTimeLeft;

I'd rather call `updateTime(pauseTimeLeft)` here instead of editing the text.

Suggestion:

                endTime = System.currentTimeMillis() + pauseTimeLeft;
                updateTime(pauseTimeLeft);

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18368#pullrequestreview-1949607461
PR Review Comment: https://git.openjdk.org/jdk/pull/18368#discussion_r1532523214
PR Review Comment: https://git.openjdk.org/jdk/pull/18368#discussion_r1532529917
PR Review Comment: https://git.openjdk.org/jdk/pull/18368#discussion_r1532538482