[11] RFR: 8202553: Update FXLauncherTest as part of removing JavaFX from JDK

[Kumar Srinivasan]

Hi Kevin,

Thanks for making the changes,  this is very  manageable and maintainable
going forward. :)

Looks good!.

Thanks
Kumar




On 5/10/2018 5:59 AM, Kevin Rushforth wrote:
> 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
>>>>
>>>
>>
>