-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update the minimum version of Jenkins used by RealJenkinsRule to prevent detached plugins being dragged in. #779
Conversation
the current baseline caused some detached plugins to be installed when starting a RealJenkinsRule. This has an undersirable effect that when `includeTestClasspathPlugins(false)` is called we still end up with some plugins that can negatively impact the test. To avoid this we are simply moving the baseline forward to 2.361 (which is what the j-t-h specifies as its Jenkins version!)
a094e3e
to
cc092ef
Compare
<relativePath /> | ||
</parent> | ||
<groupId>io.jenkins.plugins</groupId> | ||
<artifactId>RealJenkinsRuleInit</artifactId> | ||
<version>0-SNAPSHOT</version> | ||
<packaging>hpi</packaging> | ||
<properties> | ||
<jenkins.version>2.249</jenkins.version> | ||
<java.level>8</java.level> | ||
<jenkins.version>2.361</jenkins.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version has no detached plugins as of today, and matches the Jenkins version that the j-t-h itself uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this reveals at least one test was relying on this dragging in detached plugins (instance-identity
) as it was creating classic inbound agents which would fail with this update until instance-identity
was added back as a test plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was surprised that the earlier version was not based on an LTS and neither is the new version. Is there a specific reason this does not use an LTS version?
Does not block the merge, I would like to understand if there was something subtle in the choice of a weekly release instead of an LTS release.
Answered by reading your earlier comment where you said:
matches the Jenkins version that the j-t-h itself uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed for the parent bump
<relativePath /> | ||
</parent> | ||
<groupId>io.jenkins.plugins</groupId> | ||
<artifactId>RealJenkinsRuleInit</artifactId> | ||
<version>0-SNAPSHOT</version> | ||
<packaging>hpi</packaging> | ||
<properties> | ||
<jenkins.version>2.249</jenkins.version> | ||
<java.level>8</java.level> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newer version uses java11 and can not set the java.level
to 11
as it causes unknown java version 1.11
when compiling.
so updated the parent
As observed in https://github.com/jenkinsci/jenkins/pull/9351/checks?check_run_id=25813003911 the RealJenkinsRule was targetting an version of Jenkins core that required detached plugins.
This updates the version in the
RealJenkinsRuleInit
plugin to a version that does not require detached plugins.Whilst this is an interim solution to the problem, the currently selected version made no sense to me as it was lower than the Jenkins version specified for the actual test harness.
Longer term (if needed) the test HPI could be unpacked, and have its
MANIFEST.mf
optionally updated to match the Jenkins version under testincremental consumed in jenkinsci/jenkins#9351
see also #780
Testing done
Submitter checklist