On Wed, 26 May 2021 09:14:35 GMT, Jeanette Winzenburg <faste...@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