On Fri, 2 Oct 2020 16:23:23 GMT, Kevin Rushforth <[email protected]> wrote:
>> yosbits has refreshed the contents of this pull request, and previous >> commits have been removed. The incremental views >> will show differences compared to the previous content of the PR. > > modules/javafx.base/src/test/java/test/com/sun/javafx/collections/ObservableListWrapperTest.java > line 110: > >> 108: assertTrue(list.retainAll(Collections.EMPTY_SET)); >> 109: assertEquals(0, list.size()); >> 110: } > > Can you add a test for `retainAll` (in a separate test method) that removes > multiple elements in a single call? > Something like the following: > * set list to `(1, 2, 3, 4, 5)` > * remove `(2, 4)` > * verify that the list equals `(1, 3, 5)` > * remove `(1, 5)` > * verify that the list equals `(3)` > > Similarly, a test for `retainAll` that removes multiple elements in a single > call? > > * set list to `(1, 2, 3, 4, 5)` > * retain `(1, 3, 5)` > * verify that the list equals `(1, 3, 5)` > * retain `(3)` > * verify that the list equals `(3)` wondering why a new test? All concrete implementations are tested in the parameterized (on the implementations) ObservableListTest - would suggest to add to it what's not yet done (there are several around removeAll, a bit scarce on retainAll). Also would be easier to include notification testing (by following the example of the available tests) which should be done as well as the state testing (don't expect much trouble, though). ------------- PR: https://git.openjdk.java.net/jfx/pull/305
