Re: RFR: 8272342: [TEST_BUG] java/awt/print/PrinterJob/PageDialogMarginTest.java catches all exceptions [v2]

2021-08-15 Thread Alexey Ivanov
On Sat, 14 Aug 2021 01:15:45 GMT, rajat mahajan 
 wrote:

>> Removed try catch block so the test can function as expected and fail
>> when it ought to, rather than passing all the time because of catch
>> block catching all exceptions and Passing test even there is a failure.
>
> rajat mahajan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix IDE warnings, added Default Locale as US, fixed Copyright to include 
> current year.

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/print/PrinterJob/PageDialogMarginTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2016, 2021,  Oracle and/or its affiliates. All rights 
> reserved.

Suggestion:

 * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.

Two many spaces, there should be one space after the comma.

test/jdk/java/awt/print/PrinterJob/PageDialogMarginTest.java line 40:

> 38: import javax.swing.JOptionPane;
> 39: import javax.swing.SwingUtilities;
> 40: import java.text.MessageFormat;

I wonder where do DialogTypeSelection and MessageFormat come from?

The imports should be sorted by default, usually your IDE handles it perfectly 
on its own.

-

PR: https://git.openjdk.java.net/jdk/pull/5115


Re: RFR: 8272342: [TEST_BUG] java/awt/print/PrinterJob/PageDialogMarginTest.java catches all exceptions [v2]

2021-08-14 Thread Pankaj Bansal
On Sat, 14 Aug 2021 01:15:45 GMT, rajat mahajan 
 wrote:

>> Removed try catch block so the test can function as expected and fail
>> when it ought to, rather than passing all the time because of catch
>> block catching all exceptions and Passing test even there is a failure.
>
> rajat mahajan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix IDE warnings, added Default Locale as US, fixed Copyright to include 
> current year.

Marked as reviewed by pbansal (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5115


Re: RFR: 8272342: [TEST_BUG] java/awt/print/PrinterJob/PageDialogMarginTest.java catches all exceptions [v2]

2021-08-13 Thread rajat mahajan
> Removed try catch block so the test can function as expected and fail
> when it ought to, rather than passing all the time because of catch
> block catching all exceptions and Passing test even there is a failure.

rajat mahajan has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix IDE warnings, added Default Locale as US, fixed Copyright to include 
current year.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5115/files
  - new: https://git.openjdk.java.net/jdk/pull/5115/files/a5c18b2d..2c181a8a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5115&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5115&range=00-01

  Stats: 15 lines in 1 file changed: 6 ins; 2 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5115.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5115/head:pull/5115

PR: https://git.openjdk.java.net/jdk/pull/5115


Re: RFR: 8272342: [TEST_BUG] java/awt/print/PrinterJob/PageDialogMarginTest.java catches all exceptions [v2]

2021-08-13 Thread rajat mahajan
On Fri, 13 Aug 2021 19:38:12 GMT, Alexey Ivanov  wrote:

>> rajat mahajan has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix IDE warnings, added Default Locale as US, fixed Copyright to include 
>> current year.
>
> test/jdk/java/awt/print/PrinterJob/PageDialogMarginTest.java line 58:
> 
>> 56: HashPrintRequestAttributeSet aset = new 
>> HashPrintRequestAttributeSet();
>> 57: PageFormat pf;
>> 58: pf = pj.pageDialog(aset);
> 
> I suggest merging these two lines:
> Suggestion:
> 
> PageFormat pf = pj.pageDialog(aset);

Made the changes you asked.

-

PR: https://git.openjdk.java.net/jdk/pull/5115