RFR: 8234194: [TEST_BUG] Reenable few graphics unit tests
Ambarish Rapte
arapte at openjdk.org
Fri Nov 15 10:37:55 UTC 2019
On Fri, 15 Nov 2019 10:34:58 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
> On Thu, 14 Nov 2019 23:55:45 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>
>> On Thu, 14 Nov 2019 19:56:16 GMT, Ambarish Rapte <arapte at openjdk.org> 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
More information about the openjfx-dev
mailing list