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

Alexey Ivanov aivanov at openjdk.org
Thu Nov 23 17:27:17 UTC 2023 

On Wed, 22 Nov 2023 19:26:40 GMT, Phil Race <prr 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.

Changes requested by aivanov (Reviewer).

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.

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:

test/jdk/java/awt/print/PageFormat/SetOrient.java line 28:

> 26:  * @summary Confirm that the clip and transform of the Graphics2D is
> 27:  *          affected by the landscape orientation of the PageFormat.
> 28:  */

Since this test calls `pjob.print();`, it requires a printer. Indeed, it fails without a printer:


runner starting test: java/awt/print/PageFormat/SetOrient.html
runner finished test: java/awt/print/PageFormat/SetOrient.html
Failed. Execution failed: Applet thread threw exception:
        java.lang.RuntimeException: No print service found.
Test results: failed: 1

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.

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.

test/jdk/java/awt/print/PageFormat/SmallPaperPrinting.java line 42:

> 40:      System.out.println("A passing test should catch a PrinterException");
> 41:      System.out.println("and should display \"Print error: (exception msg)\".");
> 42:      System.out.println("---------------------------------------------------\n");

According to these instructions, the test is to contain a set of `@test` tags:


/*
* @test
* @key printer
* @run main/othervm SmallPaperPrinting
*/

/*
 * @test
 * @key printer
 * @run main/othervm SmallPaperPrinting 1
 */

/*
 * @test
 * @key printer
 * @run main/othervm SmallPaperPrinting 2
 */


Otherwise, it won't run all the cases and no one will ever see these instructions.

For me, the test with the added `@test` tags as above prints an error message in the first two cases:

# id0.jtr
Print error:
Paper's imageable height is too small.
# id1.jtr
Print error:
Paper's imageable width is too small.


Yet it does not print any error message in the third case where `width=-1`, and **it does not fail**.

If I run it on a system without a printer, the test also *passes successfully*. Perhaps, we can ignore it as the `@key printer` ensures there's a printer the system.

Having said the above, this test requires its own bug to fix the test.

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.

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.

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.

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.

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?

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:

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.

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:

test/jdk/java/awt/print/PrinterJob/RemoveListener.java line 25:

> 23: 
> 24: /*
> 25:  * @test 1.1 01/05/17

Suggestion:

 * @test

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

PR Review: https://git.openjdk.org/jdk/pull/16785#pullrequestreview-1746215963
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403233563
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403232953
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403250610
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403257236
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403254342
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403283083
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403301368
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403332726
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403341309
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403576760
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403595968
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403608444
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403613621
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403613833
PR Review Comment: https://git.openjdk.org/jdk/pull/16785#discussion_r1403616874

More information about the client-libs-dev mailing list