RFR: JDK-8329692 : Add more details to FrameStateTest.java test instructions

Abhishek Kumar abhiscxk at openjdk.org
Tue Apr 30 04:31:06 UTC 2024


On Mon, 29 Apr 2024 23:24:19 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

> For the following manual test, more instructions are added as to what to expect for "hide,iconify and show" vs "hide,iconify,show and restore" for clarity.

Since these lines are unchanged, can't add comments there.

1. AT L155, space after `=` is missing.
`icontst =new CreateFrame(cbIconState.getState(), cbResize.getState());`

2. In `CreateFrame` constructor, calling `pack()` is redundant as frame size is set by `setBounds(...)` method.

3. L24 to L31 may be removed. Consolidated summary can be defined in `jtreg summary tag`.

test/jdk/java/awt/Frame/FrameStateTest/FrameStateTest.java line 91:

> 89: 
> 90:         Do the above steps for all the different Frame state combinations available.<br>
> 91:         For "hide,iconify and show" case, the frame is hidden then iconified hence Window2

Minor: 
Suggestion:

        For "hide, iconify and show" case, the frame is hidden then iconified hence Window2

test/jdk/java/awt/Frame/FrameStateTest/FrameStateTest.java line 94:

> 92:         is not seen on-screen when shown as the frame is still in the ICONIFIED state.
> 93:         Window2 is visible on-screen when it is restored to NORMAL state as observed with
> 94:         "hide,iconify,show and restore" case.<br><br>

Suggestion:

        "hide, iconify, show and restore" case.<br><br>

test/jdk/java/awt/Frame/FrameStateTest/FrameStateTest.java line 112:

> 110:     CheckboxGroup cbgState = new CheckboxGroup();
> 111:     CheckboxGroup cbgResize = new CheckboxGroup();
> 112:     Checkbox cbIconState = new Checkbox("Frame state ICONIFIED", cbgState, true);

Suggestion:

    Checkbox cbIconState = new Checkbox("Frame State ICONIFIED", cbgState, true);


How about capitalize each word for Checkbox text ?

test/jdk/java/awt/Frame/FrameStateTest/FrameStateTest.java line 122:

> 120:         PassFailJFrame
> 121:                 .builder()
> 122:                 .title("GetBoundsResizeTest Instructions")

Looks like title is mismatched with the test. Should it be "Frame State and Size Test Instructions" ?

test/jdk/java/awt/Frame/FrameStateTest/FrameStateTest.java line 326:

> 324:         public void stateLog(String message) {
> 325:             PassFailJFrame
> 326:                 .log("[Current State=%d] %s %s".formatted(getState(), name, message));

Suggestion:

                .log("[Current State = %d] %s %s".formatted(getState(), name, message));

test/jdk/java/awt/Frame/FrameStateTest/FrameStateTest.java line 330:

> 328: 
> 329:         public void stateLog() {
> 330:             PassFailJFrame.log("[Current State=" + getState() + "]");

Suggestion:

            PassFailJFrame.log("[Current State = " + getState() + "]");

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

PR Review: https://git.openjdk.org/jdk/pull/19008#pullrequestreview-2030155210
PR Review Comment: https://git.openjdk.org/jdk/pull/19008#discussion_r1584104123
PR Review Comment: https://git.openjdk.org/jdk/pull/19008#discussion_r1584104632
PR Review Comment: https://git.openjdk.org/jdk/pull/19008#discussion_r1584098207
PR Review Comment: https://git.openjdk.org/jdk/pull/19008#discussion_r1584116502
PR Review Comment: https://git.openjdk.org/jdk/pull/19008#discussion_r1584106094
PR Review Comment: https://git.openjdk.org/jdk/pull/19008#discussion_r1584106363


More information about the client-libs-dev mailing list