This repository has been archived by the owner on Mar 5, 2023. It is now read-only.
Allow running tests through Gradle on JDK 10 #885
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pyokagan
force-pushed
the
jdk10
branch
3 times, most recently
from
51514a9 to
685e45f
Compare
v1@pyokagan submitted v1 for review.
Checkout this PR version locallygit fetch https://github.com/se-edu/addressbook-level4.git refs/pr/885/1/head:BRANCHNAMEwhere |
|
Running |
Nope (on Linux), otherwise I'd have known beforehand since I tested with
This suggests that this is a separate JavaFX-related problem. |
OK. Someone else with Windows will probably have to check whether they are getting the same issue then (to exclude the possibility that I screwed up the Java installation or something). |
|
Got the exact same error on Windows 10: |
|
Looks like a similar issue to this https://stackoverflow.com/questions/41065861/gradle-messageioexception-could-not-write-message-endofstream-to-127-0-0-1?rq=1 |
|
@pyokagan The maintainer of TestFX ran into a similar issue: javafxports/openjdk-jfx#66 According to this comment, it seems to be a bug on JavaFX end. Apparently they forgot to call some initialisation methods when using Windows' DirectWrite library (the GUI renderer used by JavaFX on Windows):
The current appveyor build shows traces of the error. Our previous PRs did not have appveyor failing, so migrating to JaCoCo 0.8.1 is partially responsible for the error. But if it is a JavaFX problem, I don't think we can do much about it, unless we can find some temporary workarounds. I guess that's the risk we take by upgrading to new technologies too soon, since their stability cannot be taken for granted. :P. |
Oh, I see. Nice find 👍
If you meant: That's because of TestFX/TestFX#553
That's because of #881 If @damithc restarts the build enough times, it should pass. Unfortunately it's quite hard to ensure PRs pass AppVeyor because if it stalls, I'll need to wait 1 hour for it to time out. |
|
I'll update the developer guide to strongly recommend Windows users to use JDK 9. |
|
https://docs.gradle.org/current/release-notes.html Btw... it seems that the wrapper is still using 4.6. Would upgrading to 4.7 helps? |
nvm tried it on my machine... doesn't work |
pyokagan
force-pushed
the
jdk10
branch
2 times, most recently
from
1eaee17 to
e8668d8
Compare
v2@pyokagan submitted v2 for review.
(📚 Archive) (📈 Interdiff between v1 and v2) Checkout this PR version locallygit fetch https://github.com/se-edu/addressbook-level4.git refs/pr/885/2/head:BRANCHNAMEwhere |
yamgent
approved these changes
Jul 2, 2018
Yeah, someone else will need to do this. I'm just volunteering in my free time now. |
|
build.gradle: need to change all references of gradle 4.6 to |
With JDK 10, `./gradlew allTest` fails to run tests, instead spewing out
the following errors on the console:
Exception in thread "main" java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:510)
FATAL ERROR in native method: processing of -javaagent failed
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:522)
Caused by: java.lang.RuntimeException: Class java/lang/UnknownError could not be instrumented.
at org.jacoco.agent.rt.internal_290345e.core.runtime.ModifiedSystemClassRuntime.createFor(ModifiedSystemClassRuntime.java:139)
at org.jacoco.agent.rt.internal_290345e.core.runtime.ModifiedSystemClassRuntime.createFor(ModifiedSystemClassRuntime.java:100)
at org.jacoco.agent.rt.internal_290345e.PreMain.createRuntime(PreMain.java:55)
at org.jacoco.agent.rt.internal_290345e.PreMain.premain(PreMain.java:47)
... 6 more
Caused by: java.lang.NoSuchFieldException: $jacocoAccess
at java.base/java.lang.Class.getField(Class.java:1958)
at org.jacoco.agent.rt.internal_290345e.core.runtime.ModifiedSystemClassRuntime.createFor(ModifiedSystemClassRuntime.java:137)
... 9 more
... and so forth
According to a PR[1] on the JaCoCo's issue tracker, this occurs because
"Classes of JDK 10 EA b35 use a new classfile version number and so
JaCoCo Agent 0.7.9 as well as agent from current build of master branch
fail to start". This has been fixed as of JaCoCo 0.8.1, which also
happens to be the latest release.
We do not explicitly specify the version JaCoCo to use in our
build.gradle file, and so the `jacoco` gradle plugin just uses its
default, which happens to be 0.8.0[2] on Gradle 4.6 (the version we are
using). This version of JaCoCo still has this bug, and so we get these
errors.
Fix this by upgrading to the latest version of Gradle (4.8.1). Since
Gradle 4.7[3] the default JaCoCo plugin has been upgraded to version
0.8.1, and so upgrading to the latest version of Gradle will fix this
problem and allow us to run tests on JDK 10.
The Gradle wrapper upgrade was performed by running the command:
./gradlew wrapper --gradle-version 4.8.1
and then converting the line endings of gradlew.bat to unix to satisfy
our travis line ending checks.
Also, update any references to the Gradle version we are using in the
code base to 4.8.1.
[1] jacoco/jacoco#629
[2] gradle/gradle@8a09484
[3] https://docs.gradle.org/4.7/release-notes.html#default-jacoco-version-upgraded-to-0.8.1
Due to a JavaFX bug[1], JDK 10 on Windows will fail to run tests in headless mode. As such, let's warn developers against using JDK 10 on Windows in our documentation, and recommend that they use JDK 9 instead. Some developers who have both JDK 9 and JDK 10 installed on their systems may have trouble coaxing Gradle to use JDK 10. As such, to make it easier to troubleshoot the setup of such developers, let's teach build.gradle to print a warning to console if it detects that it is running with JDK 10 on Windows. [1] javafxports/openjdk-jfx#66
v3@pyokagan submitted v3 for review.
(📚 Archive) (📈 Interdiff between v2 and v3) Checkout this PR version locallygit fetch https://github.com/se-edu/addressbook-level4.git refs/pr/885/3/head:BRANCHNAMEwhere |
damithc
approved these changes
Jul 4, 2018
pyokagan
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #884
Fixes #886