RFR: 8283664: Remove jtreg tag manual=yesno for java/awt/print/PrinterJob/PrintTextTest.java [v2]
Alexey Ivanov
aivanov at openjdk.org
Tue Nov 5 16:46:32 UTC 2024
On Thu, 31 Oct 2024 21:36:01 GMT, Daniel Gredler <duke at openjdk.org> wrote:
>> There are multiple issue with this test case
>> 1) Parser error due to yesno in @run main/manual=yesno
>> 2) User can only compare the UI rendering and compare with the print out. User can't mark the test as pass or fail due to pass or fail buttons are missing.
>> 3) When the test is executed using jtreg after user click on the print button on the print dialog the whole test UIs ( frames) gets dispose and user cannot compare the printout with the UI. But this works as expected in test is running individually using java PrintTextTest
>
> Daniel Gredler has updated the pull request incrementally with one additional commit since the last revision:
>
> Update copyright year, imports, array style, frame title
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/print/PrinterJob/PrintTextTest.java line 1:
> 1: /*
Could you make the `PrintJAText` class a nested static class inside `PrintTextTest`?
Yes, it requires more refactoring but this ensures no duplicate class names.
Since `PrintJAText` extends `PrintTextTest`, I suggest moving all the painting logic into `PrintText` class which will be a nested static class. Then `PrintJAText` (renamed to `PrintJapaneseText` if I understand the intent correctly) will extend `PrintText`.
All the other logic will remain unchanged.
Separating the painting logic from the test class itself, I believe, would also make thing clearer.
In addition to the above, let's resolve IDE warnings at lines 349 and 411 (in the current revision):
- byte data[] = new byte[s.length()];
+ byte[] data = new byte[s.length()];
- TextLayout tl = new TextLayout(s, new HashMap(), frc);
+ TextLayout tl = new TextLayout(s, new HashMap<>(), frc);
Overall, the changes look good to me. Thank you for your contribution.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21716#pullrequestreview-2416174421
PR Review Comment: https://git.openjdk.org/jdk/pull/21716#discussion_r1829683897
More information about the client-libs-dev
mailing list