On Mon, 21 Sep 2020 11:19:30 GMT, yosbits
<github.com+7517141+yos...@openjdk.org> wrote:
>> modules/javafx.base/src/main/java/com/sun/javafx/collections/ObservableListWrapper.java
>> line 191:
>>
>>> 189: if (this.isEmpty() || c.isEmpty()) {
>>> 190: return false;
>>> 191: }
>>
>> logic seems wrong: if c is empty this must be cleared
>>
>> The error shows up in a failing test (but only when running all
>> base.collections tests in Eclipse, not with gradle on
>> the commandline)
>> the test failure is in ObservableListWithExtractor.testUpdate_retainAll()
>> which - besides exposing the error above -
>> tells us two issues with the tests
>> a) the normal (that is no extractor) test is incomplete (in not testing
>> retainAll with empty array/collection)
>> b) the test is not run with gradle because it doesn't adhere to naming
>> conventions ;)
>
> I realized I was wrong. Thank you. I would probably need to add a test for
> this case.
> I plan to push new changes around tomorrow.
I found that I needed to add at least a retainAll test, but I also found that
there was no test class to directly test
ObservableListWrapper.
Should I add a test for this non-public API class?
When I add it, I'm not going to write test code for methods other than
removeAll and retainAll.
-------------
PR: https://git.openjdk.java.net/jfx/pull/305
