On Fri, 15 Nov 2019 10:34:58 GMT, Ambarish Rapte <[email protected]> wrote:
> On Thu, 14 Nov 2019 23:55:45 GMT, Kevin Rushforth <[email protected]> wrote: > >> On Thu, 14 Nov 2019 19:56:16 GMT, Ambarish Rapte <[email protected]> wrote: >> >>> Following graphics unit tests can be re-enabled. >>> >>> 1. >>> test.com.sun.javafx.scene.layout.region.BackgroundRepeatConverterTest.scenario2 >>> RepeatStructConverter.convert() is an internal method and does not verify >>> the parameters. Passing null value to this method results in NPE. >>> >>> 2. test.javafx.scene.layout.RegionCSSTest.borderImageWidth_auto >>> test.javafx.scene.layout.RegionCSSTest.borderImageWidth_1_auto >>> test.javafx.scene.layout.RegionCSSTest.borderImageWidth_1_2Percent_auto >>> It is not certain if the value of auto for -fx-border-image-width is 1 or 2 >>> by defaut. But the tests seem correct and they pass. >>> >>> 3. test.javafx.scene.Node_cssMethods_Test >>> [JDK-8094155](https://bugs.openjdk.java.net/browse/JDK-8094155) is fixed >>> and the tests can be re-enabled. >>> >>> ---------------- >>> >>> Commits: >>> - 5df00884: Reenable few of the graphics unit tests >>> >>> Changes: https://git.openjdk.java.net/jfx/pull/39/files >>> Webrev: https://webrevs.openjdk.java.net/jfx/39/webrev.00 >>> Issue: https://bugs.openjdk.java.net/browse/JDK-8234194 >>> Stats: 12 lines in 3 files changed: 0 ins; 7 del; 5 mod >>> Patch: https://git.openjdk.java.net/jfx/pull/39.diff >>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/39/head:pull/39 >> >> Most of this looks OK, but there is one I had a question on. >> >> Have you run this on all platforms? >> >> modules/javafx.graphics/src/test/java/test/com/sun/javafx/scene/layout/region/BackgroundRepeatConverterTest.java >> line 57: >> >>> 56: @Test(expected=NullPointerException.class) >>> 57: public void scenario2() { >>> 58: ParsedValue<String,BackgroundRepeat>[][] values = new >>> ParsedValueImpl[][] { >> >> This is at odds with the body of the test, since presumably, the >> `assertEquals` will never be called. >> >> An expect exception annotation is best used in the case where you do a >> single thing that is expected to generate that exception. Otherwise and >> unexpected exception could end up making the test look like it's passing. > > Hi Kevin, the test corrected now. > Removed `(expected=NullPointerException.class)` and added try, catch block. > Have you run this on all platforms? Yes, The change is verified on platforms. and It shows same result. tests: 22988, fails: 0, ignored: 279 Also I have included few more tests to enable, please take a look. PR: https://git.openjdk.java.net/jfx/pull/39
