On Mon, 26 Apr 2021 11:34:34 GMT, Jeanette Winzenburg <faste...@openjdk.org> 
wrote:

>> Florian Kirmaier has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8264127:
>>   Added missing test case
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java
>  line 921:
> 
>> 919:         assertFalse("cell must not be editing if cell index is " + 
>> cell.getIndex(), cell.isEditing());
>> 920:         assertEquals(1, events.size());
>> 921:     }
> 
> still missing: assert that list editingIndex is unchanged (see the assertTo 
> test). Without that assert, you don't see any difference in the test between 
> your previous and this commit (which was surrounding the call to cancelEdit 
> with setting the updateEditingIndex flags), do you ;) 
> 
> A personal note: tests are our friends, red-green (repeatedly) the color 
> **sequence** that brings satisfaction :) Just changing a code snippet without 
> making certain to have a failing test before (and passes after) a change is 
> .. well .. suboptimal.

Yes, i can confirm that it's required to find the issue with the 
updateEditingIndex. I've added more tests and in general its now more symmetric 
to the other test!

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

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

Reply via email to