On Thu, 7 Nov 2019 07:03:47 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
> On Tue, 5 Nov 2019 15:43:03 GMT, Jeanette Winzenburg <faste...@openjdk.org> > wrote: > >> The pull request has been updated with additional changes. >> >> ---------------- >> >> Added commits: >> - 0366a0a5: added copyright header >> >> Changes: >> - all: https://git.openjdk.java.net/jfx/pull/20/files >> - new: https://git.openjdk.java.net/jfx/pull/20/files/aabea139..0366a0a5 >> >> Webrevs: >> - full: https://webrevs.openjdk.java.net/jfx/20/webrev.01 >> - incr: https://webrevs.openjdk.java.net/jfx/20/webrev.00-01 >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8231692 >> Stats: 22 lines in 1 file changed: 21 ins; 0 del; 1 mod >> Patch: https://git.openjdk.java.net/jfx/pull/20.diff >> Fetch: git fetch https://git.openjdk.java.net/jfx pull/20/head:pull/20 > > modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/KeyEventFirerTest.java > line 204: > >> 203: public void setup() { >> 204: // This step is not needed (Just to make sure StubToolkit is >> loaded into VM) >> 205: // @SuppressWarnings("unused") > > Remove this commented code. > > modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/KeyEventFirerTest.java > line 55: > >> 54: * Most of these tests are meant to document how to use the >> KeyEventFirer and what >> 55: * happens if used incorrectly. The latter are ignored, because the >> build should pass. >> 56: * > > I see ignored tests - for false greens. > As these tests are written for new code in KeyEventFirer test infrastructure > class, I suggest not to ignore these tests. Rather adjust asserts to pass > them. A comment explaining the expected result should be added. > > modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/KeyEventFirer.java > line 85: > >> 84: >> 85: public void doUpArrowPress(KeyModifier... modifiers) { >> 86: doKeyPress(KeyCode.UP, modifiers); > > Enclose in braces {} yeah, suspected that ignoring a test is not the right thingy to do (though there are precedences in the code base, that you are currently cleaning up ;) But couldn't come up with a passing test that demonstrates the failure of a false-green. Any ideas? Or alternatively: the basic idea in these ignored tests was to document the correct vs. incorrect useage - is there any place/internal documentation to reach that goal, other than a failing test? Maybe it's only me, but it took me quite a while to find out why that darn test (on comboBox) was _not_ failing, that is was a real false-green - which I would like to spare future contributors. PR: https://git.openjdk.java.net/jfx/pull/20
