On Mon, 23 Sep 2024 15:48:00 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

> #### Requirement:
> We want to be able to test a JavaFX SDK built somewhere else other than on 
> current machine or current repo on same machine. Which means we should be 
> able to run the unit and system tests with a different JavaFX SDK.
> 
> #### Change:
> ##### Run tests using a specified JavaFX SDK:
> - Introduce a property `TEST_JAVAFX_SDK_PATH` that points to the JavaFX SDK 
> which was built elsewhere.
> - Modify the paths generated by build.gradle to point to the path sepcified 
> by `TEST_JAVAFX_SDK_PATH`
> - When a `TEST_JAVAFX_SDK_PATH` is specified `-PTEST_ONLY=true` must also be 
> specified.
> 
> ##### Generate the JavaFX SDK bundle
> - Generate the minimal JavaFX SDK bundle that is required to run the tests: 
> We require content of the _build/sdk_ and _build/shims_ directories to 
> achieve the same.
> 
> 
> #### Verification:
> ##### With a single machine:
> 1. Create two local copies JavaFX repo, [repo1, repo2]
> 2. Build sdk and shims in repo1. `gradle sdk shims`
> 3. In repo2, run different tests as,
>> gradle -PTEST_ONLY=true -PTEST_JAVAFX_SDK_PATH=**repo1/build**  :base:test 
>> <any tests>
> 4. In repo1 run `gradle all`
> 5. `javafx-sdk-shims.zip` file would be created under repo1/build. unzip it 
> any `unzip-path` on machine
> 6. In repo2, run the tests by providing the `unzip-path` as 
> TEST_JAVAFX_SDK_PATH
>> gradle -PTEST_ONLY=true -PTEST_JAVAFX_SDK_PATH=**unzip-path**  :base:test 
>> <any tests>
> 
> ##### With 2 machines.
> 1. Run `gradle all` on a machine1
> 2. Move build/javafx-sdk-shims.zip to machine2 and unzip to any `unzip-path`
> 3. Run the tests by providing the `unzip-path` as TEST_JAVAFX_SDK_PATH
>> gradle -PTEST_ONLY=true -PTEST_JAVAFX_SDK_PATH=**unzip-path**  :base:test 
>> <any tests>
> 
> 
> #### Additional changes:
> Additionally we observed that three unit tests failed because they access a 
> specific path. A path that would no exists when JavaFX SDK is not built.
> 1. Controls test: test.javafx.scene.control.ControlTest.testRT18097
> It required path: 
> modules/javafx.controls/build/classes/java/main/javafx.controls/javafx
> It is fixed by change on line 2811
> 
> 2. Graphics test: test.javafx.css.CssMetaDataTest.testRT18097
> It required path: 
> modules/javafx.graphics/build/classes/java/main/javafx.graphics/javafx
> It is fixed by change on line 2722
> 
> 3. Base test: test.com.sun.javafx.runtime.VersionInfoTest
> It required path: build/module-lib/javafx.properties
> VersionInfoTest is specific to the local build of JavaFX. So, It can be 
> safely excluded when TEST_JAVAFX_SDK_PATH is specified.
> It is fixed by change on line 2304 t...

I tried the first recipe, with repo1 pointing to a full build of the current 
jfx master, and repo2 pointing to a clean repo with the patch from this PR. I 
deliberately did not build the sdk in repo2.

I was able to compile and run the tests from all projects except `javafx.swt`. 
Those tests are disabled (not run), but compilation of the tests is enabled, 
and it fails without having first compiled the "sdk" in repo2. Here is the 
error:


$ cd repo2
$ gradle --info --continue -PTEST_ONLY=true -PTEST_JAVAFX_SDK_PATH=repo1/build 
test
...
> Task :swt:compileTestJava FAILED
Caching disabled for task ':swt:compileTestJava' because:
  Build cache is disabled
  'Forking compiler via ForkOptions.executable' satisfied
Task ':swt:compileTestJava' is not up-to-date because:
  Task has failed previously.
The input changes require a full rebuild for incremental task 
':swt:compileTestJava'.
Compilation mode: command line compilation
Compiling with toolchain '/jdk-22.0.2.jdk/Contents/Home'.
Compiling with Java command line compiler 
'/jdk-22.0.2.jdk/Contents/Home/bin/javac'.
Starting process 'command '/jdk-22.0.2.jdk/Contents/Home/bin/javac''. Working 
directory: jfx/modules/javafx.swt Command: 
/jdk-22.0.2.jdk/Contents/Home/bin/javac 
@jfx/modules/javafx.swt/build/tmp/compileTestJava/java-compiler-args.txt
Successfully started process 'command '/jdk-22.0.2.jdk/Contents/Home/bin/javac''
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasScaledTest.java:43:
 error: package javafx.embed.swt does not exist
import javafx.embed.swt.FXCanvas;
                       ^
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasScaledTest.java:123:
 error: cannot find symbol
    private static void initFX(FXCanvas canvas) {
                               ^
  symbol:   class FXCanvas
  location: class FXCanvasScaledTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/SWTCursorsTest.java:28:
 error: package javafx.embed.swt does not exist
import javafx.embed.swt.FXCanvas;
                       ^
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasTest.java:28:
 error: package javafx.embed.swt does not exist
import javafx.embed.swt.FXCanvas;
                       ^
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasScaledTest.java:71:
 error: cannot find symbol
        final FXCanvas canvas = new FXCanvas(shell, SWT.NONE);
              ^
  symbol:   class FXCanvas
  location: class FXCanvasScaledTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasScaledTest.java:71:
 error: cannot find symbol
        final FXCanvas canvas = new FXCanvas(shell, SWT.NONE);
                                    ^
  symbol:   class FXCanvas
  location: class FXCanvasScaledTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/SWTCursorsTest.java:49:
 error: cannot find symbol
        final FXCanvas canvas = new FXCanvas(shell, SWT.NONE);
              ^
  symbol:   class FXCanvas
  location: class SWTCursorsTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/SWTCursorsTest.java:49:
 error: cannot find symbol
        final FXCanvas canvas = new FXCanvas(shell, SWT.NONE);
                                    ^
  symbol:   class FXCanvas
  location: class SWTCursorsTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasTest.java:47:
 error: cannot find symbol
        final FXCanvas canvas = new FXCanvas(shell, SWT.NONE);
              ^
  symbol:   class FXCanvas
  location: class FXCanvasTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasTest.java:47:
 error: cannot find symbol
        final FXCanvas canvas = new FXCanvas(shell, SWT.NONE);
                                    ^
  symbol:   class FXCanvas
  location: class FXCanvasTest
jfx/modules/javafx.swt/src/test/java/test/javafx/embed/swt/FXCanvasTest.java:55:
 error: cannot find symbol
        assertSame(canvas, FXCanvas.getFXCanvas(canvas.getScene()));
                           ^
  symbol:   variable FXCanvas
  location: class FXCanvasTest
11 errors
Resolve mutations for :swt:processTestResources (Thread[#65,included 
builds,5,main]) started.
:swt:processTestResources (Thread[#65,included builds,5,main]) started.

> Task :swt:processTestResources UP-TO-DATE
Caching disabled for task ':swt:processTestResources' because:
  Build cache is disabled
  Not worth caching
Skipping task ':swt:processTestResources' as it is up-to-date.

FAILURE: Build failed with an exception.


Unlike the other modules, the `javafx.swt` module is not loaded from the module 
path, so the absence of the sdk build in the repo you are testing causes the 
compilations of the javafx.swt tests to fail. Since they aren't being run, 
perhaps the best solution is to disable the compileTestJava task in the `:swt` 
project if `IS_TEST_JAVAFX_SDK` is true.

I left a minor naming suggestion for the name of the new gradle property.

build.gradle line 733:

> 731: 
> 732: ext.IS_TEST_JAVAFX_SDK = false
> 733: if (hasProperty("TEST_JAVAFX_SDK_PATH")) {

Suggestion: drop `JAVAFX_` from the name of this property and just call it 
`TEST_SDK_PATH`. That's closer to what we did in JDK 8 when we had a similar 
feature (it was called `TEST_SDK`, but I think adding `_PATH` as a suffix is 
good).

build.gradle line 745:

> 743:         fail("The provided TEST_JAVAFX_SDK_PATH=${TEST_JAVAFX_SDK_PATH} 
> is invalid")
> 744:     }
> 745:     ext.IS_TEST_JAVAFX_SDK = true

Similarly, you might consider removing `JAVAFX_` from this property, but it's 
an internal-only variable, so it doesn't matter.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1577#pullrequestreview-2323660081
PR Review Comment: https://git.openjdk.org/jfx/pull/1577#discussion_r1772318410
PR Review Comment: https://git.openjdk.org/jfx/pull/1577#discussion_r1772319485

Reply via email to