On Mon, 27 Nov 2023 20:20:38 GMT, Phil Race <p...@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
