On Mon, 11 Mar 2024 09:41:47 GMT, Alexander Zuev <kiz...@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
