RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v6]

Michael Strauß mstrauss at openjdk.java.net
Fri May 28 23:35:43 UTC 2021


On Wed, 26 May 2021 09:14:35 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> Yes, `selectedItems` is broken. I agree that having an objectively incorrect test pass is fishy, but the alternative would be to not have the test at all, and lose the "free" heads-up if at some point in the future the underlying issue is fixed. Do you think it's better to have the correct test, but ignore it?
>
> well, false greens are the worst we can have, IMO :). I think the test should concentrate on the isolated selectedItems behavior which __must__ propagate all notifications that it receives (from indices) in terms of items. If it fails to do so, it's a failure of its own implementation and should produce a failing test. 
> 
> Consider two scenarios: 
> 
> A: indices firing 2 separate changes
> 
>     selectedIndices.replaceAll(i -> i == 0 ? 1 : 0);
>     assertEquals(2, changes.size());
>     assertEquals(change(replaced(0, range("foo"), range("bar"))), changes.get(0));
>     assertEquals(change(replaced(1, range("bar"), range("foo"))), changes.get(1));
> 
> B: indices firing a composed change:
> 
>     selectedIndices._beginChange();
>     selectedIndices.replaceAll(i -> i == 0 ? 1 : 0);
>     selectedIndices._endChange();
>     assertEquals(indicesChanges.size(), changes.size());
>     assertEquals(1, changes.size());
>     assertEquals(change(replaced(0, range("foo", "bar"), range("bar", "foo"))), changes.get(0));
> 
> B passes both before and after the fix, A fails both before and after the fix. Whether or not that can be helped currently, is a different story. If can't be solved right now, I would suggest to keep it failing, file an issue about it and ignore the test with the issue id.

I disabled the tests until the underlying [issue](https://bugs.openjdk.java.net/browse/JDK-8267951) is fixed.

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

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


More information about the openjfx-dev mailing list