[11] RFR: 8202553: Update FXLauncherTest as part of removing JavaFX from JDK
Kevin Rushforth
kevin.rushforth at oracle.com
Thu May 10 12:59:44 UTC 2018
Here is the updated webrev:
http://cr.openjdk.java.net/~kcr/8202553/webrev.01/
As discussed offline, this strips down the Mock LauncherImpl class to
just allow testing of the inputs. There is no need for it to try to
mimic the behavior of the FX launcher. It is sufficient to test the
arguments to the launchAplication method. This caused only minor changes
to the test.
I also addressed the other feedback.
-- Kevin
On 5/9/2018 4:32 AM, Kevin Rushforth wrote:
> I'll work up a new version of the webrev that addresses your feedback,
> and strip down the mockfx classes to the minimum needed to support the
> test cases.
>
> -- Kevin
>
>
> On 5/8/2018 3:52 PM, Kumar Srinivasan wrote:
>> Hi Kevin,
>>
>>> Please review the following test fix:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8202553
>>> http://cr.openjdk.java.net/~kcr/8202553/webrev.00/
>>
>> FXLauncherTest.java:
>>
>> 57 private static final String TEST_SRC =
>> System.getProperty("test.src");
>> Since this test extends TestHelper, it already inits a global
>> constant TEST_SOURCE_DIR
>> http://hg.openjdk.java.net/jdk/jdk/file/06d5b1f66553/test/jdk/tools/launcher/TestHelper.java#l120
>>
>> --------
>> 209 // javac -d mods/javafx.graphics mockfx/src/javafx.graphics/**
>> is not quite accurate does not mention the --source-path
>> --------
>>
>> Prefer to avoid array copies in favor of List/ArrayList
>> 228 System.arraycopy(compilerArgs, 0, fxCompilerArgs, 2,
>> compilerArgs.length); ----- 237 System.arraycopy(cmds, 1, fxCmds, 3,
>> cmds.length - 1); ditto. -----
>> Mock JavaFX:
>> test/jdk/tools/launcher/mockfx/src/javafx.graphics/com/sun/javafx/application/*
>> I have a general concern with the above classes, it seems to be
>> overly complicated for a simple launcher test(s) to prevent
>> regressions. I think this should be simply testing the logic in
>> LauncherHelper.FXHelper, specifically this table:
>> https://java.se.oracle.com/source/xref/jdk-jdk/open/src/java.base/share/classes/sun/launcher/LauncherHelper.java#852
>> Alan, Mandy, what is your take on Mock JavaFX ?
>> Thanks
>> Kumar
>>
>>> This modifies the existing FXLauncherTest as follows:
>>>
>>> 1. Reverse the check for the presence of the
>>> javafx.application.Application class and fail the test if present
>>>
>>> 2. Create a "mock" javafx.graphics module with a mocked up version
>>> of the few classes needed to validate the FX launcher functionality
>>>
>>> 3. Remove the "intermittent" and "headful" keywords, since neither
>>> apply any more
>>>
>>> 4. Remove the test from the problem list
>>>
>>> Thanks.
>>>
>>> -- Kevin
>>>
>>
>
More information about the core-libs-dev
mailing list