RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v4]

Marius Hanl mhanl at openjdk.java.net
Sun Jul 11 18:30:52 UTC 2021


On Fri, 9 Jul 2021 09:58:38 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> Hmm, but leaving a test without an assert is also bad. You have any suggestions?
>> I may can add another editable test, which will pass before and after.
>
>> 
>> 
>> Hmm, but leaving a test without an assert is also bad. You have any suggestions?
> 
> Not aware of such a rule - if we fix code throwing an exception there is not much to assert, except that it fails before and passes after. And paddling back a bit, I think a separate test for the back switch would be overdoing it :) 
> 
>          @Test 
>          ...
>          // configure: just as you do
>          comboBox.setEditable(true)
>          ...
>          // the test: just as you do - switch to false
>          comboBox.setEditable(false)
>          // safe-guard against future implementation changes: switch back to true
>          comboBox.setEditable(true)
>          // end of test

There is no rule, but in my opinion it is also not a good practise. 
I agree it's also not a big problem, but in general a test should check something. 
And I think it's not a problem to check, that we still have no value inside ComboBox, even though is not really related. 

In JUnit 5, there would be `assertDoesNotThrow​()` for that usecase. But I may found a solution for our version, there is  `@Test(expected = Test.None.class)`, which basically tells us explicitly, that we don't expect any exception (and so, the code under test did throw an exception one time in the past).

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

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


More information about the openjfx-dev mailing list