On Sat, 16 Apr 2022 23:02:13 GMT, lawrence.andrews <d...@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
