On Thu, 19 Sep 2024 04:56:13 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:
>> Few AWT MenuItem related tests are converted from applet to manual and moved
>> to open.
>
> Abhishek Kumar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Test restructured and test frames set visible true
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/MenuItem/LotsOfMenuItemsTest.java line 44:
> 42: private static final int NUM_WINDOWS = 400;
> 43: private static TestFrame firstFrame;
> 44: private static TestFrame testFrame;
`testFrame` field is no longer needed, it can be a local variable in
`componentShown`.
test/jdk/java/awt/MenuItem/LotsOfMenuItemsTest.java line 77:
> 75: public void componentShown(ComponentEvent e) {
> 76: for (int i = 1; i < NUM_WINDOWS - 1; ++i) {
> 77: testFrame = new TestFrame("Running(" + i + ")...");
Currently the created frames flicker at the upper left upper corner of the
screen, I suggest displaying them below the first frame:
public void componentShown(ComponentEvent e) {
final int x = firstFrame.getX();
final int y = firstFrame.getY() + firstFrame.getHeight() + 8;
for (int i = 1; i < NUM_WINDOWS - 1; ++i) {
testFrame = new TestFrame("Running(" + i + ")...", x, y);
You'll create the last frame using the same constructor as above in the loop.
I find this way less distracting, and I can read the instructions while the
test continues to perform its _initialisation_.
test/jdk/java/awt/MenuItem/LotsOfMenuItemsTest.java line 83:
> 81: testFrame = new TestFrame("Last Frame");
> 82: testFrame.setLocation(firstFrame.getX(),
> 83: firstFrame.getY() + firstFrame.getHeight() + 8);
I mean, these two lines will become:
testFrame = new TestFrame("Last Frame", x, y);
test/jdk/java/awt/MenuItem/LotsOfMenuItemsTest.java line 94:
> 92: }
> 93:
> 94: public TestFrame(String title, boolean visible) {
You'll need a constructor which accepts `x` and `y`:
Suggestion:
public TestFrame(String title) {
this(title, 0, 0, false);
}
public TestFrame(String s, int x, int y) {
this(s, x, y, true);
}
private TestFrame(String title,
int x, int y,
boolean visible) {
Then you'll call `setLocation(x, y);` before calling `setSize`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21029#pullrequestreview-2315786971
PR Review Comment: https://git.openjdk.org/jdk/pull/21029#discussion_r1766976607
PR Review Comment: https://git.openjdk.org/jdk/pull/21029#discussion_r1766983802
PR Review Comment: https://git.openjdk.org/jdk/pull/21029#discussion_r1766985049
PR Review Comment: https://git.openjdk.org/jdk/pull/21029#discussion_r1766989912
