RFR: 8320677: Printer tests use invalid '@run main/manual=yesno [v2]

[Alexey Ivanov]

On Wed, 22 Oct 2025 10:35:20 GMT, Anass Baya <abaya at openjdk.org> wrote:

>> The tests : 
>>   test/jdk/java/awt/print/PrinterJob/PolylinePrintingTest.java
>>   test/jdk/java/awt/print/PrinterJob/PageRanges.java      
>>   
>>   were failing because the Argument yesno to 'manual' option is invalid 
>> the fix is as follow : 
>> - Removed yesno
>> - Used PassFailJFrame manual test framework 
>> - add the missing keyboard  'test' to PolylinePrintingTest.java
>
> Anass Baya has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Verify printer availability at the beginning of the test execution
>  - Using the PolylinePrintingTest builder

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/print/PrinterJob/PageRanges.java line 49:

> 47:                  In the first dialog, select a page range of 2 to 3, and press OK
> 48:                  In the second dialog, select ALL, to print all pages (in total 5 pages).
> 49:                  Collect the two print outs and confirm the jobs printed correctly.

Suggestion:

                 This test prints two jobs and tests that the specified range
                 of pages is printed.
                 In the first dialog, select a page range of 2 to 3, and press OK.
                 In the second dialog, select ALL, to print all pages (in total 5 pages).
                 Collect the two print outs and confirm the jobs are printed correctly.


Now the test won't start if there's no printer.

test/jdk/java/awt/print/PrinterJob/PageRanges.java line 54:

> 52:     public static void main(String args[]) throws Exception {
> 53:         PrinterJob job = PrinterJob.getPrinterJob();
> 54:         if(job.getPrintService() == null) {

Suggestion:

        if (job.getPrintService() == null) {

test/jdk/java/awt/print/PrinterJob/PageRanges.java line 60:

> 58:         PassFailJFrame passFailJFrame = PassFailJFrame.builder()
> 59:                 .instructions(INSTRUCTIONS)
> 60:                 .rows((int) INSTRUCTIONS.lines().count() + 2)

This is the default value for `.rows`, thus, it can be omitted.

test/jdk/java/awt/print/PrinterJob/PageRanges.java line 78:

> 76:                      throws PrinterException {
> 77: 
> 78:         if (pi >= 5) {

At the same time, I dislike blank lines at the very start of method, they serve no purpose.
Suggestion:

                     throws PrinterException {
        if (pi >= 5) {

test/jdk/java/awt/print/PrinterJob/PageRanges.java line 83:

> 81: 
> 82:         g.drawString("Page : " + (pi+1), 200, 200);
> 83:         return PAGE_EXISTS;

I would keep the blank line here: it separates the body of the printing (drawing a string) from the return value. Although, in such a short method it's not as needed.

test/jdk/java/awt/print/PrinterJob/PolylinePrintingTest.java line 47:

> 45:     private static final String INSTRUCTIONS = """
> 46:               You must have a printer available to perform this test.
> 47:               OK the print dialog, and collect the printed page.

Suggestion:

              Click OK in the print dialog and collect the printed page.

test/jdk/java/awt/print/PrinterJob/PolylinePrintingTest.java line 62:

> 60: 
> 61:         passFailJFrame.awaitAndCheck();
> 62:     }

This second test may also throw `jtreg.SkippedException` if the printer is not available. Then you can remove this statement from the instructions.

I would also place the test code inside the main method as it's done in the `PageRanges.java` test.

I also think setting the page format and paper isn't necessary to reproduce the original bug [JDK-8041902](https://bugs.openjdk.org/browse/JDK-8041902), so this can be removed from the test, which will make the code small enough to inline into the main method.

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

PR Review: https://git.openjdk.org/jdk/pull/27916#pullrequestreview-3365146703
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451585936
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451589536
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451592735
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451608653
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451601250
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451615514
PR Review Comment: https://git.openjdk.org/jdk/pull/27916#discussion_r2451662295