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
+179 −4
Draft

SetupJenkinsfile fails for plugin kryptowire #1611

Show file tree
Hide file tree
Changes from 4 commits
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
17 changes: 14 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,14 @@ 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?

}
try {
plugin.collectMetadata(mavenInvoker);
if (plugin.hasErrors()) {
Expand All @@ -463,7 +472,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
100 changes: 100 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,104 @@ 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");

// Initially empty, then has JDK 8 after verifyQuickBuild
// Return empty for first few calls, then JDK 8 for subsequent calls
java.util.Set<io.jenkins.tools.pluginmodernizer.core.model.JDK> emptySet = java.util.Set.of();
java.util.Set<io.jenkins.tools.pluginmodernizer.core.model.JDK> jdk8Set =
java.util.Set.of(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8);

when(metadata.getJdks())
.thenReturn(emptySet) // First check in collectMetadata
.thenReturn(emptySet) // Second part of isEmpty check
.thenReturn(jdk8Set) // After verifyQuickBuild, min() call
.thenReturn(jdk8Set); // Any additional calls

// hasErrors() is checked after verifyQuickBuild
when(plugin.hasErrors()).thenReturn(false);

// Mock the exception on first collectMetadata 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:
// 1. First try with JDK 25 (empty metadata)
// 2. Build with JDK 8 to generate classes
// 3. Then collect metadata with JDK 8 (minimum from metadata)
verify(plugin).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).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_8);
verify(plugin, times(2)).collectMetadata(mavenInvoker);
}
}
Loading