RFR: 8339984: Open source AWT MenuItem related tests [v11]

Alexey Ivanov aivanov at openjdk.org
Thu Sep 19 15:57:39 UTC 2024


On Thu, 19 Sep 2024 04:56:13 GMT, Abhishek Kumar <abhiscxk at 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


More information about the client-libs-dev mailing list