RFR: 8320443: [macos] Test java/awt/print/PrinterJob/PrinterDevice.java fails on macOS [v2]

[Alexey Ivanov]

On Mon, 4 Dec 2023 23:31:47 GMT, Phil Race <prr at openjdk.org> wrote:

>> For as long as we've had the macOS port, it seems that queries on the GraphicsConfiguration associated with
>> a printer would give you null for the defaultTransform.
>> Clearly this API isn't a popular one to call in such a context else we'd have had lots of reports.
>> We have a test that would have caught it except that until recently the macOS printing implementation
>> was swallowing exceptions it should not.
>
> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8320443

Could you also update the copyright year in `CPrinterJob.java` and `PrinterDevice.java`?

src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 2096:

> 2094: 
> 2095:     protected synchronized void setGraphicsConfigInfo(AffineTransform at,
> 2096:                                             double pw, double ph) {

Suggestion:

    protected synchronized void setGraphicsConfigInfo(AffineTransform at,
                                                      double pw, double ph) {

To preserve alignment.

test/jdk/java/awt/print/PrinterJob/PrinterDevice.java line 30:

> 28:  * @summary Checks that the PrinterGraphics is for a Printer GraphicsDevice.
> 29:  * Test doesn't run unless there's a printer on the system.
> 30:  * @key printer

Minor: we agreed to put `@key` after `@bug`.

test/jdk/java/awt/print/PrinterJob/PrinterDevice.java line 52:

> 50: public class PrinterDevice implements Printable {
> 51: 
> 52:     static volatile boolean failed = false;

Is it really needed? In all the cases where you assign `true` to the `failed` field, you also explicitly throw an exception, which is enough to fail the test.

test/jdk/java/awt/print/PrinterJob/PrinterDevice.java line 71:

> 69:         if (failed) {
> 70:             throw new RuntimeException("Test failed but no exception propagated.");
> 71:         }

A comment that `pj.print` should not throw exception would suffice, even though it's implied by jtreg any way.

This statement is essentially unreachable if `failed` is set to `true`.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16773#pullrequestreview-1765958594
PR Review Comment: https://git.openjdk.org/jdk/pull/16773#discussion_r1416186891
PR Review Comment: https://git.openjdk.org/jdk/pull/16773#discussion_r1416198629
PR Review Comment: https://git.openjdk.org/jdk/pull/16773#discussion_r1416195194
PR Review Comment: https://git.openjdk.org/jdk/pull/16773#discussion_r1416196288