8339906: Open source several AWT focus tests - series 4 by prsadhuk · Pull Request #21051 · openjdk/jdk

prsadhuk · 2024-09-18T03:51:08Z

Opensource few Focus tests

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8339906: Open source several AWT focus tests - series 4 (Bug - P4)

Reviewers

Phil Race (@prrace - Reviewer) Review applies to 509ccf65
Abhishek Kumar (@kumarabhi006 - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21051/head:pull/21051
$ git checkout pull/21051

Update a local copy of the PR:
$ git checkout pull/21051
$ git pull https://git.openjdk.org/jdk.git pull/21051/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21051

View PR using the GUI difftool:
$ git pr show -t 21051

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21051.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-09-18T03:51:38Z

👋 Welcome back psadhukhan! 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 · 2024-09-18T03:52:25Z

@prsadhuk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8339906: Open source several AWT focus tests - series 4

Reviewed-by: abhiscxk, prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 83 new commits pushed to the master branch:

f4e4017: 8339984: Open source AWT MenuItem related tests
0f7d9e5: 8340436: Remove unused CompressedOops::AnyNarrowOopMode
94c3317: 8339895: Open source several AWT focus tests - series 3
969c2af: 8339192: Native annotation parsing code of deprecated annotations causes crash
fdc16a3: 8340480: Bad copyright notices in changes from JDK-8339902
296b496: 8340353: Remove CompressedOops::ptrs_base
fde8508: 8339902: Open source couple TextField related tests
75d5e11: 8340400: Shenandoah: Whitebox breakpoint GC requests may cause assertions
15ae115: 8340166: [REDO] CDS: Trim down minimum GC region alignment
ec3cba0: 8340399: Update comment in SourceVersion for language evolution history
... and 73 more: https://git.openjdk.org/jdk/compare/a0794e0a054c5e7ed051efa6362726cdd7598255...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2024-09-18T03:53:00Z

@prsadhuk The following label will be automatically applied to this pull request:

client

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.

mlbridge · 2024-09-18T03:56:42Z

Webrevs

prrace · 2024-09-19T22:50:22Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+ * @test
+ * @bug 4982943
+ * @key headful
+ * @summary focus lost in text fields or text areas, unable to enter characters from keyboar


keyboar -> keyboard

@prrace Fixed but it revokes the approval....Please reapprove or someone "R"eviewer awake in this TZ to approve this minor change..

kumarabhi006 · 2024-09-20T04:02:12Z

test/jdk/java/awt/Focus/AltTabEventsTest.java

+           1. Make F1 active by clicking on it.
+           2. Press Alt-tab.
+           In the messqge dialog area you should see that
+           WINDOW_DEACTIVATED,WINDOW_LOST_FOCUS event were generated.


Suggested change

WINDOW_DEACTIVATED,WINDOW_LOST_FOCUS event were generated.

WINDOW_DEACTIVATED, WINDOW_LOST_FOCUS event were generated.

kumarabhi006 · 2024-09-20T04:03:23Z

test/jdk/java/awt/Focus/AltTabEventsTest.java

+                .instructions(INSTRUCTIONS)
+                .rows((int) INSTRUCTIONS.lines().count() + 5)
+                .columns(35)
+                .testUI(AltTabEventsTest::createTestUI)


can be changed to new Test(); and thus leading to remove the createTestUI() method.

Looks like you missed this.

Sorry, it was a mistake on my side. Don't know why but updated test was not shown. It is updated now.

kumarabhi006 · 2024-09-20T04:04:28Z

test/jdk/java/awt/Focus/AltTabEventsTest.java

+        super("Test for 4524015");
+        WindowAdapter wa = new WindowAdapter() {
+                public void windowActivated(WindowEvent e) {
+                    println(e.toString());


can be replaced with PassFailJFrame.log everywhere. Anyways there is a repetition of println.

Suggested change

println(e.toString());

PassFailJFrame.log(e.toString());

it will be same..

println method can be removed if PassFailJFrame.log is used directly.

Then PassFailJFrame.log will be repeated, so it's same...dont think it's critical to change..

Yeah, not critical.

kumarabhi006 · 2024-09-20T04:05:49Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+    public ComponentLostFocusTest() {
+        try{
+            r = new Robot();
+        } catch(Exception e){


Suggested change

} catch(Exception e){

} catch (Exception e) {

kumarabhi006 · 2024-09-20T04:06:17Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+            }
+        });
+
+        frame.setLayout (new FlowLayout ());


Suggested change

frame.setLayout (new FlowLayout ());

frame.setLayout (new FlowLayout());

kumarabhi006 · 2024-09-20T04:10:31Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+        System.out.println("Focused window: " + KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusedWindow());
+        System.out.println("Focus owner: " + KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusOwner());


please limit it to 80 cols.

kumarabhi006 · 2024-09-20T04:10:40Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+        }
+    }
+
+    private void doRequestFocusToTextField(){


Suggested change

private void doRequestFocusToTextField(){

private void doRequestFocusToTextField() {

kumarabhi006 · 2024-09-20T04:11:05Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+    private void doRequestFocusToTextField(){
+        // do activation using press title
+        Point loc = frame.getLocationOnScreen();
+        r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);


Suggested change

r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);

r.mouseMove(loc.x + frame.getWidth() / 2, loc.y + frame.getInsets().top / 2);

kumarabhi006 · 2024-09-20T04:11:16Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+        tf.requestFocus();
+    }
+
+    public static final void main(String args[]){


Suggested change

public static final void main(String args[]){

public static final void main(String args[]) {

kumarabhi006 · 2024-09-20T04:11:52Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+        Point loc = frame.getLocationOnScreen();
+        r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);


Should it be accessed on EDT?

modified test to put relevant portion under EDT and also added dispose..

kumarabhi006 · 2024-09-20T05:28:47Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

        // do activation using press title
-        Point loc = frame.getLocationOnScreen();
-        r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);
+        r.mouseMove(loc.x + frame.getWidth() / 2, loc.y + frame.getInsets().top / 2);


frame.getWidth also should be on EDT ?

kumarabhi006 · 2024-09-20T05:30:23Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

+        r = new Robot();
+        r.setAutoDelay(100);
+
+        ComponentLostFocusTest test;


Unused variable.

kumarabhi006 · 2024-09-20T05:30:45Z

test/jdk/java/awt/Focus/ComponentLostFocusTest.java

-        r.setAutoDelay(100);
+    static Dialog dialog = null;
+    static volatile boolean passed;
+    static Point loc;


Should it be volatile also?

kumarabhi006

Hope you have run the automated test in CI.

prsadhuk · 2024-09-20T06:05:16Z

/integrate

openjdk · 2024-09-20T06:06:28Z

Going to push as commit 46b02f4.
Since your change was applied there have been 83 commits pushed to the master branch:

f4e4017: 8339984: Open source AWT MenuItem related tests
0f7d9e5: 8340436: Remove unused CompressedOops::AnyNarrowOopMode
94c3317: 8339895: Open source several AWT focus tests - series 3
969c2af: 8339192: Native annotation parsing code of deprecated annotations causes crash
fdc16a3: 8340480: Bad copyright notices in changes from JDK-8339902
296b496: 8340353: Remove CompressedOops::ptrs_base
fde8508: 8339902: Open source couple TextField related tests
75d5e11: 8340400: Shenandoah: Whitebox breakpoint GC requests may cause assertions
15ae115: 8340166: [REDO] CDS: Trim down minimum GC region alignment
ec3cba0: 8340399: Update comment in SourceVersion for language evolution history
... and 73 more: https://git.openjdk.org/jdk/compare/a0794e0a054c5e7ed051efa6362726cdd7598255...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-09-20T06:06:34Z

@prsadhuk Pushed as commit 46b02f4.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8339906: Open source several AWT focus tests - series 4

42accbb

openjdk bot added the rfr Pull request is ready for review label Sep 18, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Sep 18, 2024

Test automated

0d9821f

openjdk bot removed the rfr Pull request is ready for review label Sep 18, 2024

jcheck

509ccf6

openjdk bot added the rfr Pull request is ready for review label Sep 18, 2024

prrace approved these changes Sep 19, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 19, 2024

typo

c1a3daf

openjdk bot removed the ready Pull request is ready to be integrated label Sep 20, 2024

kumarabhi006 reviewed Sep 20, 2024

View reviewed changes

Formatting, EDT addition

4b6795d

kumarabhi006 reviewed Sep 20, 2024

View reviewed changes

prsadhuk added 2 commits September 20, 2024 06:35

EDT

fcf7620

volatile

2dcd760

kumarabhi006 approved these changes Sep 20, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 20, 2024

openjdk bot added the integrated Pull request has been integrated label Sep 20, 2024

openjdk bot closed this Sep 20, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 20, 2024

prsadhuk deleted the JDK-8339906 branch September 20, 2024 06:08

	WINDOW_DEACTIVATED,WINDOW_LOST_FOCUS event were generated.
	WINDOW_DEACTIVATED, WINDOW_LOST_FOCUS event were generated.

	frame.setLayout (new FlowLayout ());
	frame.setLayout (new FlowLayout());

		System.out.println("Focused window: " + KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusedWindow());
		System.out.println("Focus owner: " + KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusOwner());

	private void doRequestFocusToTextField(){
	private void doRequestFocusToTextField() {

	r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);
	r.mouseMove(loc.x + frame.getWidth() / 2, loc.y + frame.getInsets().top / 2);

	public static final void main(String args[]){
	public static final void main(String args[]) {

		Point loc = frame.getLocationOnScreen();
		r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);

Conversation

prsadhuk commented Sep 18, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Sep 18, 2024

Uh oh!

openjdk bot commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Sep 18, 2024

Uh oh!

mlbridge bot commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prsadhuk commented Sep 18, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Sep 18, 2024 •

edited

Loading

mlbridge bot commented Sep 18, 2024 •

edited

Loading