RFR: 8316146: Open some swing tests 4 [v3]

[Alexey Ivanov]

On Mon, 25 Sep 2023 22:32:05 GMT, Alisen Chung <achung at openjdk.org> wrote:

>> Opening closed tests:
>> 12 javax/swing/ToolTipManager/5078214/bug5078214.java
>> 13 javax/swing/plaf/basic/BasicMenuItemUI/4239714/bug4239714.java
>> 14 javax/swing/plaf/basic/BasicMenuUI/4244616/bug4244616.java
>> 15 javax/swing/plaf/metal/4306431/bug4306431.java
>
> Alisen Chung has updated the pull request incrementally with one additional commit since the last revision:
> 
>   updated tests based on feedback

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/BasicMenuUI/bug4244616.java line 41:

> 39: public class bug4244616 {
> 40:     static PrintStream oldOut;
> 41:     static PrintStream oldErr;

Declare them before the try block, there's no need for them to be fields. See below.

test/jdk/javax/swing/BasicMenuUI/bug4244616.java line 61:

> 59: 
> 60:             oldOut = System.out;
> 61:             oldErr = System.err;

// Redirect streams
        final PrintStream oldOut = System.out;
        final PrintStream oldErr = System.err;
        try (ByteArrayOutputStream bout = new ByteArrayOutputStream();
             ByteArrayOutputStream berr = new ByteArrayOutputStream();
             PrintStream out = new PrintStream(bout);
             PrintStream err = new PrintStream(berr)) {

             // Perform the test
        } finally {
            // Restore streams
            System.setOut(oldOut);
            System.setErr(oldErr);
        }

test/jdk/javax/swing/ToolTipManager/bug5078214.java line 58:

> 56:     static volatile Rectangle bounds;
> 57:     static volatile Insets insets;
> 58:     static volatile JRobot r;

Robot is created and used on the main thread only, it doesn't need to be `volatile`. You should rename it to `robot`.

test/jdk/javax/swing/ToolTipManager/bug5078214.java line 147:

> 145:         });
> 146: 
> 147:     }

As I said, there's absolutely no reason to call `GraphicsEnvironment` methods on EDT — these are not Swing components.

There's no reason to store any of these as static fields, they are local variables.

Suggestion:

    private static GraphicsConfiguration getGraphicsConfig() {
        GraphicsDevice[] devices = GraphicsEnvironment.getLocalGraphicsEnvironment()
                                                      .getScreenDevices();
        for (GraphicsDevice device : devices) {
            GraphicsConfiguration[] gc = device.getConfigurations();
            for (int i = 0; i < gc.length; i++) {
                insets = Toolkit.getDefaultToolkit().getScreenInsets(gc[i]);
                if (insets.bottom != 0) {
                    return gc[i];
                }
            }
        }
        return null;
    }


You call this function from the `main` method.

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

PR Review: https://git.openjdk.org/jdk/pull/15875#pullrequestreview-1644408576
PR Review Comment: https://git.openjdk.org/jdk/pull/15875#discussion_r1337294239
PR Review Comment: https://git.openjdk.org/jdk/pull/15875#discussion_r1337301304
PR Review Comment: https://git.openjdk.org/jdk/pull/15875#discussion_r1337327968
PR Review Comment: https://git.openjdk.org/jdk/pull/15875#discussion_r1337319542