RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v3]

[Alexey Ivanov]

On Wed, 7 Feb 2024 17:16:55 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Disposed g2D object and similar test parm into one line
>
> test/jdk/java/awt/print/PrinterJob/Cancel/PrinterJobCancel.java line 57:
> 
>> 55:              "was properly cancelled.\n" +
>> 56:              "You will need to kill the application manually since regression\n" +
>> 57:              "tests apparently aren't supposed to call System.exit()";
> 
> The tester cannot see the console. The test must handle this situation and provide the feedback to the tester as required or fail the test automatically.
> 
> The tester should not terminate the application manually, the test must exit gracefully in either case: success or failure.
> 
> It could be reasonable to submit a new bug for this test only and use the changes in this PR as the starting point. I didn't analyse thoroughly the tests when I submitted this bug. You already deemed, there were too many tests for a single changeset.

Update the instructions to explain the test will continue automatically.

> test/jdk/java/awt/print/PrinterJob/ValidatePage/ValidatePage.java line 180:
> 
>> 178:         });
>> 179: 
>> 180:         Button pageButton = new Button("Page Setup..");
> 
> Suggestion:
> 
>         Button pageButton = new Button("Page Setup...");
> 
> Three dots are usually used to indicate a button opens a dialog to request more details.

It's still relevant.

> test/jdk/java/awt/print/PrinterJob/ValidatePage/ValidatePage.java line 195:
> 
>> 193:         });
>> 194: 
>> 195:         Button chooseButton = new Button("Printer..");
> 
> Suggestion:
> 
>         Button chooseButton = new Button("Printer...");

It's still relevant.

> test/jdk/java/awt/print/PrinterJob/ValidatePage/ValidatePage.java line 229:
> 
>> 227:         add(panel);
>> 228:         TextArea ta = new TextArea(10, 45);
>> 229:         String ls = System.getProperty("line.Separator", "\n");
> 
> Does `\n` not work in `TextArea` on all the platforms? In particular on Windows? If it does, just embed it into the text below.

It does work correctly with `\n`; please embed `\n` into the string and remove `ls` variable.

> test/jdk/java/awt/print/PrinterJob/ValidatePage/ValidatePage.java line 259:
> 
>> 257:                         o == PageFormat.LANDSCAPE ? "LANDSCAPE" :
>> 258:                                 o == PageFormat.REVERSE_LANDSCAPE ? "REVERSE_LANDSCAPE" :
>> 259:                                         "<invalid>"));
> 
> The code for converting orientation to string is repeated here. Create a helper method which does it — eliminate duplicate code.

Create a helper method which returns the string representation of the orientation and use the helper method in both places where you convert orientation to string, specifically lines 83–88 and 251–256.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1487599250
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1487943617
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1487943861
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1487956260
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1487928320