RFR: 8339906: Open source several AWT focus tests - series 4 [v4]

[Prasanta Sadhukhan]

On Fri, 20 Sep 2024 04:02:12 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   typo
>
> test/jdk/java/awt/Focus/AltTabEventsTest.java line 61:
> 
>> 59:            2. Press Alt-tab.
>> 60:            In the messqge dialog area you should see that
>> 61:            WINDOW_DEACTIVATED,WINDOW_LOST_FOCUS event were generated.
> 
> Suggestion:
> 
>            WINDOW_DEACTIVATED, WINDOW_LOST_FOCUS event were generated.

ok

> test/jdk/java/awt/Focus/AltTabEventsTest.java line 80:
> 
>> 78:                 .rows((int) INSTRUCTIONS.lines().count() + 5)
>> 79:                 .columns(35)
>> 80:                 .testUI(AltTabEventsTest::createTestUI)
> 
> can be changed to `new Test();` and thus leading to remove the `createTestUI()` method.

ok

> test/jdk/java/awt/Focus/AltTabEventsTest.java line 104:
> 
>> 102:         WindowAdapter wa = new WindowAdapter() {
>> 103:                 public void windowActivated(WindowEvent e) {
>> 104:                     println(e.toString());
> 
> can be replaced with `PassFailJFrame.log` everywhere. Anyways there is a repetition of `println`.
> Suggestion:
> 
>                     PassFailJFrame.log(e.toString());

it will be same..

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 54:
> 
>> 52: 
>> 53:     public ComponentLostFocusTest() {
>> 54:         try{
> 
> Suggestion:
> 
>         try {

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 56:
> 
>> 54:         try{
>> 55:             r = new Robot();
>> 56:         } catch(Exception e){
> 
> Suggestion:
> 
>         } catch (Exception e) {

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 69:
> 
>> 67:         });
>> 68: 
>> 69:         frame.setLayout (new FlowLayout ());
> 
> Suggestion:
> 
>         frame.setLayout (new FlowLayout());

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 79:
> 
>> 77:     public void doTest() {
>> 78:         // Do requesting focus to the modal dialog in order to after that
>> 79:         // to do requesting focus to the frame
> 
> Doesn't sound correct, can it be rephrased?

removed

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 103:
> 
>> 101:         tf.addFocusListener(new FocusAdapter() {
>> 102:             public void focusGained(FocusEvent e) {
>> 103:                 System.out.println("TextField gained focus: "+e);
> 
> Suggestion:
> 
>                 System.out.println("TextField gained focus: " + e);

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 111:
> 
>> 109: 
>> 110:         System.out.println("Focused window: " + KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusedWindow());
>> 111:         System.out.println("Focus owner: " + KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusOwner());
> 
> please limit it to 80 cols.

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 118:
> 
>> 116:     }
>> 117: 
>> 118:     private void doRequestFocusToTextField(){
> 
> Suggestion:
> 
>     private void doRequestFocusToTextField() {

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 121:
> 
>> 119:         // do activation using press title
>> 120:         Point loc = frame.getLocationOnScreen();
>> 121:         r.mouseMove(loc.x + frame.getWidth()/2, loc.y + frame.getInsets().top/2);
> 
> Suggestion:
> 
>         r.mouseMove(loc.x + frame.getWidth() / 2, loc.y + frame.getInsets().top / 2);

ok

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 121:
> 
>> 119:         // do activation using press title
>> 120:         Point loc = frame.getLocationOnScreen();
>> 121:         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..

> test/jdk/java/awt/Focus/ComponentLostFocusTest.java line 131:
> 
>> 129:     }
>> 130: 
>> 131:     public static final void main(String args[]){
> 
> Suggestion:
> 
>     public static final void main(String args[]) {

ok

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992547
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992609
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992742
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993053
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992807
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992856
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767992946
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993001
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993094
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993136
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993175
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993524
PR Review Comment: https://git.openjdk.org/jdk/pull/21051#discussion_r1767993207