RFR: 8316412: javax/print tests fail without printer set up

Aleksey Shipilev shade at openjdk.org
Wed Sep 20 16:48:09 UTC 2023


On Tue, 19 Sep 2023 14:45:47 GMT, Michal Sobierski <duke at openjdk.org> wrote:

> For NullClipARGB we are throwing SkippedException only if instance of caught exception is PrinterException. All other exceptions are considered unrelated to printer configuration.
> 
> For CountPrintServices we want to throw SkippedException in the case when lpstat is not installed, where an IOException is caught in that case.
> 
> For ExceptionTest we want to throw SkippedException for all PrinterExceptions which were not caused by IndexOutOfBoundsException.
> 
> For rest of the tests if PrintService was not created instead RuntimeException we are throwing now SkippedException.
> 
> Tested on environment without printer installed.
> Test are passing with skipped exception as expected.
> 
> Passed: java/awt/print/PrinterJob/ExceptionTest.java
> Passed: java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java
> Passed: java/awt/print/PrinterJob/PrtException.java
> Passed: javax/print/attribute/AttributeTest.java
> Passed: javax/print/attribute/GetCopiesSupported.java
> Passed: javax/print/attribute/SidesPageRangesTest.java
> Passed: javax/print/attribute/SupportedPrintableAreas.java
> Passed: javax/print/PrintServiceLookup/CountPrintServices.java
> Passed: javax/print/CheckDupFlavor.java
> 
> Additionally unused imports have been removed.

The copyright years need to be adjusted a bit.


Copyright (c) 2007, Oracle and/or its affiliates. All rights reserved.


...should become:


Copyright (c) 2007, 2023, Oracle and/or its affiliates. All rights reserved.

Apart from stylistic comments, this looks good.

Looks good to me. Once OCA clears, someone familiar with this area would need to take a look too.

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 43:

> 41: 
> 42:     public static void main( String[] args ) {
> 43:         PrintService[] svc = PrintServiceLookup.lookupPrintServices(DocFlavor.SERVICE_FORMATTED.PRINTABLE, null);

Do we need to ask for `DocFlavor.SERVICE_FORMATTED.PRINTABLE` here? Can we just ask for `null`?

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 43:

> 41:         try {
> 42:             PrinterJob pj = PrinterJob.getPrinterJob();
> 43:             pj.setPrintable(new NullClipARGB());

I see this is a cleanup, but we don't need to do it here, to keep patch simple and backportable.

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 47:

> 45:         } catch (Exception ex) {
> 46:             if (ex instanceof PrinterException) {
> 47:                 throw new SkippedException("Printer is required for this test.  TEST ABORTED");

I think for this test, we need to check `PrintServiceLookup.lookupPrintServices(DocFlavor.SERVICE_FORMATTED.PRINTABLE, null);`, like other tests do it. Let's not assume that `PrinterException` is thrown here only when printer is not configured. Add the block right at the beginning of `main`?

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 53:

> 51:             pj.print();
> 52:             } catch (Exception ex) {
> 53:             throw new RuntimeException(ex);

One more redundant whitespace change.

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 58:

> 56: 
> 57:     public int print(Graphics g, PageFormat pf, int pageIndex)
> 58:             throws PrinterException{

Redundant whitespace change.

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 58:

> 56: 
> 57:     public int print(Graphics g, PageFormat pf, int pageIndex)
> 58:                 throws PrinterException{

One more redundant whitespace change.

test/jdk/java/awt/print/PrinterJob/ImagePrinting/NullClipARGB.java line 71:

> 69:         g2.drawString("This text should be visible through the image", 0, 20);
> 70:         BufferedImage bi = new BufferedImage(100, 100,
> 71:                 BufferedImage.TYPE_INT_ARGB );

Redundant whitespace change.

test/jdk/javax/print/CheckDupFlavor.java line 33:

> 31: 
> 32: import javax.print.*;
> 33: import javax.print.attribute.*;

Here and later, keep the imports in place, for the same reason: targeted backportable patch.

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

PR Review: https://git.openjdk.org/jdk/pull/15821#pullrequestreview-1633824490
PR Review: https://git.openjdk.org/jdk/pull/15821#pullrequestreview-1635371892
Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15821#pullrequestreview-1635892563
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331464422
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331465944
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1330467074
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331726090
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331462305
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331726158
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331462355
PR Review Comment: https://git.openjdk.org/jdk/pull/15821#discussion_r1331466547


More information about the client-libs-dev mailing list