RFR: 8284898 : Enhance PassFailJFrame [v8]

[Alexey Ivanov]

On Sat, 16 Apr 2022 23:02:13 GMT, lawrence.andrews <duke at openjdk.java.net> wrote:

>> Fixed the following issue.
>> 1) Removed yes/no since test was failing due to "Parser error due to yesno in @run main/manual=yesno"
>> 2) After removing yes/no test run( just shows the UI and gets dispose immediately). User cannot interact with the test UI and mark the test pass or failed. 
>> So added Pass and Fail button to mark the test result.
>> 3) Added timeout if in case user does not interact with the test UI.
>> 
>> @shurymury 
>> @aivanov-jdk
>
> lawrence.andrews has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update PassFailJFrame.java

Changes requested by aivanov (Reviewer).

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

> 52:     private static final int COLUMNS = 40;
> 53:     private static final long TEST_TIMEOUT = 5;
> 54:     private static final String TITLE = "Test Instruction Frame";

Please put the constants above the latch and separate them with a blank line.

I'd put the constants in the order of parameters: title, timeout, rows, columns.

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

> 87:      *
> 88:      * @param title                title of the Frame.
> 89:      * @param instructions         instruction for the tester on how to test and what

_the instructions_ for the tester…

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

> 102:             InvocationTargetException {
> 103:         if (isEventDispatchThread()) {
> 104:             createUI(title, instructions, testTimeOutInMinutes, rows, columns);

Does it make sense to use shorter version like `testTimeout` or `timeout` instead of longer `testTimeoutInMinutes`?

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

> 191:      * @throws InterruptedException      exception thrown when thread is
> 192:      *                                   interrupted
> 193:      * @throws InvocationTargetException exception thrown while creating UI

_…Exception exception_ doesn't look good.

In the rendered javadoc it reads like: throws `InterruptedException` ... (followed by text). The text usually starts with the word *if* followed by the condition when the exception is thrown.

The first item was documented in a usual way. The second one should look something like: _if an exception is thrown while disposing of frames on EDT._

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

> 311:      */
> 312:     public static void forceFail() {
> 313:         latch.countDown();

You must set `failed` to `true`.

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

PR: https://git.openjdk.java.net/jdk/pull/8004