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