On Fri, 20 Sep 2024 02:36:34 GMT, Jay Bhaskar <jbhas...@openjdk.org> wrote:
>> Successfully converted Non-parametrized base tests to JUnit 5 > > Jay Bhaskar has updated the pull request incrementally with one additional > commit since the last revision: > > one missing , formating looked at each line, double checking: - before->beforeEach etc. - exception types in assertThrows Could not run the junit4 query because there are still parameterized tests left. left a few comments, plus some tests need to be brought back. otherwise looks good. modules/javafx.base/src/test/java/test/com/sun/javafx/binding/ContentBindingListTest.java line 203: > 201: assertFalse(golden.equals(ContentBinding.bind(op2, op3))); > 202: ContentBinding.unbind(op2, op3); > 203: } several lines seem to have indentation problem. could you check please? modules/javafx.base/src/test/java/test/com/sun/javafx/binding/ErrorLoggingUtiltity.java line 28: > 26: package test.com.sun.javafx.binding; > 27: > 28: import static org.junit.jupiter.api.Assertions.*; Noticed the copyright year has not been updated in this file (and I am sure in others too). Should it be? modules/javafx.base/src/test/java/test/javafx/collections/VetoableObservableListTest.java line 151: > 149: ListIterator<String> it = list.listIterator(); > 150: it.next(); > 151: assertThrows(NullPointerException.class, () -> it.set(null)); even though this is not an equivalent change, I think it actually makes the test better. good job! ------------- PR Review: https://git.openjdk.org/jfx/pull/1576#pullrequestreview-2319200535 PR Review Comment: https://git.openjdk.org/jfx/pull/1576#discussion_r1769170527 PR Review Comment: https://git.openjdk.org/jfx/pull/1576#discussion_r1769172840 PR Review Comment: https://git.openjdk.org/jfx/pull/1576#discussion_r1769191221
