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