On Tue, 1 Apr 2025 18:13:09 GMT, Phil Race <p...@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
