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