8339791: Refactor MiscUndecorated/ActiveAWTWindowTest.java #26471
Conversation
|
👋 Welcome back smandalika! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
Can someone take a quick look and provide feedback on this simple refactoring change ? |
| private final Object lock3 = new Object(); | ||
| private static final CountDownLatch windowActivatedLatch = new CountDownLatch(1); | ||
| private static final CountDownLatch windowDeactivatedLatch = new CountDownLatch(1); | ||
| private static final CountDownLatch windowFocusGainedLatch = new CountDownLatch(1); |
There was a problem hiding this comment.
I see where we decrement this latch but i do not see where we check it or wait on it. Is it really needed?
There was a problem hiding this comment.
also no need to use ExtendedRobot as all I can see is mousePress/mouseRelease which can be used from java.awt.Robot class itself..
There was a problem hiding this comment.
static final variables can be capitalized.
There was a problem hiding this comment.
Few more observations :
- Add a space wherever application. At L76...
try { - Don't think it is required to print stack trace, you can throw it in RTE.
- At L165, robot.waitForIdle(5 * delay);
- Access UI components on EDT. e.g L166 etc.
- Can remove
//captureScreenAndSave();andSystem.err.println("Test failed!");lines
| private final Object lock3 = new Object(); | ||
| private static final CountDownLatch windowActivatedLatch = new CountDownLatch(1); | ||
| private static final CountDownLatch windowDeactivatedLatch = new CountDownLatch(1); | ||
| private static final CountDownLatch windowFocusGainedLatch = new CountDownLatch(1); |
There was a problem hiding this comment.
static final variables can be capitalized.
| private final Object lock3 = new Object(); | ||
| private static final CountDownLatch windowActivatedLatch = new CountDownLatch(1); | ||
| private static final CountDownLatch windowDeactivatedLatch = new CountDownLatch(1); | ||
| private static final CountDownLatch windowFocusGainedLatch = new CountDownLatch(1); | ||
| private boolean passed = true; | ||
| private final int delay = 150; |
There was a problem hiding this comment.
| private final int delay = 150; | |
| private final int DELAY = 150; |
| @@ -93,13 +97,7 @@ private void initializeGUI() { | |||
| frame.addWindowFocusListener(new WindowFocusListener() { | |||
| public void windowGainedFocus(WindowEvent event) { | |||
There was a problem hiding this comment.
add @OverRide for all overridden methods.
The java/awt/Frame/MiscUndecorated/ActiveAWTWindowTest.java test uses object monitors and wait/notify to synchronise actions in the test.
Using CountDownLatch could make the test simpler, shorter, clearer.
Tested the code on a windows-x64, macos-x64 and lnux-x64 machines and the test is working as expected.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26471/head:pull/26471$ git checkout pull/26471Update a local copy of the PR:
$ git checkout pull/26471$ git pull https://git.openjdk.org/jdk.git pull/26471/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26471View PR using the GUI difftool:
$ git pr show -t 26471Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26471.diff
Using Webrev
Link to Webrev Comment