JDK-8131896 : Run gradle test tasks using a Jigsaw (jake) build
  • Type: Bug
  • Component: javafx
  • Sub-Component: build
  • Affected Version: 9-repo-jigsaw
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-07-18
  • Updated: 2016-01-14
  • Resolved: 2015-12-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9-repo-jigsawFixed
Related Reports
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Blocks :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
Running "gradle test" fails due to the following gradle bug:

https://issues.gradle.org/browse/GRADLE-3287

As such we need a way to run the tests that doesn't hit this bug. Either forking off an "ant" task and running the tests from ant, or possibly by launching our own test harness to replace gradle's default test runner (if they support that).

Need to refactor to following module junit test packages - to separate white/black box so junit can work:
   X base
   X graphics
   X controls
   X fxml
   o fxpackager - in flux, just disable for now
   o jmx - no tests.
   o swing
   X web
   o systemTests
Comments
Basic Junit test functionality is present for base, graphics, controls. Other modules will be reenabled later as we complete refactoring efforts. Changeset: 2f11ff8a4ab3 Author: ddhill Date: 2015-12-23 09:44 -0500 URL: http://hg.openjdk.java.net/openjfx/sandbox-9-jake/rt/rev/2f11ff8a4ab3 8131896: Run gradle test tasks using a Jigsaw (jake) build Reviewed-by: kcr ! build.gradle + buildSrc/src/main/java/jarjar/org/gradle/process/internal/child/BootstrapSecurityManager.java + buildSrc/src/main/java/workaround/GradleJUnitWorker.java + modules/base/src/test/addExports + modules/controls/src/test/addExports + modules/graphics/src/test/addExports
23-12-2015

Thanks Kevin for the comments - have integrated them dropping some of the code as suggested. re for #6 (module-info.class). From what I can tell, a module-info in the patch area is always ignored in favor of the existing definition in the runtime - except possibly if we use the version++ mechanism which I have not included. Retesting on Linux before pushing.
23-12-2015

Outstanding! It will be nice to get this in. I found two bugs and have a few minor comments: 1. You need to disable the fxml tests if IS_JDK9_TEST since they aren't ready to run as JDK9_TESTS 2. USE_JDK9 is set as a String property but used as if it were a boolean. I recommend changing the set to: if (JDK9_HOME == null || JDK9_HOME == "") { logger.warn("JDK9_HOME is not set"); ext.USE_JDK9 = false } else { ext.USE_JDK9 = true } since I don't think it needs to be command line overridable (if it does, then you need to use the "IS_" pattern and create an IS_USE_JDK9 boolean var) +1 with the bug fixes (no need for a new webrev) ------------------- Here are some minor comments and questions, none of which need to be fixed before pushing (or you can fix the simple typos along with the bug fixes without a new webrev). 3. Should the following default to false now? +defineProperty("FORCE_TESTS", "true") 4. Could the following be done by logging with a loglevel of "info"? + if (IS_SHOW_TESTS) { + beforeTest { descriptor -> + println("Running test: " + descriptor) + } + } 5. Typo in the following: //FIXME: suppert JDK9 should be "support" 6. Do you not need module-info.class? Without this, I guess it will use the module-info from the JDK9_HOME you are using. This is probably something to note and fix in the future (as it stands it requires a JDK9_HOME that contains javafx modules, and it might fail if you are testing a build with changes to module-info). 7. Minor formatting nits for the two new class files: A) There should be a blank line between the copyright header block and the package name; B) the comment block describing the function should be after the imports (could be in the class-level javadoc block).
22-12-2015

Kevin, Please review this JDK9/Jake specific change: Cannot run gradle test tasks using a Jigsaw (jake) build Note, that this change only enabled JDK9 based testing for base, graphics, controls. Coming soon: fxml and web which have been refactored and will be enabled later. SystemTest refactoring is in progress. http://cr.openjdk.java.net/~ddhill/8131896
22-12-2015

Not blocked, but certainly would like it.
19-09-2015

The solution to classpath lengths is probably JDK-8027634 (@argfile). I was not aware of this detail, but am now. Unfortunately this is not quite in jake yet, so hacky workarounds may be the order of the day.
27-07-2015

One caution I should mention now. The reason the original BootstrapSecurityManager was created was the problem of command line lengths. It is easy to see how a big project or a deep filesystem could create a really, really long class path chain. My workaround assumes (perhaps badly) that our test class paths will not exceeded some magic number of characters.
27-07-2015

Good news - I have my proof of concept working with a java only path. I found that if I add -Xbootclasspath/a: pointing to my jar, override the hardcoded -Djava.security.manager=jarjar.org.gradle.process.internal.child.BootstrapSecurityManager with my own stub and then slip in my class WorkaroundWrapper as the executed class then I can build the desired command that can use Jake and then execute gradel's worker in another new process (jarjar.org.gradle.process.internal.launcher.GradleWorkerMain) and things just work in my simple standalone proof of concept space. Now to integrate this mess into the JFX gradle build. There I need to build a jar that contains my two override classes, and then conditionally use them in test{}. easy :-)
25-07-2015

Here is the output of my showme bash script, which just reported the arguments passed. Not all of them seem to be adjustable: Executed as ./showme -Djava.security.manager=jarjar.org.gradle.process.internal.child.BootstrapSecurityManager -Dproperty=value -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant -ea -cp /Users/ddhill/.gradle/caches/2.5/workerMain/gradle-worker.jar jarjar.org.gradle.process.internal.launcher.GradleWorkerMain 'Gradle Test Executor 1' for example the -Djava.security.manager= appears to be hardcoded The key here is that BootstrapSecurityManager is fed the proposed classpath via stdin, and then needs to execute GradleWorkerMain I have been rooting through the gradle sources to try to understand what overrides are built in, and so far don't have a great solution.
24-07-2015

So is Gradle walking the class loader hierarchy to find the application class loader? If it calls getSystemClassLoader (which it appears to do) then it will get MyLoader rather than the built-in app class loader.
24-07-2015

I still wonder whether you can leave the test executable to be "java" (from JDK 9 for running tests) and instead munge the arguments passed to java so that it runs your worker class instead of the default?
24-07-2015

There are several levels of complication here that I doubt an ordinary developer will encounter for a while. * we are bootstrap building with JDK 8, testing with JDK 9 + this is a new issue for us, because up to this point we have always run our tests with the same JDK used for building JFX + this is likely to stay this way until the Jake tool chain is finalized, and we work through any new issues (like gradle 1.8 does not like the java version string, but 2.5 is ok with it, so we would need up update our minimum). * gradle forks a worker process (currently all java) which: + needs to have a class path that matches the desired test suite, and they pass that classpath to the child which uses URLClassLoader to add to/update the working class path of the child. This is where the most obvious failure is as the default classloader is no longer URLClassLoader. + receives from the parent one or more test classes to run * we want the test worker to run as JDK 9, - with module overrides to allow testing the current JFX build - with overrides to allow for private access for testing - with overrides to allow for package scope (to allow tests to be in an existing package) - and hopefully at some point, the specific overrides for a test I found that there is a simple means to change the executable forked by gradle: test { executable ("_some_path_to_an_executable") } but this needs to be intelligent, as it receives first the desired classpath, and then the list of classes to tests. The first part is normally handled by org.gradle.process.internal.child.BootstrapSecurityManager, and the second by jarjar.org.gradle.process.internal.launcher.GradleWorkerMain (both are in the gradle cache .gradle/caches/1.8/workerMain/gradle-worker.jar). I have a proof of concept that replaces BootstrapSecurityManager and uses a bash wrapper script then java app to tie it all together. This proof was done using the gradle quickstart example, so will need a fair amount of work to be stuffed into the JFX build. Adding a bash script will be problematic across platforms, particularly windows. I have yet to figure out a way from gradle source to change this to a java application (with the need for more than a single argument). In addition to the BootstrapSecurityManager, there is also a fair amount of build.gradle that builds a classpath adding the right jfxrt.jar for example. This will need to be (conditionally ?) reworked as well.
24-07-2015

Thanks for the suggestion, but it doesn't look like that would be sufficient, since gradle also assumes that the application ClassLoader is a URLClassLoader. Here is the exception I get: :test Error occurred during initialization of VM java.lang.ClassCastException: sun.misc.ClassLoaders$AppClassLoader (in module: java.base) cannot be cast to java.net.URLClassLoader (in module: java.base) at jarjar.org.gradle.process.internal.child.BootstrapSecurityManager.checkPermission(BootstrapSecurityManager.java:58) at java.lang.SecurityManager.checkPropertyAccess(java.base@9.0/SecurityManager.java:1285) at java.lang.System.getProperty(java.base@9.0/System.java:722) at java.lang.ClassLoader.initSystemClassLoader(java.base@9.0/ClassLoader.java:1559) at java.lang.System.initPhase3(java.base@9.0/System.java:1286) :test FAILED
24-07-2015

I wonder if it possible to workaround the Gradle bug by running with -Djava.system.class.loader=MyLoader where MyLoader is a URLClassLoader like this: import java.net.URLClassLoader; import java.net.URL; public class MyLoader extends URLClassLoader { public MyLoader(ClassLoader parent) { super(new URL[0], parent); } }
24-07-2015