RFR: 8251946: ObservableList.setAll does not conform to specification [v3]

Kevin Rushforth kcr at openjdk.java.net
Sat Sep 26 13:55:29 UTC 2020

On Fri, 25 Sep 2020 16:47:37 GMT, Leon Linhart <github.com+4029915+TheMrMilchmann at openjdk.org> wrote:

>> Hi, this PR fixes [JDK-8251946](https://bugs.openjdk.java.net/browse/JDK-8251946) computing whether the list was
>> actually modified instead of just returning `true`. The list was modified if 1. it was not empty (modified by calling
>> `#clear()`), or if 2. it was modified as result of the `#addAll()` call.
>> If you want any test coverage for this please let me know.
>> I reported the issue a couple of days ago via web formula and waited for the confirmation to open this PR but now I see
>> that @kevinrushforth (sorry for pinging) is already assigned to the JBS issue. I'm not too familiar with how you handle
>> JBS issues or more specifically whether assigning is used to indicate that the issue is in the assignee's "domain", or
>> that the assignee is already working on it. In the latter case, please feel free to close this PR. (My OCA submission
>> is still pending anyway.)
> Leon Linhart has updated the pull request incrementally with one additional commit since the last revision:
>   Reverted incorrect change and improved test coverage

The code change looks good. I have one suggestion for the test.

modules/javafx.base/src/test/java/test/javafx/collections/ObservableListTest.java line 268:

> 266:
> 267:         r = list.setAll();
> 268:         assertTrue(r);

Can you also test that calling `setAll` when the list is currently empty returns true? Repeating one of the earlier
checks should work:

        r = list.setAll("one");


PR: https://git.openjdk.java.net/jfx/pull/284

More information about the openjfx-dev mailing list