On Fri, 25 Sep 2020 16:47:37 GMT, Leon Linhart <github.com+4029915+themrmilchm...@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"); assertTrue(r); ------------- PR: https://git.openjdk.java.net/jfx/pull/284
