Integrated: JDK-8333360 : PrintNullString.java doesn't use float arguments
On Tue, 4 Jun 2024 12:07:40 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > I have updated the test case with passing float value for evaluation and a > typo. Please review and let me know your suggestions if any. This pull request has now been integrated. Changeset: 86b5 Author: Renjith Kannath Pariyangad Committer: Alexey Ivanov URL: https://git.openjdk.org/jdk/commit/86b57172d45d1126c50efc270c6e49aba7a5 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 860: PrintNullString.java doesn't use float arguments Reviewed-by: aivanov, abhiscxk, achung - PR: https://git.openjdk.org/jdk/pull/19540
RFR: JDK-8333360 : PrintNullString.java doesn't use float arguments
Hi Reviewers, I have updated the test case with passing float value for evaluation and a typo. Please review and let me know your suggestions if any. - Commit messages: - JDK-860 : PrintNullString.java doesn't use float arguments Changes: https://git.openjdk.org/jdk/pull/19540/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19540=00 Issue: https://bugs.openjdk.org/browse/JDK-860 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19540.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19540/head:pull/19540 PR: https://git.openjdk.org/jdk/pull/19540
Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v3]
> Hi Reviewers, > > Added pageloader cancel before new page creation along with code > restructuring. Moved all page loading calls inside synchronize to make it > thread safe. > > Regards, > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Rearranged if based on suggesion - Changes: - all: https://git.openjdk.org/jdk/pull/18670/files - new: https://git.openjdk.org/jdk/pull/18670/files/2ca2ae11..be168e9d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18670=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18670=01-02 Stats: 21 lines in 1 file changed: 9 ins; 4 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/18670.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18670/head:pull/18670 PR: https://git.openjdk.org/jdk/pull/18670
Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v2]
On Thu, 18 Apr 2024 16:41:34 GMT, Alexey Ivanov wrote:
>> Renjith Kannath Pariyangad has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> Suggesions updated
>
> src/java.desktop/share/classes/javax/swing/JEditorPane.java line 497:
>
>> 495: // we need to cancel background loading
>> 496: if (pageLoader != null) {
>> 497: pageLoader.cancel(true);
>
> Suggestion:
>
> pageLoader.cancel(true);
> pageLoader = null;
>
> Let's assign `null` to indicate there's no active `pageLoader` any more.
Updated
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18670#discussion_r1572292328
Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v2]
> Hi Reviewers, > > Added pageloader cancel before new page creation along with code > restructuring. Moved all page loading calls inside synchronize to make it > thread safe. > > Regards, > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Suggesions updated - Changes: - all: https://git.openjdk.org/jdk/pull/18670/files - new: https://git.openjdk.org/jdk/pull/18670/files/fe4b78e0..2ca2ae11 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18670=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18670=00-01 Stats: 14 lines in 1 file changed: 8 ins; 5 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18670.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18670/head:pull/18670 PR: https://git.openjdk.org/jdk/pull/18670
Integrated: 8329322 : Convert PageFormat/Orient.java to use PassFailJFrame
On Thu, 4 Apr 2024 12:07:46 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > > I have updated the test with PassFailJFrame along with printer availability > check. Please review and let me know your suggestions. > > Renjith. This pull request has now been integrated. Changeset: f713766c Author: Renjith Kannath Pariyangad Committer: Alexey Ivanov URL: https://git.openjdk.org/jdk/commit/f713766cab649947e543f0290717e7bcc501f063 Stats: 418 lines in 1 file changed: 19 ins; 355 del; 44 mod 8329322: Convert PageFormat/Orient.java to use PassFailJFrame Reviewed-by: abhiscxk, aivanov - PR: https://git.openjdk.org/jdk/pull/18624
Re: RFR: 8329322 : Convert PageFormat/Orient.java to use PassFailJFrame [v2]
On Mon, 8 Apr 2024 07:57:11 GMT, Renjith Kannath Pariyangad wrote: >> Hi Reviewers, >> >> I have updated the test with PassFailJFrame along with printer availability >> check. Please review and let me know your suggestions. >> >> Renjith. > > Renjith Kannath Pariyangad has updated the pull request incrementally with > two additional commits since the last revision: > > - Updated insturction style > - Updated suggesions Its working for me, please make sure you have configured pdf printer as your default printer. In this test there is no option to select printer. - PR Comment: https://git.openjdk.org/jdk/pull/18624#issuecomment-2050993229
Re: RFR: 8329322 : Convert PageFormat/Orient.java to use PassFailJFrame [v2]
On Mon, 8 Apr 2024 07:57:11 GMT, Renjith Kannath Pariyangad wrote: >> Hi Reviewers, >> >> I have updated the test with PassFailJFrame along with printer availability >> check. Please review and let me know your suggestions. >> >> Renjith. > > Renjith Kannath Pariyangad has updated the pull request incrementally with > two additional commits since the last revision: > > - Updated insturction style > - Updated suggesions Yes this will work on Print to pdf, I don't think physical printer is must. - PR Comment: https://git.openjdk.org/jdk/pull/18624#issuecomment-2050953129
Re: RFR: 8329322 : Convert PageFormat/Orient.java to use PassFailJFrame [v2]
> Hi Reviewers, > > I have updated the test with PassFailJFrame along with printer availability > check. Please review and let me know your suggestions. > > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with two additional commits since the last revision: - Updated insturction style - Updated suggesions - Changes: - all: https://git.openjdk.org/jdk/pull/18624/files - new: https://git.openjdk.org/jdk/pull/18624/files/e151b4ca..fcf3830a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18624=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18624=00-01 Stats: 24 lines in 1 file changed: 12 ins; 7 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/18624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18624/head:pull/18624 PR: https://git.openjdk.org/jdk/pull/18624
RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled
Hi Reviewers, Added pageloader cancel before new page creation along with code restructuring. Moved all page loading calls inside synchronize to make it thread safe. Regards, Renjith. - Commit messages: - Removed space - JDK-8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled Changes: https://git.openjdk.org/jdk/pull/18670/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18670=00 Issue: https://bugs.openjdk.org/browse/JDK-8328977 Stats: 80 lines in 1 file changed: 33 ins; 28 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/18670.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18670/head:pull/18670 PR: https://git.openjdk.org/jdk/pull/18670
RFR: 8329322 : Convert PageFormat/Orient.java to use PassFailJFrame
Hi Reviewers, I have updated the test with PassFailJFrame along with printer availability check. Please review and let me know your suggestions. Renjith. - Commit messages: - JDK-8329322:Convert PageFormat/Orient.java to use PassFailJFrame Changes: https://git.openjdk.org/jdk/pull/18624/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18624=00 Issue: https://bugs.openjdk.org/browse/JDK-8329322 Stats: 413 lines in 1 file changed: 11 ins; 352 del; 50 mod Patch: https://git.openjdk.org/jdk/pull/18624.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18624/head:pull/18624 PR: https://git.openjdk.org/jdk/pull/18624
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v17]
> 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: Function name updated - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/5991937f..03b4076d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=16 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=15-16 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Integrated: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1
On Mon, 29 Jan 2024 06:24:29 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 This pull request has now been integrated. Changeset: 7eb78e33 Author:Renjith Kannath Pariyangad Committer: Alexey Ivanov URL: https://git.openjdk.org/jdk/commit/7eb78e332080df3890b66ca04338a4ba69af45eb Stats: 1723 lines in 5 files changed: 438 ins; 1014 del; 271 mod 8320676: Manual printer tests have no Pass/Fail buttons, instructions close set 1 Reviewed-by: honkar, achung, aivanov - PR: https://git.openjdk.org/jdk/pull/17607
Integrated: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2
On Mon, 29 Jan 2024 06:52:01 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 This pull request has now been integrated. Changeset: ed821cbe Author:Renjith Kannath Pariyangad Committer: Alexey Ivanov URL: https://git.openjdk.org/jdk/commit/ed821cbe857363e049f3c640ae4546c340a577ac Stats: 1299 lines in 6 files changed: 230 ins; 902 del; 167 mod 8324807: Manual printer tests have no Pass/Fail buttons, instructions close set 2 Reviewed-by: tr, achung, aivanov - PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v16]
> 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: Force fail added for user cancel case - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/cd8d67dd..5991937f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=15 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=14-15 Stats: 4 lines in 2 files changed: 2 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Integrated: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3
On Mon, 29 Jan 2024 07:22:57 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 This pull request has now been integrated. Changeset: af7c6af0 Author:Renjith Kannath Pariyangad Committer: Phil Race URL: https://git.openjdk.org/jdk/commit/af7c6af0cc1eb6c42199c05933c7feb032bd6353 Stats: 1638 lines in 6 files changed: 296 ins; 1069 del; 273 mod 8324808: Manual printer tests have no Pass/Fail buttons, instructions close set 3 Reviewed-by: tr, achung, aivanov - PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v14]
> 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: Suggestions integrated - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/aa801855..0243809f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=13 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=12-13 Stats: 5 lines in 4 files changed: 2 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v15]
> 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: Optimized switch case - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/a3d2a6d4..cd8d67dd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=14 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=13-14 Stats: 27 lines in 1 file changed: 10 ins; 12 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v14]
> 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: Updated PassFailJFrame object creation - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/892be4a0..a3d2a6d4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=13 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=12-13 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Integrated: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs
On Thu, 14 Mar 2024 11:52:09 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > > I have updated the test with PassFailJFrame with information panel, earlier > this was informed through terminal. > Please review and let me know your suggestions if any. This pull request has now been integrated. Changeset: 9f5ad433 Author:Renjith Kannath Pariyangad Committer: Tejesh R URL: https://git.openjdk.org/jdk/commit/9f5ad43358a4e209b4cd1c91bcc86b997f371548 Stats: 238 lines in 1 file changed: 54 ins; 154 del; 30 mod 8320675: PrinterJob/SecurityDialogTest.java hangs Co-authored-by: Alexey Ivanov Reviewed-by: aivanov, tr - PR: https://git.openjdk.org/jdk/pull/18299
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v13]
> 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: Updated PassFailJFrame object creation - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/69fe7d96..aa801855 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=12 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=11-12 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs [v4]
> Hi Reviewers, > > I have updated the test with PassFailJFrame with information panel, earlier > this was informed through terminal. > Please review and let me know your suggestions if any. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Removed extra space - Changes: - all: https://git.openjdk.org/jdk/pull/18299/files - new: https://git.openjdk.org/jdk/pull/18299/files/b95f9022..cc6d560d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18299=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18299=02-03 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18299.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18299/head:pull/18299 PR: https://git.openjdk.org/jdk/pull/18299
Re: RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs [v3]
> Hi Reviewers, > > I have updated the test with PassFailJFrame with information panel, earlier > this was informed through terminal. > Please review and let me know your suggestions if any. Renjith Kannath Pariyangad has updated the pull request incrementally with two additional commits since the last revision: - Removed frame - Updated PassFailJFrame object access - Changes: - all: https://git.openjdk.org/jdk/pull/18299/files - new: https://git.openjdk.org/jdk/pull/18299/files/6e80eaa0..b95f9022 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18299=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18299=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18299.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18299/head:pull/18299 PR: https://git.openjdk.org/jdk/pull/18299
Re: RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs [v2]
On Fri, 15 Mar 2024 03:33:24 GMT, Tejesh R wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Added summary > > test/jdk/java/awt/print/PrinterJob/SecurityDialogTest.java line 43: > >> 41: * @key printer >> 42: * @library /java/awt/regtesthelpers >> 43: * @build PassFailJFrame > > Any reason why test summary is not there? It was not part of original test case, I have updated. - PR Review Comment: https://git.openjdk.org/jdk/pull/18299#discussion_r1525919146
Re: RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs [v2]
> Hi Reviewers, > > I have updated the test with PassFailJFrame with information panel, earlier > this was informed through terminal. > Please review and let me know your suggestions if any. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Added summary - Changes: - all: https://git.openjdk.org/jdk/pull/18299/files - new: https://git.openjdk.org/jdk/pull/18299/files/ede65c0b..6e80eaa0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18299=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18299=00-01 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18299.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18299/head:pull/18299 PR: https://git.openjdk.org/jdk/pull/18299
Re: RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs
On Fri, 15 Mar 2024 03:43:13 GMT, Tejesh R wrote: >> Hi Reviewers, >> >> I have updated the test with PassFailJFrame with information panel, earlier >> this was informed through terminal. >> Please review and let me know your suggestions if any. > > test/jdk/java/awt/print/PrinterJob/SecurityDialogTest.java line 67: > >> 65: } >> 66: >> 67: PassFailJFrame passFailJFrame = new PassFailJFrame.Builder() > > I am getting an error for using new `PassFailJFrame passFailJFrame = new > PassFailJFrame.Builder()`, stating _"'Builder()' has private access in > 'PassFailJFrame.Builder'"_ Probably because I have fetched latest > PassFailJFrame. > And also you can use `PassFailJFrame.builder()` method. For me its building and running properly - PR Review Comment: https://git.openjdk.org/jdk/pull/18299#discussion_r1525848915
Re: RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs
On Fri, 15 Mar 2024 03:31:32 GMT, Tejesh R wrote:
>> Hi Reviewers,
>>
>> I have updated the test with PassFailJFrame with information panel, earlier
>> this was informed through terminal.
>> Please review and let me know your suggestions if any.
>
> test/jdk/java/awt/print/PrinterJob/SecurityDialogTest.java line 47:
>
>> 45: */
>> 46: public class SecurityDialogTest extends Frame {
>> 47: private static final String INSTRUCTIONS =
>
> `INSTRUCTIONS` can be moved inside main.
Most of the test with PassFailJFrame instruction stored as string inside class,
what additional benefit will get keeping it inside Main ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18299#discussion_r1525799780
RFR: JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs
Hi Reviewers, I have updated the test with PassFailJFrame with information panel, earlier this was informed through terminal. Please review and let me know your suggestions if any. - Commit messages: - Removed extra space before instruction lines - Updated with message - Use new PageFormat() for pageDialog - Move printing services after Security Manager is set - Update instructions - Refactor the test - JDK-8320675 : PrinterJob/SecurityDialogTest.java hangs Changes: https://git.openjdk.org/jdk/pull/18299/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18299=00 Issue: https://bugs.openjdk.org/browse/JDK-8320675 Stats: 232 lines in 1 file changed: 47 ins; 149 del; 36 mod Patch: https://git.openjdk.org/jdk/pull/18299.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18299/head:pull/18299 PR: https://git.openjdk.org/jdk/pull/18299
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v13]
> 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: Updated instructions - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/8cb1d7d4..892be4a0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=12 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=11-12 Stats: 8 lines in 3 files changed: 4 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v12]
> 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: Updated instructions and access specifiers - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/bec0332e..69fe7d96 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=11 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=10-11 Stats: 14 lines in 3 files changed: 1 ins; 3 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v12]
> 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: Updated instructions - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/e6047f06..2160b4f5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=11 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=10-11 Stats: 25 lines in 3 files changed: 12 ins; 3 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v11]
> 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: Fixed comple issue - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/df3831c8..e6047f06 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=10 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=09-10 Stats: 6 lines in 2 files changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v10]
> 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: Added Override - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/b2b8548d..df3831c8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=09 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=08-09 Stats: 24 lines in 5 files changed: 9 ins; 7 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v11]
> 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: Added Override - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/a9aebeb8..bec0332e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=10 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=09-10 Stats: 16 lines in 4 files changed: 10 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v10]
On Tue, 5 Mar 2024 13:07:30 GMT, Alexey Ivanov wrote:
>> Renjith Kannath Pariyangad has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> Updated instruction
>
> test/jdk/java/awt/print/PrinterJob/Collate2DPrintingTest.java line 74:
>
>> 72: if (pj.printDialog(prSet)) {
>> 73: pj.print(prSet);
>> 74: }
>
> In this case, the print dialog has *Collated* checked and disabled; the
> number of copies is set to 1 and I can't change the value.
>
> Does it work for you? Is it a bug in JDK?
For print to pdf, yes not able to change number of copies, but for actual
printer able to change the number.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17608#discussion_r1513869975
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v12]
> 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: Added Override - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/10c2c695..8cb1d7d4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=11 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=10-11 Stats: 36 lines in 5 files changed: 8 ins; 19 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v9]
> 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: Suggesions added - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/3847671a..b2b8548d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=08 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=07-08 Stats: 26 lines in 2 files changed: 4 ins; 3 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v10]
> 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: Updated instruction - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/d075faab..a9aebeb8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=09 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=08-09 Stats: 24 lines in 1 file changed: 8 ins; 12 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v11]
> 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: Suggesions incorporated - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/38c4f395..10c2c695 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=10 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=09-10 Stats: 27 lines in 4 files changed: 2 ins; 11 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v9]
> 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: Removed test/lib - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/39275aae..d075faab Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=08 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=07-08 Stats: 31 lines in 6 files changed: 4 ins; 21 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v8]
> 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: Updated with split UI - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/1d9bb6bc..3847671a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=07 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=06-07 Stats: 256 lines in 6 files changed: 82 ins; 89 del; 85 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Integrated: 8321192 : j.a.PrintJob/ImageTest/ImageTest.java: Fail or skip the test if there's no printer
On Fri, 9 Feb 2024 14:59:06 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > I have updated the test with 'PassFailJFrame' with programmatically > generating image and print , please review and let me know your suggestions. This pull request has now been integrated. Changeset: 724a2a2c Author: Renjith Kannath Pariyangad Committer: Alexey Ivanov URL: https://git.openjdk.org/jdk/commit/724a2a2c4a6020188b7907509cd48aa126b79b0f Stats: 164 lines in 1 file changed: 164 ins; 0 del; 0 mod 8321192: j.a.PrintJob/ImageTest/ImageTest.java: Fail or skip the test if there's no printer Reviewed-by: aivanov, tr - PR: https://git.openjdk.org/jdk/pull/17790
Re: RFR: 8321192 : j.a.PrintJob/ImageTest/ImageTest.java: Fail or skip the test if there's no printer [v3]
> Hi Reviewers, > I have updated the test with 'PassFailJFrame' with programmatically > generating image and print , please review and let me know your suggestions. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Removed test/lib - Changes: - all: https://git.openjdk.org/jdk/pull/17790/files - new: https://git.openjdk.org/jdk/pull/17790/files/351bd721..9cdc8fb9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17790=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17790=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17790.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17790/head:pull/17790 PR: https://git.openjdk.org/jdk/pull/17790
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v8]
> 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: Updated with split UI - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/1d1450fd..39275aae Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=07 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=06-07 Stats: 129 lines in 2 files changed: 63 ins; 46 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v7]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge master - Updated with static function - Updated with minimum 2 printer required - Replaced skipped exception with runtime exception - Disposed g2D object and similar test parm into one line - Capitalized static final variables - JDK-8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/5bb4837c..1d9bb6bc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=06 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=05-06 Stats: 54318 lines in 2171 files changed: 22413 ins; 14552 del; 17353 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v7]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge master - Replaced skipped exception with runtime exception - Suggestions added - Review comments integrated - Disposed g2D object and similar test parm into one line - Capitalized static final variables - JDK-8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/81e0fc2f..1d1450fd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=06 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=05-06 Stats: 54413 lines in 2171 files changed: 22407 ins; 14626 del; 17380 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v10]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision: - Merge master - Fixed white space error and added space - Added braces for one-line blocks. - Suggestions incorporated - Fixed compiler error - Added graphic obj dispose - Replaced skipped exception with runtime exception - Disposed g2D object and similar test parm into one line - Suggestions integrated - JDK-8320676 : Manual printer tests have no Pass/Fail buttons, instructions close - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/1514ec4a..38c4f395 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=09 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=08-09 Stats: 54413 lines in 2171 files changed: 22407 ins; 14626 del; 17380 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v9]
> 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: Fixed white space error and added space - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/f5ed5519..1514ec4a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=08 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=07-08 Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v8]
> 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: Added braces for one-line blocks. - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/806ed2f4..f5ed5519 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=07 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=06-07 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v7]
> 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: Suggestions incorporated - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/f39d5752..806ed2f4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=06 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=05-06 Stats: 87 lines in 5 files changed: 17 ins; 25 del; 45 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v6]
On Tue, 13 Feb 2024 10:43:38 GMT, Alexey Ivanov wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Fixed compiler error > > test/jdk/java/awt/print/PrinterJob/Cancel/PrinterJobCancel.java line 74: > >> 72: Thread.sleep(5000); >> 73: pjc.pj.cancel(); >> 74: } > > Should the test fail if the tester clicks Cancel instead of OK / Print? This may be required for other tests too - PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1488923499
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v3]
On Thu, 8 Feb 2024 15:15:39 GMT, Alexey Ivanov wrote: >> 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 > > test/jdk/java/awt/print/PrinterJob/PageDlgPrnButton.java line 48: > >> 46: "For non-windows OS, this test PASSes.\n" + >> 47: "You must have at least 2 printers available to perform this >> test.\n" + >> 48: "This test brings up a native Windows page dialog.\n" + > > Can we verify this too? That is the number of print services is at least 2. This test is showing only a windows _page setup_ without any option to select printer and printing to default printer. Do you think we should add this condition ? - PR Review Comment: https://git.openjdk.org/jdk/pull/17609#discussion_r1487735604
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v6]
> 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: Updated with static function - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/e78d5802..5bb4837c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=04-05 Stats: 9 lines in 1 file changed: 1 ins; 5 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v5]
> 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: Updated with minimum 2 printer required - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/138c13a6..e78d5802 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=03-04 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v4]
> 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: Replaced skipped exception with runtime exception - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/61f3a837..138c13a6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=02-03 Stats: 116 lines in 6 files changed: 13 ins; 56 del; 47 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v6]
> 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: Fixed compiler error - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/c2c65cec..f39d5752 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=04-05 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v3]
On Fri, 9 Feb 2024 11:04:49 GMT, Renjith Kannath Pariyangad wrote: >> test/jdk/java/awt/print/PrinterJob/PrintAllFonts.java line 40: >> >>> 38: * @library /test/lib /java/awt/regtesthelpers >>> 39: * @build PassFailJFrame jtreg.SkippedException >>> 40: * @summary Font specified with face name loses style on printing >> >> It is unclear to me what is implied by _“loses style on printing”_ and what >> _“with correct font style”_ in the instructions mean. >> >> Printing code, derives a font with `PLAIN` style and size of 16. I see no >> other styles applied. >> >> Could you check the bugid for more details? > > Yes, there is a catch wanted to share with you. > This test more inclined with 7183516 issue. With respect to 4884389 issue > problem mentioned like _but when its > printed italic fonts are printed as plain in Win32._ This workflow missing as > like you pointed out **derives a font with PLAIN style and size of 16** I have added Italic also to print along with plain, due to space constrain reduced the font size to 14. - PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1487417156
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close set 1 [v5]
> 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: Added graphic obj dispose - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/3b21b28f..c2c65cec Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=03-04 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v4]
> 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: Replaced skipped exception with runtime exception - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/dd31c19f..3b21b28f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=02-03 Stats: 176 lines in 5 files changed: 26 ins; 50 del; 100 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v6]
> 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: Replaced skipped exception with runtime exception - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/14b6468d..81e0fc2f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=05 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=04-05 Stats: 24 lines in 6 files changed: 0 ins; 12 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8321192 : j.a.PrintJob/ImageTest/ImageTest.java: Fail or skip the test if there's no printer [v2]
> Hi Reviewers, > I have updated the test with 'PassFailJFrame' with programmatically > generating image and print , please review and let me know your suggestions. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Replaced skipped exception with runtime exception - Changes: - all: https://git.openjdk.org/jdk/pull/17790/files - new: https://git.openjdk.org/jdk/pull/17790/files/b0b3590d..351bd721 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17790=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17790=00-01 Stats: 11 lines in 1 file changed: 3 ins; 3 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/17790.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17790/head:pull/17790 PR: https://git.openjdk.org/jdk/pull/17790
RFR: 8321192 : j.a.PrintJob/ImageTest/ImageTest.java: Fail or skip the test if there's no printer
Hi Reviewers, I have updated the test with 'PassFailJFrame' with programmatically generating image and print , please review and let me know your suggestions. - Commit messages: - JDK-8321192 : j.a.PrintJob/ImageTest/ImageTest.java: Fail or skip the test if there's no printer Changes: https://git.openjdk.org/jdk/pull/17790/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17790=00 Issue: https://bugs.openjdk.org/browse/JDK-8321192 Stats: 164 lines in 1 file changed: 164 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17790.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17790/head:pull/17790 PR: https://git.openjdk.org/jdk/pull/17790
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v3]
On Wed, 7 Feb 2024 17:48:41 GMT, Alexey Ivanov wrote: >> 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 > > test/jdk/java/awt/print/PrinterJob/PrintAllFonts.java line 40: > >> 38: * @library /test/lib /java/awt/regtesthelpers >> 39: * @build PassFailJFrame jtreg.SkippedException >> 40: * @summary Font specified with face name loses style on printing > > It is unclear to me what is implied by _“loses style on printing”_ and what > _“with correct font style”_ in the instructions mean. > > Printing code, derives a font with `PLAIN` style and size of 16. I see no > other styles applied. > > Could you check the bugid for more details? Yes, there is a catch want to share with you. This test more inclined with 7183516 issue. With respect to 4884389 issue problem mentioned like _but when its printed italic fonts are printed as plain in Win32._ This workflow missing as like you pointed out **derives a font with PLAIN style and size of 16** - PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1484171813
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v5]
> 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: Suggestions added - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/90367b92..14b6468d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=03-04 Stats: 59 lines in 4 files changed: 17 ins; 20 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v4]
On Thu, 8 Feb 2024 12:03:19 GMT, Alexey Ivanov wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Review comments integrated > > test/jdk/java/awt/print/PrinterJob/DrawImage.java line 131: > >> 129: Graphics2D g2D = (Graphics2D) result.getGraphics(); >> 130: g2D.setRenderingHint(RenderingHints.KEY_ANTIALIASING, >> 131: RenderingHints.VALUE_ANTIALIAS_OFF); > > This hasn't been agreed upon (yet). > > I prefer the style where the wrapped parameters are aligned to the start of > the expression. That is the way it was before. Generally, I'd avoid modifying > lines you don't change. > > What do other reviewers think? Used IDE alignment for conveniences, its a side effect of that. - PR Review Comment: https://git.openjdk.org/jdk/pull/17608#discussion_r1484081884
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v4]
> 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: Review comments integrated - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/6e00b7fe..90367b92 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=02-03 Stats: 91 lines in 6 files changed: 21 ins; 23 del; 47 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v3]
> 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: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/8f6e94c3..61f3a837 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=01-02 Stats: 228 lines in 6 files changed: 52 ins; 135 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v3]
On Wed, 31 Jan 2024 23:07:45 GMT, Harshitha Onkar wrote: >> Updating the size impacting layouts, its visible in my system. > > With the present frame size and layout settings, Set Paper" and "Print" > options are not visible. > >  Yes, Mac have an issue because of button size and spacing, in window layouts are proper. I have updated the size with 700x500 and removed text box size setting for avoiding the clipping. - PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1474170976
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v3]
> 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: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/bf42b41e..dd31c19f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=01-02 Stats: 44 lines in 5 files changed: 8 ins; 14 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v2]
On Wed, 31 Jan 2024 23:07:45 GMT, Harshitha Onkar wrote: >> Updating the size impacting layouts, its visible in my system. > > With the present frame size and layout settings, Set Paper" and "Print" > options are not visible. > >  Seems like dpi/scale factor playing a role, In my windows machine  - PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1473932131
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v2]
On Wed, 31 Jan 2024 21:53:25 GMT, Alisen Chung wrote:
>> Renjith Kannath Pariyangad has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> Capitalized static final variables
>
> test/jdk/java/awt/print/PrinterJob/InvalidPage.java line 73:
>
>> 71: pJob.print();
>> 72: }
>> 73: } catch (PrinterException pe) {
>
> should this exception just be propagated into main?
This try catch inside an action listener, didn't find any way to propagate.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17608#discussion_r1473899350
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v2]
On Wed, 31 Jan 2024 21:51:16 GMT, Alisen Chung wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Capitalized static final variables > > test/jdk/java/awt/print/PrinterJob/DrawStringMethods.java line 70: > >> 68: .build(); >> 69: >> 70: PrinterJob pjob = PrinterJob.getPrinterJob(); > > i think this could be moved into a helper function and called by > passFailJFrame builder via .testui(DrawStringMethods::helperFunction) These class do not have any UI/frame, so I don't think it will workout here. - PR Review Comment: https://git.openjdk.org/jdk/pull/17608#discussion_r1473892594
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v3]
> 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: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/85cf485c..6e00b7fe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=01-02 Stats: 54 lines in 6 files changed: 5 ins; 22 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v2]
On Wed, 31 Jan 2024 09:22:41 GMT, Tejesh R wrote:
>> Renjith Kannath Pariyangad has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> Capitalized static final variables
>
> test/jdk/java/awt/print/PrinterJob/PrintNullString.java line 142:
>
>> 140: } catch (NullPointerException e) {
>> 141: g.drawString("caught expected NPE for null String,
>> int", 20, 40);
>> 142: }/* catch (Exception e) {
>
> I guess u can remove these instead of commenting? Any reason for commenting
> these instead of removing?
These lines are commented in existing code. Felt it may be useful for further
debugging in case of failure, so kept as it is.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17609#discussion_r1472645679
Re: RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 [v2]
> 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: Capitalized static final variables - Changes: - all: https://git.openjdk.org/jdk/pull/17609/files - new: https://git.openjdk.org/jdk/pull/17609/files/1785a63a..8f6e94c3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17609=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17609=00-01 Stats: 31 lines in 6 files changed: 0 ins; 12 del; 19 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v2]
On Tue, 30 Jan 2024 00:14:05 GMT, Harshitha Onkar wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Suggestions integrated > > 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); Updating the size impacting layouts, its visible in my system. - PR Review Comment: https://git.openjdk.org/jdk/pull/17607#discussion_r1470729593
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v2]
On Tue, 30 Jan 2024 06:30: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: > > Suggestions integrated @honkar-jdk Thank you for your review and valuable suggestions. I have updated the code - PR Comment: https://git.openjdk.org/jdk/pull/17607#issuecomment-1916197020
Re: RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 [v2]
> 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: Capitalized static final variables - Changes: - all: https://git.openjdk.org/jdk/pull/17608/files - new: https://git.openjdk.org/jdk/pull/17608/files/5522649d..85cf485c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17608=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17608=00-01 Stats: 48 lines in 6 files changed: 2 ins; 18 del; 28 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v2]
> 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: Suggestions integrated - Changes: - all: https://git.openjdk.org/jdk/pull/17607/files - new: https://git.openjdk.org/jdk/pull/17607/files/64b68c79..bf42b41e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17607=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17607=00-01 Stats: 120 lines in 5 files changed: 4 ins; 46 del; 70 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
RFR: 8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3
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 - Commit messages: - JDK-8324808 : Manual printer tests have no Pass/Fail buttons, instructions close set 3 Changes: https://git.openjdk.org/jdk/pull/17609/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17609=00 Issue: https://bugs.openjdk.org/browse/JDK-8324808 Stats: 1427 lines in 6 files changed: 203 ins; 841 del; 383 mod Patch: https://git.openjdk.org/jdk/pull/17609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17609/head:pull/17609 PR: https://git.openjdk.org/jdk/pull/17609
RFR: 8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2
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 - Commit messages: - JDK-8324807 : Manual printer tests have no Pass/Fail buttons, instructions close set 2 Changes: https://git.openjdk.org/jdk/pull/17608/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17608=00 Issue: https://bugs.openjdk.org/browse/JDK-8324807 Stats: 1181 lines in 6 files changed: 201 ins; 825 del; 155 mod Patch: https://git.openjdk.org/jdk/pull/17608.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17608/head:pull/17608 PR: https://git.openjdk.org/jdk/pull/17608
RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close
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 - Commit messages: - JDK-8320676 : Manual printer tests have no Pass/Fail buttons, instructions close Changes: https://git.openjdk.org/jdk/pull/17607/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17607=00 Issue: https://bugs.openjdk.org/browse/JDK-8320676 Stats: 1650 lines in 5 files changed: 401 ins; 885 del; 364 mod Patch: https://git.openjdk.org/jdk/pull/17607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17607/head:pull/17607 PR: https://git.openjdk.org/jdk/pull/17607
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v3]
On Fri, 26 Jan 2024 00:27:45 GMT, Harshitha Onkar wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Fixed whitespace errors > > @Renjithkannath There are many test changes in a single PR, it would be > easier to review if the tests are split into different PRs. For instance one > PR/JBS issue per 5-6 tests. @honkar-jdk Thank you for your suggestion will split this into 3 separate set for easy to review. Closing this - PR Comment: https://git.openjdk.org/jdk/pull/17565#issuecomment-1913961900
Withdrawn: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close
On Thu, 25 Jan 2024 04:52:15 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 This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/17565
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v3]
> 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: Fixed whitespace errors - Changes: - all: https://git.openjdk.org/jdk/pull/17565/files - new: https://git.openjdk.org/jdk/pull/17565/files/92720ce9..35c5c47e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17565=02 - incr: https://webrevs.openjdk.org/?repo=jdk=17565=01-02 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/17565.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17565/head:pull/17565 PR: https://git.openjdk.org/jdk/pull/17565
Re: RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close [v2]
> 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: Removed window close handler from Collate2DPrintingTest - Changes: - all: https://git.openjdk.org/jdk/pull/17565/files - new: https://git.openjdk.org/jdk/pull/17565/files/8d861959..92720ce9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17565=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17565=00-01 Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17565.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17565/head:pull/17565 PR: https://git.openjdk.org/jdk/pull/17565
RFR: 8320676 : Manual printer tests have no Pass/Fail buttons, instructions close
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 - Commit messages: - JDK-8320676 : Manual printer tests have no Pass/Fail buttons, instructions close Changes: https://git.openjdk.org/jdk/pull/17565/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17565=00 Issue: https://bugs.openjdk.org/browse/JDK-8320676 Stats: 4255 lines in 17 files changed: 805 ins; 2546 del; 904 mod Patch: https://git.openjdk.org/jdk/pull/17565.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17565/head:pull/17565 PR: https://git.openjdk.org/jdk/pull/17565
Integrated: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix
On Thu, 30 Nov 2023 03:34:08 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > There was a typo for color conversion instead of dstColorSpace function > srcColorSpace was used. Please review and let me know your suggestions if > any. > > Renjith. This pull request has now been integrated. Changeset: dadd9cd1 Author: Renjith Kannath Pariyangad Committer: Sergey Bylokhov URL: https://git.openjdk.org/jdk/commit/dadd9cd1e8434cffaafc7406a864eaa55954cfb8 Stats: 155 lines in 2 files changed: 151 ins; 0 del; 4 mod 8316497: ColorConvertOp - typo for non-ICC conversions needs one-line fix Co-authored-by: Sergey Bylokhov Co-authored-by: Alexey Ivanov Reviewed-by: aivanov, serb - PR: https://git.openjdk.org/jdk/pull/16895
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v16]
> Hi Reviewers, > There was a typo for color conversion instead of dstColorSpace function > srcColorSpace was used. Please review and let me know your suggestions if > any. > > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Fixed small typo - Changes: - all: https://git.openjdk.org/jdk/pull/16895/files - new: https://git.openjdk.org/jdk/pull/16895/files/895de12f..c0ac3b03 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16895=15 - incr: https://webrevs.openjdk.org/?repo=jdk=16895=14-15 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16895.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16895/head:pull/16895 PR: https://git.openjdk.org/jdk/pull/16895
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v15]
> Hi Reviewers, > There was a typo for color conversion instead of dstColorSpace function > srcColorSpace was used. Please review and let me know your suggestions if > any. > > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Updated the code with latest suggestion - Changes: - all: https://git.openjdk.org/jdk/pull/16895/files - new: https://git.openjdk.org/jdk/pull/16895/files/bec09a31..895de12f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16895=14 - incr: https://webrevs.openjdk.org/?repo=jdk=16895=13-14 Stats: 10 lines in 1 file changed: 0 ins; 6 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16895.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16895/head:pull/16895 PR: https://git.openjdk.org/jdk/pull/16895
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix
On Fri, 12 Jan 2024 02:55:59 GMT, Sergey Bylokhov wrote: >> @mrserb , >> >> Did you mean to use the wrapper for the middle like `ColorSpace mid = >> createCS(ColorSpaceSelector.WRAPPED_PYCC);` , So we can achieve : >> >> **From** >> wrapper->icc_color_space->wrapper >> icc_color_space->icc_color_space->icc_color_space >> >>**To** : >> wrapper->wrapper->wrapper >> icc_color_space->wrapper->icc_color_space >> >> Correct me if I am wrong > >>Did you mean to use the wrapper for the middle like ColorSpace mid = >>createCS(ColorSpaceSelector.WRAPPED_PYCC); , So we can achieve : > > Just repeat existed checks using wrapper. So you will have all combinations: > >> wrapper->icc_color_space->wrapper >> icc_color_space->icc_color_space->icc_color_space >> wrapper->wrapper->wrapper >> icc_color_space->wrapper->icc_color_space @mrserb thank you for clarifying, repeated the check for covering all cases please review. - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1888770449
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v14]
> Hi Reviewers, > There was a typo for color conversion instead of dstColorSpace function > srcColorSpace was used. Please review and let me know your suggestions if > any. > > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Included wrapper check - Changes: - all: https://git.openjdk.org/jdk/pull/16895/files - new: https://git.openjdk.org/jdk/pull/16895/files/6d47f16b..bec09a31 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16895=13 - incr: https://webrevs.openjdk.org/?repo=jdk=16895=12-13 Stats: 19 lines in 1 file changed: 16 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16895.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16895/head:pull/16895 PR: https://git.openjdk.org/jdk/pull/16895
Integrated: 8320673 : PageFormat/CustomPaper.java has no Pass/Fail buttons; multiple instructions
On Wed, 27 Dec 2023 04:20:42 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > > I have updated the test case. Now test has pass/fail option also test will > execute two time for covering different cases which are part of the test. > > Please review and let me know your suggestions. > > Regards, > Renjith. This pull request has now been integrated. Changeset: 0ff2deab Author: Renjith Kannath Pariyangad Committer: Sergey Bylokhov URL: https://git.openjdk.org/jdk/commit/0ff2deab5d6b8228ddfed9ae08820d2adf2330c2 Stats: 279 lines in 1 file changed: 82 ins; 144 del; 53 mod 8320673: PageFormat/CustomPaper.java has no Pass/Fail buttons; multiple instructions Reviewed-by: serb, aivanov - PR: https://git.openjdk.org/jdk/pull/17194
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix
On Wed, 10 Jan 2024 19:50:45 GMT, Sergey Bylokhov wrote: >> Can this patch be covered by the new test? > >>I'm not sure… We want the filter to take another path, there could be a list >>of filters applied, if I understand @mrserb correctly. Sergey could be able >>to provide a more detailed guidance. > > The current test validates two code paths: > - wrapper>icc_color_space->wrapper > - icc_color_space->icc_color_space->icc_color_space > > The color space in the middle is always icc_color_space: > https://github.com/openjdk/jdk/pull/16895/files#diff-70b19b2642d6d3f44904de8b6eb2993e1c97320e3476898c4372db364c4288b7R130 > > If we will use the wrapper for the middle as well we will cover this code > path: > https://github.com/openjdk/jdk/pull/16895/files#diff-e3d6eea060882cab00827c00e1a83b0e0a5b2a31fa9a9aa2122841bbd57c4a6dL853 @mrserb , Did you mean to use the wrapper for the middle like `ColorSpace mid = createCS(ColorSpaceSelector.WRAPPED_PYCC);` , So we can achieve : **Current** **New** wrapper->icc_color_space->wrapper> wrapper->wrapper->wrapper icc_color_space->icc_color_space->icc_color_space > icc_color_space->wrapper->icc_color_space Correct me if I am wrong - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1886405188
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v13]
> Hi Reviewers, > There was a typo for color conversion instead of dstColorSpace function > srcColorSpace was used. Please review and let me know your suggestions if > any. > > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision: Fixed other two typos - Changes: - all: https://git.openjdk.org/jdk/pull/16895/files - new: https://git.openjdk.org/jdk/pull/16895/files/1788ef69..6d47f16b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16895=12 - incr: https://webrevs.openjdk.org/?repo=jdk=16895=11-12 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16895.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16895/head:pull/16895 PR: https://git.openjdk.org/jdk/pull/16895
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v6]
On Fri, 5 Jan 2024 16:16:51 GMT, Alexey Ivanov wrote: >> @mrserb for avoiding _ArrayIndexOutOfBoundsException_ I have modified >> `CS_GRAY `with `CS_LINEAR_RGB ` for making same number of channels. But the >> test passed irrespective of fix. >> Do you find any alternative way to fail the test with out fix ? > >> There are at least two similar copy/paste typos. In the next line >> >> https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L771 >> we use the srcNumComp which is the number of components for the incoming >> source color space, but the loop should be done over iccSrcNumComp which is >> a number of components of the "intermidiate" source color space. Similar >> issue is in the next line but for the destination >> >> https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L785 >> >> It is possible that these compensate/affects each other and produce some >> non-completly broken result. The test from the comment above start to pass >> after these will be fixed. > > Indeed, these should be `iccSrcNumComp` and `iccDstNumComp` according to > number of components in the allocated arrays. Modifying these lines make the > test pass. > >> Since we found a few bugs in this code path, it will be useful to check the >> code path in the nonICCBIFilter where the CSList is not null(the >> intermidiate transform is wrapper as well, as of now our test uses the >> built-in ColorSpace.getInstance(CS_sRGB)) > > Agree, another (unit) test is required which exercises other code paths. These typos also fixed - PR Review Comment: https://git.openjdk.org/jdk/pull/16895#discussion_r1445952849
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix
On Mon, 8 Jan 2024 13:53:21 GMT, Alexey Ivanov wrote: >>> I think if condition missed `not (!)` (original code return _true_ if match >>> and _false_ if docent, in Sergey sample `compareImages ` returns _true_ if >>> not match and _false_ if match but `if (compareImages(destTest, destGold))` >>> same. @mrserb hope this observation is correct. >> >> I believe Sergey's code in the test is correct. There are two images: >> `destTest` and `destGold`. The former is produced using the wrapper class, >> the latter is produced using the ICC color space. The wrapper >> `TestColorSpace` forwards all the calls to the `ICC_ColorSpace` that it >> wraps, therefore the transformed images must be *equal* because the applied >> transforms are absolutely the same. >> >> To avoid any confusion, I suggest renaming `compareImages` to >> `areImagesEqual` which leaves no ambiguity for its return value. I've >> updated the code in [commit >> `1788ef6`](https://github.com/aivanov-jdk/jdk/commit/1788ef69958cefef7c65fdf37607f0410b868aff). >> >>> Thank to @mrserb and @aivanov-jdk for your time and investigation, this >>> helped to bring up few more issues. What do you suggest to cover these >>> cases ? >> >> I'm not sure… We want the filter to take another path, there could be a list >> of filters applied, if I understand @mrserb correctly. Sergey could be able >> to provide a more detailed guidance. > >> To avoid any confusion, I suggest renaming `compareImages` to >> `areImagesEqual` which leaves no ambiguity for its return value. I've >> updated the code in [commit >> `1788ef6`](https://github.com/aivanov-jdk/jdk/commit/1788ef69958cefef7c65fdf37607f0410b868aff). > > @Renjithkannath, you can merge the changes I made into your branch using the > following command: > > > git pull https://github.com/aivanov-jdk/jdk.git > renjith/sergey-8316497-colorConvertOp > > > The command will merge the changes from the remote branch into your currently > checked out branch. @aivanov-jdk Thank you for sharing the updated code I have merged your changes from the branch and pushed in. - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1882425761
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v12]
> Hi Reviewers, > There was a typo for color conversion instead of dstColorSpace function > srcColorSpace was used. Please review and let me know your suggestions if > any. > > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with three additional commits since the last revision: - Rename compareImages to areImagesEqual compareImages is ambiguous. Does it return true when images equal? The name areImagesEqual clearly conveys the intended return values, one doesn't need to look into its code to understand what's expected. Thus, the return values in the method are reversed, and the condition of the test is negated. - Improvements to Sergey's test Use enum instead of two booleans to select ColorSpace. Create the ColorSpace and pass it to createImage. Use constants to control the size of the image. Print the position of the failed pixel. - 8316497: Sergey's version of the non-ICC test - Changes: - all: https://git.openjdk.org/jdk/pull/16895/files - new: https://git.openjdk.org/jdk/pull/16895/files/56ce5c5e..1788ef69 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16895=11 - incr: https://webrevs.openjdk.org/?repo=jdk=16895=10-11 Stats: 60 lines in 1 file changed: 28 ins; 6 del; 26 mod Patch: https://git.openjdk.org/jdk/pull/16895.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16895/head:pull/16895 PR: https://git.openjdk.org/jdk/pull/16895
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v11]
On Fri, 5 Jan 2024 15:49:56 GMT, Alexey Ivanov wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Removed unwanted converion > > I've been thinking about it more, and I agree with Sergey. There are two > types of `ColorSpace`s: ICC and non-ICC. The `TestColorSpace` is a wrapper on > top of an `ICC_ColorSpace` (with Sergey's implementation that forwards all > the method calls to its wrapped `ColorSpace` instance stored in the `cs` > field. > > The expectation is that using the same ColorSpace result in the same output. > We — _Sergey_ — do just that. > > 1. Take two images with an `ICC_ColorSpace` and transform them. > 2. Take another pair of images with the wrapped `ICC_ColorSpace` and > transform them. > > The result of the transformation must be the same, right? It's a better test. > > I've created > [`renjith/sergey-8316497-colorConvertOp`](https://github.com/aivanov-jdk/jdk/tree/renjith/sergey-8316497-colorConvertOp) > branch. It's based on Renjith's > `[8316497-v1](https://github.com/Renjithkannath/jdk/tree/8316497-v1)` which > corresponds to this PR. > > Sergey's code from [his > comment](https://github.com/openjdk/jdk/pull/16895#discussion_r1441366764) > above is available as [commit > `ac6b10f`](https://github.com/aivanov-jdk/jdk/commit/ac6b10fa82e64934b46a642ad9a87d64d80affbf). > > I refactored it by introducing an enum for selecting the `ColorSpace`, it > makes it easier to understand which `ColorSpace` is created. I modified > `createTestImage` to accept a `ColorSpace`, this way it's not hidden inside > but explicitly pass, which also improves the readability of the test. This is > [commit > `8e85480`](https://github.com/aivanov-jdk/jdk/commit/8e854807d63fe8dc5909c8562cb64cad53679749). > > Here's the link to my latest version of [the `NonICCFilterTest.java` > test](https://github.com/aivanov-jdk/jdk/blob/8e854807d63fe8dc5909c8562cb64cad53679749/test/jdk/java/awt/color/NonICCFilterTest.java). > > Indeed, without Renjith's fix, the test fails with > `ArrayIndexOutOfBoundsException`: > > > Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Index 1 > out of bounds for length 1 > at > java.desktop/java.awt.image.ColorConvertOp.nonICCBIFilter(ColorConvertOp.java:821) > at > java.desktop/java.awt.image.ColorConvertOp.filter(ColorConvertOp.java:275) > at NonICCFilterTest.main(NonICCFilterTest.java:132) > > > With the fix, the test still fails but differently: > > > x = 0, y = 0 > rgb1 = fff61041 > rgb2 = ff00ff00 > Exception in thread "main" java.lang.RuntimeException: Test failed > at NonICCFilterTest.main(NonICCFilterTest.java:138) > > > which means non-ICC color spaces aren't handled correctly. @aivanov-jdk , Thank you for the detailed investigation. Earlier I didn't do much investigation with or with out fix after getting _ArrayIndexOutOfBoundsException_. Now its narrow down as with fix there is no _array out of bound_ and with out there is. About the failure post fix : I think its because of typo, As per the logic comparing two image and if its not equal then test pass. I think if condition missed `not (!)` (original code return _true_ if match and _false_ if docent, in Sergey sample `compareImages ` returns _true_ if not match and _false_ if match but `if (compareImages(destTest, destGold))` same. @mrserb hope this observation is correct. Thank to @mrserb and @aivanov-jdk for your time and investigation, this helped to bring up few more issues. What do you suggest to cover these cases ? - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1880373713
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v6]
On Fri, 5 Jan 2024 03:32:10 GMT, Sergey Bylokhov wrote: >>> Still, the current test catches the typo in the code srcColorSpace → >>> dstColorSpace that's now replaced. From this point view, the stated bug is >>> fixed. >> >> But the implementation of TestColorSpacewe is wrong, and it may not work as >> expected after any other future updates. > > There are at least two similar copy/paste typos. > In the next line > https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L771 > we use the srcNumComp which is the number of components for the incoming > source color space, but the loop should be done over iccSrcNumComp which is a > number of components of the "intermidiate" source color space. Similar issue > is in the next line but for the destination > https://github.com/openjdk/jdk/blob/f73dbb985823d2d59bfcff8ba6951a0d9eb2cefc/src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java#L785 > > It is possible that these compensate/affects each other and produce some > non-completly broken result. > The test from the comment above start to pass after these will be fixed. > > Since we found a few bugs in this code path, it will be useful to check the > code path in the nonICCBIFilter where the CSList is not null(the intermidiate > transform is wrapper as well, as of now our test uses the built-in > ColorSpace.getInstance(CS_sRGB)) @mrserb for avoiding _ArrayIndexOutOfBoundsException_ I have modified `CS_GRAY `with `CS_LINEAR_RGB ` for making same number of channels. But the test passed irrespective of fix. Do you find any alternative way to fail the test with out fix ? - PR Review Comment: https://git.openjdk.org/jdk/pull/16895#discussion_r1442869387
