-
Notifications
You must be signed in to change notification settings - Fork 126
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
[JENKINS-48885] - Fix getPlugin() in JenkinsRule #92
base: master
Are you sure you want to change the base?
Conversation
pom.xml
Outdated
<!--to have access to User.getById--> | ||
<version>1.651.2</version> | ||
<!--to run with plugin unbundling by default--> | ||
<version>2.7.3</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.
Then you would need to do some cleanup. TestPluginManager
has distinct modes for 1.x vs. 2.x.
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.
…which I guess did not get cleaned up in #135, so maybe do that here?
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 believe we will wipe 1.x entirely after https://groups.google.com/forum/#!topic/jenkinsci-dev/SdMOMKs7XIQ
@@ -89,6 +90,9 @@ public void setup(JenkinsRule jenkinsRule, WithPlugin recipe) throws Exception { | |||
public void decorateHome(JenkinsRule jenkinsRule, File home) throws Exception { | |||
for (String plugin : a.value()) { | |||
URL res = getClass().getClassLoader().getResource("plugins/" + plugin); | |||
if (res == null) { | |||
throw new IOException("Cannot find resource for plugin " + 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.
This only applies to tests actually annotated with @WithPlugin
. No effect on tests assuming plugin existence due to a test
-scoped dependency.
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.
Yes. It is not really related to the defect. Just better diagnostics for another issue I hit. Can detach it to a separate PR if you prefer
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 think I've been impacted by this issue too doing some tests in support-core plugin. Here the folder plugins doesn't exist: https://github.com/jenkinsci/support-core-plugin/blob/54fde93fa28ac1c69adc0a2a6c4962ca2c629b3c/src/main/java/com/cloudbees/jenkins/support/impl/AboutJenkins.java#L837
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.
IIRC the plugin registry is just not initialized in the Test plugin manager
I plan to revisit it using the approach suggested in jenkinsci/jenkinsfile-runner#389 and jenkinsci/jenkinsfile-runner#390. |
WiP for now, just to not forget about it.
https://issues.jenkins-ci.org/browse/JENKINS-48885
@reviewbybees