Skip to content

SetupJenkinsfile fails for plugin kryptowire #1611

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.

Sign up for GitHub

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

Draft
akash-manna-sky wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

SetupJenkinsfile fails for plugin kryptowire #1611

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8eecf43
SetupJenkinsfile fails for plugin kryptowire
akash-manna-sky Mar 4, 2026
00db2c2
Refactor PluginModernizerTest to improve JDK usage verification and m…
akash-manna-sky Mar 4, 2026
94482d5
fix formatting issues
akash-manna-sky Mar 4, 2026
fb61f86
Merge branch 'main' into issue-1162
akash-manna-sky Mar 4, 2026
dce83fb
fix: Enhance JDK selection logic in metadata collection for plugins
akash-manna-sky Mar 4, 2026
aa33f94
fix style issue
akash-manna-sky Mar 4, 2026
226f6e0
Merge branch 'main' into issue-1162
akash-manna-sky Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions ...izer-core/src/main/java/io/jenkins/tools/pluginmodernizer/core/impl/PluginModernizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,9 @@ private void process(Plugin plugin) {

// Recollect metadata after modernization
if (!config.isFetchMetadataOnly()) {
plugin.withJDK(JDK.JAVA_25);
// Use the minimum JDK from metadata that was successfully used to build the plugin
JDK jdkForMetadata = JDK.min(plugin.getMetadata().getJdks());
Copy link
Collaborator

@jonesbusy jonesbusy Mar 4, 2026

Choose a reason for hiding this comment

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

What happen if we modernize to change JDK? The lowest will not be compatible anymore

plugin.withJDK(jdkForMetadata);
Comment on lines +365 to +369
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

In the post-modernization metadata recollection, this uses JDK.min(plugin.getMetadata().getJdks()) which may select an older/unsupported JDK (and also ignores the Jenkins version compatibility logic used elsewhere in verifyPlugin/compilePlugin). This can cause the subsequent metadata recollection build to run under a JDK that cannot verify/compile the modernized plugin. Consider reusing the JDK that successfully verified the plugin (returned by verifyPlugin) or computing a compatible JDK via JDK.min(metadata.getJdks(), metadata.getJenkinsVersion()) plus the same supported() adjustment.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@jonesbusy jonesbusy Mar 4, 2026

Choose a reason for hiding this comment

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

Copilot is right about JDK version

plugin.clean(mavenInvoker);
collectMetadata(plugin, false);
LOG.debug(
Expand Down Expand Up @@ -444,7 +446,13 @@ private void process(Plugin plugin) {
*/
private void collectMetadata(Plugin plugin, boolean retryAfterFirstCompile) {
LOG.trace("Collecting metadata for plugin {}... Please be patient", plugin.getName());
plugin.withJDK(JDK.JAVA_25);
// Use JDK 25 for initial metadata collection if no JDK is set yet
// If metadata already has JDKs, use the minimum one
if (plugin.getMetadata().getJdks() == null || plugin.getMetadata().getJdks().isEmpty()) {
plugin.withJDK(JDK.JAVA_25);
} else {
plugin.withJDK(JDK.min(plugin.getMetadata().getJdks()));
Copy link
Collaborator

@jonesbusy jonesbusy Mar 4, 2026

Choose a reason for hiding this comment

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

OpenRewrite will no run with JDK lower than 17. Any reason to use min?

}
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

collectMetadata() now dereferences plugin.getMetadata().getJdks() before metadata has necessarily been loaded/created. In process(), collectMetadata(plugin, true) is invoked specifically when !plugin.hasMetadata(), so plugin.getMetadata() can be null (CacheManager#get returns null on cache miss), leading to an immediate NullPointerException on first metadata collection. Handle plugin.getMetadata() == null by defaulting to JDK.JAVA_25 (or initializing metadata) before reading JDKs.

Copilot uses AI. Check for mistakes.
try {
plugin.collectMetadata(mavenInvoker);
if (plugin.hasErrors()) {
Expand All @@ -463,7 +471,9 @@ private void collectMetadata(Plugin plugin, boolean retryAfterFirstCompile) {
plugin.getName());
plugin.raiseLastError();
}
plugin.withJDK(JDK.JAVA_25);
// After successful build with JDK 8, use the minimum JDK from metadata for collection
JDK jdkForRetry = JDK.min(plugin.getMetadata().getJdks());
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

In the retryAfterFirstCompile path, jdkForRetry is computed from plugin.getMetadata().getJdks() before copyMetadata()/loadMetadata() has run. When collectMetadata() is called for a plugin with no cached metadata (common case), plugin.getMetadata() will still be null here, causing an NPE during the retry flow. Use a JDK derived from the retry build itself (e.g., the JDK used for verifyQuickBuild) or ensure metadata is loaded/initialized before accessing it.

Suggested change
// After successful build with JDK 8, use the minimum JDK from metadata for collection
JDK jdkForRetry = JDK.min(plugin.getMetadata().getJdks());
// After successful build with JDK 8, prefer the minimum JDK from metadata for collection,
// but fall back to the JDK used for the retry build if metadata is not yet available.
PluginMetadata metadata = plugin.getMetadata();
JDK jdkForRetry;
if (metadata != null && metadata.getJdks() != null && !metadata.getJdks().isEmpty()) {
jdkForRetry = JDK.min(metadata.getJdks());
} else {
// Fallback: use the JDK that was used for the quick build retry
jdkForRetry = JDK.JAVA_8;
}

Copilot uses AI. Check for mistakes.
plugin.withJDK(jdkForRetry);
plugin.collectMetadata(mavenInvoker);
} else {
LOG.info("Failed to collect metadata for plugin {}. Not retrying.", plugin.getName());
Expand Down
95 changes: 95 additions & 0 deletions ...-core/src/test/java/io/jenkins/tools/pluginmodernizer/core/impl/PluginModernizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,4 +367,99 @@ private Recipe createMockRecipe(String name, String description) {
when(recipe.getDescription()).thenReturn(description);
return recipe;
}

@Test
void testCollectMetadata_WithExistingJdks_ShouldUseMinimumJdk() throws Exception {
// Setup
Plugin plugin = mock(Plugin.class);
io.jenkins.tools.pluginmodernizer.core.extractor.PluginMetadata metadata =
mock(io.jenkins.tools.pluginmodernizer.core.extractor.PluginMetadata.class);
when(plugin.getMetadata()).thenReturn(metadata);
when(plugin.getName()).thenReturn("test-plugin");

// Plugin metadata has JDK 8 and JDK 11
when(metadata.getJdks())
.thenReturn(java.util.Set.of(
io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8,
io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_11));
when(plugin.hasErrors()).thenReturn(false);

// Execute - Invoke collectMetadata using reflection
java.lang.reflect.Method method =
PluginModernizer.class.getDeclaredMethod("collectMetadata", Plugin.class, boolean.class);
method.setAccessible(true);
method.invoke(pluginModernizer, plugin, false);

// Verify that plugin was set to use JDK 8 (minimum of 8 and 11)
verify(plugin).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8);
verify(plugin).collectMetadata(mavenInvoker);
verify(plugin).copyMetadata(cacheManager);
verify(plugin).loadMetadata(cacheManager);
verify(plugin).enrichMetadata(pluginService);
}

@Test
void testCollectMetadata_WithNoJdks_ShouldUseJdk25() throws Exception {
// Setup
Plugin plugin = mock(Plugin.class);
io.jenkins.tools.pluginmodernizer.core.extractor.PluginMetadata metadata =
mock(io.jenkins.tools.pluginmodernizer.core.extractor.PluginMetadata.class);
when(plugin.getMetadata()).thenReturn(metadata);
when(plugin.getName()).thenReturn("test-plugin");

// Plugin metadata has no JDKs initially
when(metadata.getJdks()).thenReturn(java.util.Set.of());
when(plugin.hasErrors()).thenReturn(false);

// Execute
java.lang.reflect.Method method =
PluginModernizer.class.getDeclaredMethod("collectMetadata", Plugin.class, boolean.class);
method.setAccessible(true);
method.invoke(pluginModernizer, plugin, false);

// Verify that plugin was set to use JDK 25 (default for initial collection)
verify(plugin).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_25);
verify(plugin).collectMetadata(mavenInvoker);
}
Comment on lines +401 to +423
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The new collectMetadata() logic now depends on plugin.getMetadata() being non-null. There is currently no test case covering the scenario where plugin.getMetadata() is null on first metadata collection (the common path when the cache is empty), which would have caught the introduced NPE risk. Add a unit test for collectMetadata() with plugin.getMetadata() == null to validate the intended default JDK selection and avoid regressions.

Copilot uses AI. Check for mistakes.

@Test
void testCollectMetadata_WithRetry_ShouldUseMinimumJdkAfterJdk8Build() throws Exception {
// Setup
Plugin plugin = mock(Plugin.class);
io.jenkins.tools.pluginmodernizer.core.extractor.PluginMetadata metadata =
mock(io.jenkins.tools.pluginmodernizer.core.extractor.PluginMetadata.class);
when(plugin.getMetadata()).thenReturn(metadata);
when(plugin.getName()).thenReturn("test-plugin");

// First call: no JDKs, then after JDK 8 build, metadata has JDK 8
when(metadata.getJdks())
.thenReturn(java.util.Set.of()) // First time - empty
.thenReturn(java.util.Set.of(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8)); // After JDK 8 build

// First attempt fails, second succeeds after JDK 8 build
when(plugin.hasErrors())
.thenReturn(true) // First attempt fails
.thenReturn(false); // After JDK 8 build succeeds

Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

This retry test stubs plugin.hasErrors() to return true on the first call, but in collectMetadata() that first hasErrors() check happens right after verifyQuickBuild(). If it returns true, collectMetadata() calls plugin.raiseLastError(), which on a real Plugin throws and aborts the retry path. As written, the mock will silently continue, so the test can pass while the real flow would fail. Adjust the stubbing so the quick build succeeds (hasErrors == false) and/or stub raiseLastError() to throw when hasErrors is true to better match production behavior.

Suggested change
// First attempt fails, second succeeds after JDK 8 build
when(plugin.hasErrors())
.thenReturn(true) // First attempt fails
.thenReturn(false); // After JDK 8 build succeeds
// Ensure quick build is treated as successful (no errors reported)
when(plugin.hasErrors())
.thenReturn(false);

Copilot uses AI. Check for mistakes.
// Mock the exception on first attempt
doThrow(new io.jenkins.tools.pluginmodernizer.core.model.ModernizerException("Build failed"))
.doNothing()
.when(plugin)
.collectMetadata(mavenInvoker);

// Execute - with retry flag
java.lang.reflect.Method method =
PluginModernizer.class.getDeclaredMethod("collectMetadata", Plugin.class, boolean.class);
method.setAccessible(true);
method.invoke(pluginModernizer, plugin, true);

// Verify sequence of JDK usage:
// 1. First try with JDK 25 (no metadata yet)
// 2. Build with JDK 8 to generate classes
// 3. Then collect metadata with JDK 8 (minimum from metadata)
verify(plugin, atLeastOnce()).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_25);
verify(plugin).verifyQuickBuild(mavenInvoker, io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8);
verify(plugin, atLeastOnce()).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8);
verify(plugin, times(2)).collectMetadata(mavenInvoker);
}
}
Loading