RFR: 8316388: Opensource five Swing component related regression tests

Mon Mar 11 14:17:59 UTC 2024

On Mon, 11 Mar 2024 09:41:47 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

> Clean up five more tests.
> 
> test/jdk/javax/swing/JDesktopPane/4132993/bug4132993.java
> test/jdk/javax/swing/JDesktopPane/4773378/bug4773378.java
> test/jdk/javax/swing/JEditorPane/4325606/bug4325606.java
> test/jdk/javax/swing/JEditorPane/4330998/bug4330998.java
> test/jdk/javax/swing/JEditorPane/4694598/FrameContent.html
> test/jdk/javax/swing/JEditorPane/4694598/bug4694598.java

The copyright year should be updated to 2024.

The directory structure should be flattened, there's no need for additional folder in the path.

test/jdk/javax/swing/JDesktopPane/4132993/bug4132993.java line 47:

> 45:             } catch (PropertyVetoException ex) {
> 46:                 ex.printStackTrace();
> 47:             }

Shouldn't the test fail if an unexpected exception is thrown?

test/jdk/javax/swing/JDesktopPane/4132993/bug4132993.java line 59:

> 57:             throw new RuntimeException("Test interrupted by "
> 58:                     + e.getLocalizedMessage());
> 59:         }

You can add `throws Exception` clause to the `main` method and remove this `catch` block.

test/jdk/javax/swing/JDesktopPane/4773378/bug4773378.java line 69:

> 67:                         keyTyped = true;
> 68:                         bug4773378.this.notifyAll();
> 69:                     }

A `CountDownLatch` would work great in this case.

And `keyTyped` is a confusing name for a flag which gets to `true` when the internal frame is activated.

test/jdk/javax/swing/JDesktopPane/4773378/bug4773378.java line 85:

> 83:     public void performTest() {
> 84:         try {
> 85:             jif.setSelected(true);

This should be called via `SwingUtilities.invokeLater` on EDT.

test/jdk/javax/swing/JDesktopPane/4773378/bug4773378.java line 102:

> 100:         } catch (Throwable t) {
> 101:             t.printStackTrace();
> 102:         }

A `Throwable` should fail the test, it must be propagated.

test/jdk/javax/swing/JEditorPane/4325606/bug4325606.java line 83:

> 81:             robo.setAutoDelay(100);
> 82:             robo.delay(1000);
> 83:             robo.mouseMove(100,100);

This may fail if the frame location of (50, 50) isn't respected by a window manager.

test/jdk/javax/swing/JEditorPane/4325606/bug4325606.java line 115:

> 113:         try {
> 114:             SwingUtilities.invokeAndWait(b::setupGUI);
> 115:             safeSleep(3000);

Instead of sleeping, you can use a `CountDownLatch(3)` which you'll `countDown()` for each mouse click. Here you'll call `await(3, TimeUnit.SECONDS)` and throw a timeout error if `await` returns `false`.

Another synchroniser may be used to handle the case where `BadLocationException` is thrown to fail the test right away. Alternatively, `BadLocationException` may be wrapped into `RuntimeException` and re-thrown.

test/jdk/javax/swing/JEditorPane/4694598/FrameContent.html line 1:

> 1: <HTML><BODY>

Can this file be created by the test on the fly? There's no need for a supporting file.

In #18147, Prasanta handled the same situation.

test/jdk/javax/swing/JEditorPane/4694598/bug4694598.java line 29:

> 27:  * @summary JEditor pane throws NullPointerException on mouse movement.
> 28:  * @library ../../regtesthelpers
> 29:  * @build JRobot

The test doesn't seem to use any of the extended functionality provided by JRobot, so the standard `java.awt.Robot` can be used instead.

test/jdk/javax/swing/JEditorPane/4694598/bug4694598.java line 40:

> 38: 
> 39: public class bug4694598 {
> 40:     JFrame frame = null;

Suggestion:

    JFrame frame;

`null` is the default value any way.

test/jdk/javax/swing/JEditorPane/4694598/bug4694598.java line 69:

> 67:     public void performTest() throws InterruptedException,
> 68:             InvocationTargetException {
> 69:         JRobot jRobo = JRobot.getRobot();

Suggestion:

        JRobot jRobo = JRobot.getRobot();
        jRobo.waitForIdle();

Let the frame appear on the screen.

test/jdk/javax/swing/JEditorPane/4694598/bug4694598.java line 81:

> 79:             try {
> 80:                 Thread.sleep(50);
> 81:             } catch (InterruptedException ex) {}

Suggestion:

            jRobo.delay(50);

test/jdk/javax/swing/JEditorPane/4694598/bug4694598.java line 93:

> 91: 
> 92:     public static void main(String args[]) throws InterruptedException,
> 93:             InvocationTargetException {

Suggestion:

    public static void main(String[] args) throws Exception {

Java-style array declaration, shortened `throws` clause.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18184#pullrequestreview-1927877446
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519748640
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519751913
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519757762
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519759741
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519764367
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519773893
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519778086
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519785057
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519790641
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519791220
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519789098
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519787470
PR Review Comment: https://git.openjdk.org/jdk/pull/18184#discussion_r1519793301