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

Reply via email to