RFR: 8320608: Many jtreg printing tests are missing the @printer keyword

Mon Nov 27 19:29:20 UTC 2023

On Thu, 23 Nov 2023 11:10:35 GMT, Alexey Ivanov <aivanov 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.
>
> test/jdk/java/awt/PrintJob/EdgeTest/EdgeTest.java line 30:
> 
>> 28:  * @summary Verifies that (0, 0) is the upper-left corner of the page, not
>> 29:  *          the upper-left corner adjusted for the margins.
>> 30:  * @author dpm
> 
> Suggestion:
> 
> 
> Perhaps, remove the `@author` tag here too.

will do, although removing author tags was just something I was doing as I went along and happened to spot them ..

> test/jdk/java/awt/PrintJob/RoundedRectTest/RoundedRectTest.java line 29:
> 
>> 27:  * @bug 4061440
>> 28:  * @summary Checks that rounded rectangles print correctly.
>> 29:  * @author dpm
> 
> Suggestion:

will do

> test/jdk/java/awt/print/PageFormat/SmallPaperPrinting.java line 30:
> 
>> 28: 
>> 29: import java.awt.*;
>> 30: import java.awt.print.*;
> 
> Could you expand the wildcard imports, please?
> 
> I would appreciate if you would add the jtreg tags *after* the imports.

I can .. but I am not aware of even a convention to do that ordering.
I'll update the imports in this file since I touched it more than any other case

> test/jdk/java/awt/print/PageFormat/SmallPaperPrinting.java line 35:
> 
>> 33: {
>> 34:    public static void main(String args[])
>> 35:    {
> 
> Suggestion:
> 
> public class SmallPaperPrinting {
>    public static void main(String[] args) {
> 
> Since you're reformatting the test source, it's better to follow the Java code style… throughout the entire source file.

ok.

> test/jdk/java/awt/print/PageFormat/WrongPaperPrintingTest.java line 29:
> 
>> 27:    @key printer
>> 28:    @run main/manual WrongPaperPrintingTest
>> 29:  */
> 
> It's minor but still, in majority of tests, the `@key` tag follows the `@bug` tag, and you mostly follow the convention in this PR. Keeping the order consistent helps scanning the test description with eyes quickly.

updated

> test/jdk/java/awt/print/PrinterJob/EmptyFill.java line 30:
> 
>> 28:  * @key printer
>> 29:  * @run main EmptyFill
>> 30:  */
> 
> This test does not need a printer, it prints into a byte array and analyses the output. The test passes successfully on a system without a printer.

You are right, I mis-read this
       PrinterJob pj = PrinterJob.getPrinterJob();
       if (svc == null) {
           return;
       }
that test bails if there's no service but that's if it failed to create the output stream service
Not sure why the code is ordered that way round since we can check for null first.

> test/jdk/java/awt/print/PrinterJob/GetUserNameTest.java line 26:
> 
>> 24:  * @test
>> 25:  * @bug 6197099
>> 26:  * @key printer
> 
> The test works correctly without a printer, it prints the expected message:
> 
> SecurityException thrown as user.name permission not given
> 
> 
> No other exceptions are thrown but the warnings that `SecurityManager` is deprecated.

fixed

> test/jdk/java/awt/print/PrinterJob/MultiMonPrintDlgTest.java line 40:
> 
>> 38:  * @test
>> 39:  * @bug 8138749
>> 40:  * @key printer
> 
> Suggestion:
> 
>  * @key printer multimon
> 
> The test requires two monitors.

fixed. I meant to add this but forgot.

> test/jdk/java/awt/print/PrinterJob/PrintAllFonts.java line 26:
> 
>> 24: /**
>> 25:  *
>> 26:  * @bug 4884389 7183516
> 
> Is it intentional that there's no `@test` tag?

I think this is another test that needs some work. It doesn't wait to see if the user gave it a pass.
I'll add "test" but not "@test".

> test/jdk/java/awt/print/PrinterJob/PrintAllFonts.java line 29:
> 
>> 27:  * @key printer
>> 28:  * @summary Font specified with face name loses style on printing
>> 29:  * @run main/manual PrintRotatedText
> 
> Is it intentional that this test presumably runs another test — `PrintRotatedText` — instead?

that looks wrong. will fix

> test/jdk/java/awt/print/PrinterJob/PrintTranslatedFont.java line 29:
> 
>> 27:  * @key printer
>> 28:  * @summary Test that fonts with a translation print where they should.
>> 29:  * @author prr
> 
> Suggestion:

fixed

> test/jdk/java/awt/print/PrinterJob/PrinterDevice.java line 31:
> 
>> 29:  * @key printer
>> 30:  * @run main/othervm PrinterDevice
>> 31:  */
> 
> There's no `@test` tag, is it intentional?
> 
> Probably, you should remove this test from this PR because you're modifying in #16773.

agreed

> test/jdk/java/awt/print/PrinterJob/PrinterDialogsModalityTest/PrinterDialogsModalityTest.html line 30:
> 
>> 28:   @key printer
>> 29:   @summary check whether Print- and Page- dialogs are modal and correct window activated after their closing
>> 30:   @author Your Name: area=PrinterJob.modality
> 
> Suggestion:

fixed

> test/jdk/java/awt/print/PrinterJob/RemoveListener.java line 25:
> 
>> 23: 
>> 24: /*
>> 25:  * @test 1.1 01/05/17
> 
> Suggestion:
> 
>  * @test

fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406595049
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406595720
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406610431
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406621800
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406627499
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406630862
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406631860
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406633247
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406639812
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406638321
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406640707
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406642687
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406643230
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1406643654