On Fri, 6 Jan 2023 07:21:12 GMT, Manukumar V S <[email protected]> wrote:
>> java/awt/PopupMenu/PopupMenuLocation.java seems to be unstable in MacOS
>> machines, especially in MacOSX 12 & 13 machines. It seems to be a testbug as
>> adding some stability improvements fixes the issue. It intermittently fails
>> in CI causing some noise. This test was already problem listed in windows
>> due to an umbrella bug JDK-8238720. This fix doesn't cover the Windows
>> issue, so the problem listing in windows will remain same.
>>
>> Fix:
>> Some stability improvements have been done and the test has been run 100
>> times per platform in mach5 and got full PASS.
>> Also updated the screen capture code, made frame undecorated, added some
>> prints for better debugging.
>
> Manukumar V S has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Review comments fixed: Replaced one robot.waitForIdle with robot.delay(),
> formatting corrections, taken the insets into account for initial frame
> position
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 77:
> 75: while (point.x <
> 76: screenBounds.x + screenBounds.width - insets.right
> -
> 77: SIZE) {
Suggestion:
final int yBound = screenBounds.y + screenBounds.height
- insets.bottom - SIZE;
final int xBound = screenBounds.x + screenBounds.width
- insets.right - SIZE;
while (point.y < yBound) {
while (point.x < xBound) {
I think this looks clearer and more readable than the wrapped condition.
test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 87:
> 85: }
> 86:
> 87: private static void test(final Point tmp) {
Suggestion:
private static void test(final Point loc) {
Let's give a better name to the parameter.
test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 104:
> 102: frame.setSize(SIZE, SIZE);
> 103: frame.setVisible(true);
> 104: frame.setLocation(tmp.x, tmp.y);
Suggestion:
frame.setLocation(tmp);
frame.setVisible(true);
You can pass a `Point` object to `setLocation` directly.
Setting the location before making the frame visible avoids flickering where
the frame appears at 0, 0 and then moves to another location.
-------------
PR: https://git.openjdk.org/jdk/pull/10655
