RFR: 8353309: Open source several Swing text tests

Alexey Ivanov aivanov at openjdk.org
Wed Apr 2 16:16:51 UTC 2025


On Tue, 1 Apr 2025 18:13:09 GMT, Phil Race <prr at openjdk.org> wrote:

> Open source several Swing text / html related regression tests.

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/text/BoxView/BaselineTest.java line 114:

> 112:             doc.insertString(doc.getLength(), " Large Size Text ", set);
> 113:         } catch (BadLocationException ble) {
> 114:         }

`BadLocationException` is not expected. If it occurs, the test should fail. To avoid adding throws clauses, `BadLocationException` should be re-thrown wrapped in `RuntimeException`.

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.

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.

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.

test/jdk/javax/swing/text/html/FormView/4473401/bug4473401.java line 76:

> 74:     public void hyperlinkUpdate(HyperlinkEvent e) {
> 75:         if (e instanceof FormSubmitEvent) {
> 76:             jep.setText("If you see this page the test<font color=green> PASSED</font></CENTER>");

Does it mean, the test is semi-automatic, and you can call `PassFailJFrame.forcePass()` in this case?

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.

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.

-------------

PR Review: https://git.openjdk.org/jdk/pull/24364#pullrequestreview-2736844154
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025156716
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025161080
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025169641
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025167183
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025165536
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025172107
PR Review Comment: https://git.openjdk.org/jdk/pull/24364#discussion_r2025174815


More information about the client-libs-dev mailing list