On Mon, 25 Sep 2023 22:32:05 GMT, Alisen Chung <[email protected]> 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
