Skip to content

Conversation

@MarkEWaite
Copy link
Contributor

@MarkEWaite MarkEWaite commented Apr 6, 2022

Test Java 17 in additional to Java 11 and Java 8

Use Java 17 in the Jenkinsfile to compile and test.

Tests are failing with a serialization error. I need more guidance on the changes needed to fix the failing tests.

Checklist

  • I have read the CONTRIBUTING doc
  • Unit tests pass locally with my changes
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • Documentation in README has been updated as necessary
  • Online help has been added and reviewed for any new or modified fields
  • I have interactively tested my changes
  • Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

  • Dependency or infrastructure update

Jenkinsfile Outdated
configurations: [
[platform: 'linux', jdk: '11'],
[platform: 'windows', jdk: '8'],
[platform: 'linux', jdk: '11', jenkins: '2.342'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean

Suggested change
[platform: 'linux', jdk: '11', jenkins: '2.342'],
[platform: 'linux', jdk: '17', jenkins: '2.342'],

?

On the mailing list you posted a stack trace from CliGitSCMTriggerLocalPollTest but of course that is not visible in CI because you are not actually running Java 17.

Copy link
Contributor Author

@MarkEWaite MarkEWaite Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that. Made the same change in multiple repositories and failed to finish the job in this one.

Fixed the mistake in c8e4e1f

@jglick
Copy link
Member

jglick commented Apr 6, 2022

Running locally I do see

java.io.IOException: java.lang.RuntimeException: Failed to serialize hudson.model.AbstractProject#scm for class hudson.model.FreeStyleProject
	at hudson.XmlFile.write(XmlFile.java:220)
	at hudson.model.AbstractItem.save(AbstractItem.java:617)
	at hudson.model.Job.save(Job.java:193)
	at hudson.model.AbstractProject.save(AbstractProject.java:289)
	at hudson.model.AbstractProject.setScm(AbstractProject.java:1506)
	at hudson.plugins.git.AbstractGitProject.setupProject(AbstractGitProject.java:193)
	at hudson.plugins.git.SCMTriggerTest.check(SCMTriggerTest.java:244)
	at hudson.plugins.git.SCMTriggerTest.testNamespaces_with_refsHeadsMaster(SCMTriggerTest.java:63)
	at …
Caused by: java.lang.RuntimeException: Failed to serialize hudson.model.AbstractProject#scm for class hudson.model.FreeStyleProject
	at …
Caused by: java.lang.RuntimeException: Failed to serialize hudson.plugins.git.GitSCM#userRemoteConfigs for class hudson.plugins.git.GitSCM
	at …
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field protected transient int java.util.AbstractList.modCount accessible: module java.base does not "opens java.util" to unnamed module @38a8f1a9
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:178)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:172)
	at com.thoughtworks.xstream.converters.reflection.FieldDictionary.buildDictionaryEntryForClass(FieldDictionary.java:176)
	at com.thoughtworks.xstream.converters.reflection.FieldDictionary.buildMap(FieldDictionary.java:142)
	at com.thoughtworks.xstream.converters.reflection.FieldDictionary.fieldsFor(FieldDictionary.java:80)
	at com.thoughtworks.xstream.converters.reflection.PureJavaReflectionProvider.visitSerializableFields(PureJavaReflectionProvider.java:167)
	at hudson.util.RobustReflectionConverter.doMarshal(RobustReflectionConverter.java:206)
	at …

I do see earlier in the build

[INFO] Setting jenkins.addOpens to --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED

as expected. The test passes with

diff --git pom.xml pom.xml
index 34e29fa1..afe0d153 100644
--- pom.xml
+++ pom.xml
@@ -62,7 +62,6 @@
     <changelist>-SNAPSHOT</changelist>
     <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-    <argLine>-Dfile.encoding=${project.build.sourceEncoding}</argLine>
     <jenkins.version>2.289.1</jenkins.version>
     <no-test-jar>false</no-test-jar>
     <useBeta>true</useBeta>

Not sure why

git-plugin/pom.xml

Lines 64 to 65 in 1435a5e

<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<argLine>-Dfile.encoding=${project.build.sourceEncoding}</argLine>
are even here to begin with. Added in #862 for no obvious reason. I think both could be deleted.

@basil is it known that an explicit argLine is not compatible with the Java 17 changes as of jenkinsci/plugin-pom#524? As in jenkinsci/plugin-pom#453 I think the idea is that plugins should configure the Surefire mojo where needed to add system properties, rather than overriding this property. If confirmed, should be mentioned in https://github.com/jenkinsci/plugin-pom/releases/tag/plugin-4.40.

@jglick
Copy link
Member

jglick commented Apr 6, 2022

(Or we could introduce a property like surefire.argLine.basic with default JVM settings that you could override, and include that in argLine, or better yet directly in Surefire <configuration>.)

Basil explained:

> [You are] overriding the <argLine> from the plugin parent POM in your
> plugin POM. This override is not extending the original value but
> replacing it, just as overriding a method in a subclass without calling
> super() in Java code would replace the original method rather than
> extend it. As of 4.39 this meant that your plugin was blanking out the
> -Xms, -Xmx, -XX:HeapDumpOnOutOfMemoryError, -XX:TieredCompilation, and
> -XX:TieredStopAtLevel settings from the plugin parent POM – incorrect,
> but harmless. As of 4.40, this means your plugin is blanking out the
> @{jenkins.addOpens} and @{jenkins.insaneHook} settings from the plugin
> parent POM – incorrect, and harmful when running on Java 17.

Nice improvement to remove an unhelpful setting.
Earlier change did not actually test Java 17.
@github-actions github-actions bot added the dependencies Dependency related change label Apr 6, 2022
@MarkEWaite
Copy link
Contributor Author

Also removed the source encoding setting without seeing any negative affect on compile time messages. See e711ad8

Java 17 does not accept it
@MarkEWaite MarkEWaite merged commit 4d91d24 into jenkinsci:master Apr 6, 2022
@MarkEWaite MarkEWaite deleted the test-java-17 branch April 6, 2022 21:17
@MarkEWaite MarkEWaite mentioned this pull request Apr 7, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency related change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants