On Wed, 2 Apr 2025 16:04:31 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Open source several Swing text / html related regression tests.
>
> test/jdk/javax/swing/text/BoxView/BaselineTest.java line 119:
>
>> 117: }
>> 118:
>> 119: class PaintLabel extends JLabel {
>
> Could you please move all the supporting classes `PaintLabel`,
> `CustomComponentView`, `CustomEditorKit` into the main test class
> `BaselineTest` as static nested classes.
>
> This greatly simplifies working with test in IDE, there'll be no duplicate
> classes.
I guess I can but I don't know where you are seeing duplicate classes.
> test/jdk/javax/swing/text/html/FormView/4473401/bug4473401.java line 51:
>
>> 49: """;
>> 50:
>> 51: static volatile JEditorPane jep;
>
> There's no need for the `volatile` modifier. `JEditorPane` is created on EDT,
> the `hyperlinkUpdate` listener is called on EDT.
ok
> test/jdk/javax/swing/text/html/FormView/4473401/bug4473401.java line 67:
>
>> 65: jep.setPage(file.toURL());
>> 66: } catch (Exception e) {
>> 67: }
>
> If an exception occurs, the test is unusable. The exception should be wrapped
> into `RuntimeException` and re-thrown.
ok ...
> test/jdk/javax/swing/text/html/FormView/4473401/frameset.html line 11:
>
>> 9: <FRAME name="main" SRC="frame2.html">
>> 10: </FRAMESET>
>> 11: </HTML>
>
> Having a blank line in the end of the file, that is ending the file with
> `\n`, is recommended, though not required.
I'm just moving these files. I don't see a problem here.
> test/jdk/javax/swing/text/html/FormView/bug4529702.java line 56:
>
>> 54: public static void main(String[] args) throws Exception {
>> 55: PassFailJFrame.builder()
>> 56: .title(" Test Instructions")
>
> `"Test Instructions"` is the default title, or rather `<testname> - Test
> Instructions` when run with jtreg, thus it can be omitted.
ok
> test/jdk/javax/swing/text/html/FrameSetView/4890934/bug4890934.java line 63:
>
>> 61: jep.setPage(file.toURL());
>> 62: } catch (Exception e) {
>> 63: }
>
> The test should fail if any exception occurs, silently ignoring the exception
> isn't good.
updated
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025467805
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025470309
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025469734
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025470894
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025472514
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025473107
