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 all 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
38 changes: 34 additions & 4 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 @@ -361,8 +361,12 @@ private void process(Plugin plugin) {
}

// Recollect metadata after modernization
if (!config.isFetchMetadataOnly()) {
plugin.withJDK(JDK.JAVA_25);
if (!config.isFetchMetadataOnly() && plugin.getMetadata() != null) {
// Use the minimum JDK from metadata that was successfully used to build the plugin
// If no JDKs in metadata, determine based on Jenkins version
JDK jdkForMetadata = JDK.min(
plugin.getMetadata().getJdks(), plugin.getMetadata().getJenkinsVersion());
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 +448,21 @@ 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 metadata exists yet
// If metadata already has JDKs, use minimum but ensure at least JDK 17 for OpenRewrite compatibility
PluginMetadata metadata = plugin.getMetadata();
if (metadata == null || metadata.getJdks() == null || metadata.getJdks().isEmpty()) {
plugin.withJDK(JDK.JAVA_25);
} else {
// Use minimum JDK from metadata, but ensure it supports OpenRewrite (JDK 17+)
JDK minJdk = JDK.min(metadata.getJdks());
if (minJdk.getMajor() < 17) {
// If minimum JDK is less than 17, use JDK 17 or higher based on Jenkins version
plugin.withJDK(JDK.min(metadata.getJdks(), metadata.getJenkinsVersion()));
} else {
plugin.withJDK(minJdk);
}
}
try {
plugin.collectMetadata(mavenInvoker);
if (plugin.hasErrors()) {
Expand All @@ -463,7 +481,19 @@ 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
// Ensure metadata exists and use Jenkins version compatibility if needed
PluginMetadata retryMetadata = plugin.getMetadata();
JDK jdkForRetry;
if (retryMetadata != null
&& retryMetadata.getJdks() != null
&& !retryMetadata.getJdks().isEmpty()) {
jdkForRetry = JDK.min(retryMetadata.getJdks(), retryMetadata.getJenkinsVersion());
} else {
// Fallback: use the JDK that was used for the quick build retry
jdkForRetry = JDK.JAVA_8;
}
plugin.withJDK(jdkForRetry);
plugin.collectMetadata(mavenInvoker);
} else {
LOG.info("Failed to collect metadata for plugin {}. Not retrying.", plugin.getName());
Expand Down
145 changes: 145 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,149 @@ 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);
}

@Test
void testCollectMetadata_WithEmptyJdksInRetry_ShouldUseMavenDefault() 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");

// getJdks() returns empty throughout (simulating a plugin with no JDK info)
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 defaults to JDK 25 when no JDK info exists
verify(plugin).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_25);
verify(plugin).collectMetadata(mavenInvoker);
}

@Test
void testCollectMetadata_WithNullMetadata_ShouldUseJdk25() throws Exception {
// Setup
Plugin plugin = mock(Plugin.class);
when(plugin.getName()).thenReturn("test-plugin");

// Plugin metadata is null (simulating first collection before metadata exists)
when(plugin.getMetadata()).thenReturn(null);
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 defaults to JDK 25 when metadata is null
verify(plugin).withJDK(io.jenkins.tools.pluginmodernizer.core.model.JDK.JAVA_25);
verify(plugin).collectMetadata(mavenInvoker);
}
}
Loading