On Sat, 3 Oct 2020 10:09:40 GMT, Jeanette Winzenburg <faste...@openjdk.org> 
wrote:

>> 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).

That's a good suggestion.

-------------

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

Reply via email to