On Sun, 11 Jul 2021 18:28:24 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>>> >>> >>> 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). agree to all that makes the test code easier to read/understand, the JUnit 5 looks good :) Your solution for JU4 isn't really easier to understand .. but could be only me, or simple enough to get used to. Just: adding an assert that basically asserts nothing or the wrong (because unspecified) thing is .. worse than a test that's hard to read, IMO ------------- PR: https://git.openjdk.java.net/jfx/pull/557
