RFR: 8334509: Cancelling PageDialog does not return the same PageFormat object

Abhishek Kumar abhiscxk at openjdk.org
Wed Jun 19 06:52:12 UTC 2024


On Wed, 19 Jun 2024 05:34:55 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> On cancelling PageDialog, same PageFormat object should be returned which stopped working after [JDK-8307160](https://bugs.openjdk.org/browse/JDK-8307160).
> Fix is made to reinstate "doIt" flag removed in JDK-8307160 so that correct value is returned from PageDialog.show action..
> An automated printing testcase is created since the issue was caught by manual test and so having another manual test run the risk of not being executed during CI testing..

test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 28:

> 26:  * @bug 8334366
> 27:  * @key printer
> 28:  * @summary Verifies oriignal pageobject is returned unmodified

Suggestion:

 * @summary Verifies original pageobject is returned unmodified

test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 46:

> 44:         Robot robot = new Robot();
> 45:         Thread t1 = new Thread(() -> {
> 46:             robot.delay(2000);

usually we keep a delay of 1000 ms, any specific reason for longer delay?

test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 51:

> 49:                 robot.keyRelease(KeyEvent.VK_TAB);
> 50:                 robot.waitForIdle();
> 51:                 robot.delay(500);

I think shorter delay of 100ms should be ok here and below as well.

test/jdk/java/awt/print/PrinterJob/PageDialogCancelTest.java line 64:

> 62:             System.out.println("Test Passed");
> 63:         } else {
> 64:             throw new RuntimeException("Original PageFormat not returned on cancelling PageDialog");

`result` variable may not be required as  `newFormat.equals(oldFormat)` can be evaluated in if condition. I guess there is no need to log "Test Passed", we can check for the failure condition and throw the run time exception.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645468790
PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645471727
PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645500851
PR Review Comment: https://git.openjdk.org/jdk/pull/19786#discussion_r1645477497


More information about the client-libs-dev mailing list