On Thu, 1 Feb 2024 07:09:00 GMT, Renjith Kannath Pariyangad
wrote:
>> Hi Reviewers,
>>
>> Updated manual printer test cases with 'PassFailJFrame', also removed unused
>> variables. Added 'SkippedException' in case of printer missing or not
>> configured.
>>
>> Please review and let me know your suggestions.
>>
>> Regards,
>> Renjith
>
> 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
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/print/PrinterJob/Collate2DPrintingTest.java line 40:
> 38: import java.awt.event.ActionListener;
> 39: import java.awt.event.WindowAdapter;
> 40: import java.awt.event.WindowEvent;
`WindowAdapter` and `WindowEvent` aren't used.
test/jdk/java/awt/print/PrinterJob/Collate2DPrintingTest.java line 113:
> 111: } catch (Exception e) {
> 112: e.printStackTrace();
> 113: }
Display an error message?
test/jdk/java/awt/print/PrinterJob/Collate2DPrintingTest.java line 139:
> 137: private static final String INSTRUCTIONS =
> 138: "You must have a printer available to perform this test\n" +
> 139: "The print result should be collated.";
Expand the instructions to explain what to do?
There are two buttons, which do I click?
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 54:
> 52: protected BufferedImage _image;
> 53:
> 54: protected PageFormat _pageFormat;
I suggest getting rid of the leading underscores. The fields could be `private`
(and `final` too), because test classes aren't extended, some classes are even
marked as `final` too.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 63:
> 61:
> 62: protected int printImage(Graphics g, PageFormat pf, BufferedImage
> image) {
> 63:
A blank at the start of a method is redundant.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 68:
> 66:
> 67: int paperW = (int) pf.getImageableWidth(), paperH =
> 68: (int) pf.getImageableHeight();
Suggestion:
int paperW = (int) pf.getImageableWidth();
int paperH = (int) pf.getImageableHeight();
Declare the variable separately.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 70:
> 68: (int) pf.getImageableHeight();
> 69:
> 70: int x = (int) pf.getImageableX(), y = (int) pf.getImageableY();
Suggestion:
int x = (int) pf.getImageableX();
int y = (int) pf.getImageableY();
Declare the variables separately.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 80:
> 78: g2D.drawImage(image, scaleOp, x + _objectBorder, y +
> _objectBorder);
> 79:
> 80: g2D.dispose();
Did you create the `Graphics` object? If not, do not dispose of it.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 94:
> 92: }
> 93: return result;
> 94: });
Suggestion:
pj.setPrintable(this::printImage);
Move the logic to return `NO_SUCH_PAGE` into the `printImage`, modify the
declaration of `printImage`:
private int printImage(Graphics g, PageFormat pf, int pageIndex) {
It doesn't need the `image` parameter — it can access the member `_image`.
Fail the test immediately if the image cannot be created.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 101:
> 99: } catch (Exception e) {
> 100: e.printStackTrace();
> 101: }
Display an error message? Let the fail?
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 107:
> 105: "You must have a printer available to perform this test.\n" +
> 106: "\n" +
> 107: "The test passes if you get a printout of a gray
> rectangle\n" +
Expand the instructions to explain that a print job is started automatically.
This kind of tests could benefit from having a secondary UI for the tester to
initiate printing explicitly. Unfortunately, the changes for split UI in
`PassFassJFrame` ([JDK-8294148](https://bugs.openjdk.org/browse/JDK-8294148))
aren't ready yet.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 108:
> 106: "\n" +
> 107: "The test passes if you get a printout of a gray
> rectangle\n" +
> 108: "with white text without any exception.";
Which means that you should make the test fail if any exception occurs rather
swallowing the exception and printing its stack trace.
test/jdk/java/awt/print/PrinterJob/DrawImage.java line 114:
> 112: if (PrinterJob.lookupPrintServices().length == 0) {
> 113: throw new SkippedException("Printer not configured or
> available."
> 114: + " Test cannot continue.");
Suggestion:
throw new SkippedException("Printer not configured or available.");
test/jdk/java/awt/print/PrinterJob/DrawStringMethods.java line 1:
> 1: /*
Replace C-style (`type a[]`) arra
