On Mon, 29 Jan 2024 06:24:29 GMT, Renjith Kannath Pariyangad
<rkannathp...@openjdk.org> 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
test/jdk/java/awt/print/PageFormat/PageSetupDialog.java line 30:
> 28: * @bug 6358747
> 29: * @bug 6574633
> 30: * @summary Page setup dialog settings
bug-ids can be added to a single line
`@bug 4197377 4299145 6358747 6574633`
test/jdk/java/awt/print/PageFormat/PageSetupDialog.java line 77:
> 75: boolean reverse = false;
> 76:
> 77: private static final String instructions =
Capitalize static final vars
Suggestion:
private static final String INSTRUCTIONS =
test/jdk/java/awt/print/PageFormat/PageSetupDialog.java line 159:
> 157: c.add(pgbm = new Label());
> 158:
> 159: reverseCB.addItemListener(new ItemListener() {
Can be replaced by lambda expression. There are few other places in the test
that require similar changes
Suggestion:
reverseCB.addItemListener(e -> {
test/jdk/java/awt/print/PageFormat/PageSetupDialog.java line 174:
> 172: Panel panel = new Panel();
> 173: Button pageButton = new Button("Page Setup...");
> 174: pageButton.addActionListener(new ActionListener() {
Suggestion:
pageButton.addActionListener(e -> {
test/jdk/java/awt/print/PageFormat/PageSetupDialog.java line 181:
> 179: });
> 180: Button printButton = new Button("Print ...");
> 181: printButton.addActionListener(new ActionListener() {
Here as well.
test/jdk/java/awt/print/PageFormat/PageSetupDialog.java line 254:
> 252: .instructions(instructions)
> 253: .testUI(PageSetupDialog::new)
> 254: .testTimeOut(5)
Since the default timeout in PassFailJFrame is set to 5 mins, `.testTimeOut(5)`
is not necessary.
test/jdk/java/awt/print/PrinterJob/Cancel/PrinterJobCancel.java line 61:
> 59: "tests apparently aren't supposed to call System.exit()";
> 60:
> 61: public static void main(String args[]) throws Exception {
Suggestion:
public static void main(String[] args) throws Exception {
test/jdk/java/awt/print/PrinterJob/PrintAllFonts.java line 35:
> 33:
> 34: /*
> 35: *
Remove the extra asterisk '*'
test/jdk/java/awt/print/PrinterJob/PrintAllFonts.java line 70:
> 68: .title("PrintAllFonts Test Instructions")
> 69: .instructions(instructions)
> 70: .testTimeOut(5)
.testTimeOut(5) can be omitted since default timeout is 5 mins
test/jdk/java/awt/print/PrinterJob/ValidatePage/ValidatePage.java line 209:
> 207:
> 208: Button chooseButton = new Button("Printer..");
> 209: chooseButton.addActionListener(new ActionListener() {
Same changes at other places as well
Suggestion:
chooseButton.addActionListener(e -> {
test/jdk/java/awt/print/PrinterJob/ValidatePage/ValidatePage.java line 263:
> 261: ta.setEditable(false);
> 262: add(ta);
> 263: setSize(500, 650);
Set paper and Print options are not visible with current frame size.
Suggestion:
setSize(680, 500);
test/jdk/java/awt/print/PrinterJob/raster/RasterTest.java line 65:
> 63: "The printed output should match the on-screen display,
> although\n" +
> 64: "only colour printers will be able to accurately reproduce
> the\n" +
> 65: "subtle color changes.";
When saved to pdf the gradient square color looks different from the one in the
test window. Probably it works as expected on a color printer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470374379
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470375632
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470379063
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470377804
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470377835
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470382538
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470387961
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470390245
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470390856
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470401708
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470400268
PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470407737
