Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8238436: java/awt/Frame/FrameLocationTest/FrameLocationTest.java fails #527

Closed
wants to merge 1 commit into from

Conversation

pankaj-bansal
Copy link

@pankaj-bansal pankaj-bansal commented Oct 6, 2020

The test fail intermittently on all platforms, though issue is filled for only Ubuntu.
The test generated random numbers and sets the frame location to those random numbers. There are two issues in the test.

  1. The test has logic to make sure maximum location coordinates are such that the frame will always fit in the screen. There is no logic to set the minimum possible location for frame because of taskbar. Eg, on my Mac, if the y coordinate is < 23, the frame y location is set to 23 on frame.setLocation(), so the test will fail. The minimum x,y location coordinate will depend upon the platform.
  2. The test is setting the frame location and then verifying the location in a loop, but there is no wait/delay added to let the frame.setLocation complete.

The fix sets the minimum location coordinates possible for the frame by using the frame location it is shown first time. Also, Robot is added to use delay/wait to let setLocation complete. The mach5 passes after the fix with multiple runs of the test. Link in JBS.

The test is moved from closed to open repo, so looks like a new test.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Testing

Linux x64 Windows x64 macOS x64
Build ✔️ (3/3 passed) ✔️ (2/2 passed) ✔️ (2/2 passed)
Test (tier1) ✔️ (9/9 passed) ❌ (1/9 failed) ✔️ (9/9 passed)

Failed test task

Issue

  • JDK-8238436: java/awt/Frame/FrameLocationTest/FrameLocationTest.java fails

Download

$ git fetch https://git.openjdk.java.net/jdk pull/527/head:pull/527
$ git checkout pull/527

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 6, 2020

👋 Welcome back pbansal! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Oct 6, 2020
@openjdk
Copy link

openjdk bot commented Oct 6, 2020

@pankaj-bansal The following label will be automatically applied to this pull request:

  • awt

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the awt client-libs-dev@openjdk.org label Oct 6, 2020
@mlbridge
Copy link

mlbridge bot commented Oct 6, 2020

Webrevs

@mrserb
Copy link
Member

mrserb commented Oct 6, 2020

The test is setting the frame location and then verifying the location in a loop, but there is no wait/delay added to let the >frame.setLocation complete.
It is intentionally was implemented that way, the setLocation sets the property "location" for the frame, it is stored in the frame itself, the getLocation() method if called immediately after setLocation() should return that property.

@pankaj-bansal
Copy link
Author

The test is setting the frame location and then verifying the location in a loop, but there is no wait/delay added to let the >frame.setLocation complete.
It is intentionally was implemented that way, the setLocation sets the property "location" for the frame, it is stored in the frame itself, the getLocation() method if called immediately after setLocation() should return that property.

ok, so we should not add delay after setLocation. But then the frame is not repainted at new location at all. The test will just set and get the location without actually moving the frame to the location. The test is unstable in this scenario.

@mrserb
Copy link
Member

mrserb commented Oct 6, 2020

ok, so we should not add delay after setLocation. But then the frame is not repainted at new location at all. The test will just set and get the location without actually moving the frame to the location. The test is unstable in this scenario.

If it just sets/gets location then why it is unstable?

@pankaj-bansal pankaj-bansal deleted the JDK-8238436 branch October 13, 2020 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awt client-libs-dev@openjdk.org rfr Pull request is ready for review
2 participants