RFR: 8320608: Many jtreg printing tests are missing the @printer keyword [v2]

Alexey Ivanov aivanov at openjdk.org
Thu Nov 30 17:22:21 UTC 2023


On Mon, 27 Nov 2023 20:20:38 GMT, Phil Race <prr at openjdk.org> wrote:

>> Many printing tests do not have the @printer keyword. This adds them to those that need it.
>> I also found one test that has nothing to do with printing in the print folder and moved it out.
>
> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8320608

test/jdk/java/awt/print/PageFormat/SmallPaperPrinting.java line 73:

> 71:           job.print();
> 72:       }
> 73:       catch (PrinterException e) {

Suggestion:

      } catch (PrinterException e) {

The `catch` keyword should be on the line with the closing brace of the `try {}` block.

It can be addressed in [JDK-8320671](https://bugs.openjdk.org/browse/JDK-8320671) when the test is updated.

test/jdk/java/awt/print/PrinterJob/EmptyFill.java line 72:

> 70:            return;
> 71:        }
> 72:        PrinterJob pj = PrinterJob.getPrinterJob();

It looks good, however, I'd move the blank so that the code groups logically:


       StreamPrintService svc = spfs[0].getPrintService(baos);
       if (svc == null) {
           return;
       }

       PrinterJob pj = PrinterJob.getPrinterJob();


Perhaps, the test should fail if `svc` happens to be `null`, after all it's quite unexpected if it's `null`, isn't it?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1410886948
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1410980732


More information about the client-libs-dev mailing list