On Wed, 19 Jun 2024 05:34:55 GMT, Prasanta Sadhukhan <psadhuk...@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
