RFR: JDK-8290469: Add new positioning options to PassFailJFrame test framework [v8]

Phil Race prr at openjdk.org
Sun Aug 21 22:12:33 UTC 2022


On Thu, 11 Aug 2022 22:27:39 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> Additional position setting (TOP_LEFT_CORNER) and a method to obtain bounds of test instruction frame are added to PassFailJFrame to handle positioning of multiple test frames.
>> 
>> In scenarios where multiple test windows might be present, the test windows might overlap the instruction frame. In order to fix this TOP_LEFT_CORNER position option is added that positions the test instruction frame at top left corner with main test window below it.
>> 
>> Additionally `getInstructionFrameBounds()` is added to obtain the position and dimensions of test instruction frame.
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review changes

A few comments which are mostly (or all?) nit picking about the javadoc of the method.

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

> 263:     /**
> 264:      * Position the instruction frame with testWindow (testcase created
> 265:      * window) by the specified position. If testWindow is null, only

The pre-existing docs don't read very well to me and some of it doesn't make sense, so in this update
we should try to fix that.

First sentence would be better as 
"Positions the instruction frame relative to the test window as specified by the {@code position} parameter.

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

> 267:      * parameter.
> 268:      * Note: This method should be invoked from the method that creates
> 269:      * testWindow. At test-level, the testWindow must be made visible

I think we should remove that Note sentence - surely the calling method is irrelevant. 
 It is sufficient to explain that 
"This method should be called before making the test window visible".
But why is that ?

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

> 270:      * only after calling this method.
> 271:      *
> 272:      * @param testWindow test window that the test is created. It can be null.

"It can be null" -> "May be {@code null}"

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

> 271:      *
> 272:      * @param testWindow test window that the test is created. It can be null.
> 273:      * @param position  position can either be:

position must be one of

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

> 276:      *                  center and the test window (if not null) is placed to
> 277:      *                  the right of the instruction frame.
> 278:      *

"vertical center" means half way between the top and the bottom which is absolutely not what you mean, is it ?

horizontal center surely ?

But what if the test window is > 0.5 the width of the screen ? Now it is off the edge .. wouldn't it be better
to move the instruction window to the left to accommodate it ?
Although that might be tricky if we don't know its size yet ..

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

PR: https://git.openjdk.org/jdk/pull/9525



More information about the client-libs-dev mailing list