[11] RFR: 8202553: Update FXLauncherTest as part of removing JavaFX from JDK (original) (raw)

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Thu May 10 13:58:06 UTC 2018

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 TESTSRC = System.getProperty("test.src"); Since this test extends TestHelper, it already inits a global constant TESTSOURCEDIR 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