On Thu, 21 May 2020 07:55:16 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
> As discussed > [here](https://github.com/openjdk/jfx/pull/201#issuecomment-621772586) during > #201 , This PR converts the > system tests in TabPanePermuteGetTabsTest.java to unit test. > Thanks @kleopatra , for providing the test. I have added few more tests and > found few failures. > [JDK-8245528](https://bugs.openjdk.java.net/browse/JDK-8245528) is created to > fix these failures, the failing tests are > currently ignored using this bug. > Please take a look. I'll do a more careful review + test later. I left a few inline comments below. modules/javafx.controls/src/main/java/javafx/scene/control/skin/TabPaneSkin.java line 2293: > 2292: > 2293: void test_disableAnimations() { > 2294: closeTabAnimation.set(TabAnimation.NONE); Might be better to change this to `test_setAnimations(TabAnimation mode)` so the test can restore it in the cleanup method. modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TabPaneSkinShim.java line 41: > 40: > 41: public static void disableAnimations(TabPaneSkin tpSkin) { > 42: tpSkin.test_disableAnimations(); See comment in `TabPaneSkin.java` modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TabPaneSkinHeaderOrderTest.java line 25: > 24: */ > 25: package test.javafx.scene.control.skin; > 26: Minor: add blank line between the copyright header and package. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TabPaneSkinHeaderOrderTest.java line 52: > 51: * updating the tab headers. > 52: * > 53: */ Minor: this blank comment line can be removed. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TabPaneSkinHeaderOrderTest.java line 274: > 273: public void cleanup() { > 274: stage.hide(); > 275: } I recommend to restore animation here. ------------- PR: https://git.openjdk.java.net/jfx/pull/230
