On Wed, 26 May 2021 09:14:35 GMT, Jeanette Winzenburg <[email protected]>
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
