Skip to content

Commit 4bc9f87

Browse files
jonesbusyjonesbusy
committed
Better Jenkinsfile update support (ordering, windows check etc...)
1 parent 8363651 commit 4bc9f87

2 files changed

Lines changed: 129 additions & 14 deletions

File tree

‎plugin-modernizer-core/src/main/java/io/jenkins/tools/pluginmodernizer/core/recipes/UpdateJenkinsfileForJavaVersion.java‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import io.jenkins.tools.pluginmodernizer.core.model.PlatformConfig;
66
import io.jenkins.tools.pluginmodernizer.core.visitors.UpdateJenkinsFileVisitor;
77
import java.util.ArrayList;
8+
import java.util.Comparator;
89
import java.util.HashSet;
910
import java.util.List;
1011
import java.util.Set;
@@ -116,24 +117,21 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
116117
.anyMatch(p -> "linux".equalsIgnoreCase(p.name().toString())
117118
&& p.jdk().getMajor() == javaVersion);
118119

119-
if (hasJavaVersion) {
120+
// see if windows platform is already present
121+
boolean hasWindows = model.platformConfigs.stream()
122+
.anyMatch(p -> "windows".equalsIgnoreCase(p.name().toString()));
123+
124+
if (hasJavaVersion && (jdksToRemove == null || jdksToRemove.isEmpty())) {
120125
LOG.info("Java {} configuration already exists. No update needed.", javaVersion);
121126
return method;
122127
}
123128

124-
// Remove JDKs if specified on jdksToRemove
125-
if (jdksToRemove != null && !jdksToRemove.isEmpty()) {
126-
Set<Integer> jdksToRemoveSet = new HashSet<>(jdksToRemove);
127-
model.platformConfigs = model.platformConfigs.stream()
128-
.filter(p -> !jdksToRemoveSet.contains(p.jdk().getMajor()))
129-
.collect(Collectors.toList());
129+
// add the java version configuration to the model if not present
130+
if (!hasJavaVersion) {
131+
LOG.info("Adding Java {} configuration to Jenkinsfile.", javaVersion);
132+
model.platformConfigs.add(PlatformConfig.build(Platform.LINUX, jdkVersion));
130133
}
131134

132-
// add the java version configuration to the model.
133-
model.platformConfigs.add(PlatformConfig.build(Platform.LINUX, jdkVersion));
134-
135-
// limit the number of JDK configurations to totalJdk
136-
137135
// If we limit the number of JDK to save build resources
138136
if (totalJdk < DEFAULT_TOTAL_JDK) {
139137

@@ -152,6 +150,30 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
152150
}
153151
}
154152

153+
// Remove JDKs if specified on jdksToRemove
154+
if (jdksToRemove != null && !jdksToRemove.isEmpty()) {
155+
Set<Integer> jdksToRemoveSet = new HashSet<>(jdksToRemove);
156+
model.platformConfigs = model.platformConfigs.stream()
157+
.filter(p -> !jdksToRemoveSet.contains(p.jdk().getMajor()))
158+
.collect(Collectors.toList());
159+
160+
// If windows and more that one platform we need to ensure the lowest JDK is on windows platform
161+
if (hasWindows && model.platformConfigs.size() > 1) {
162+
PlatformConfig lowest = model.platformConfigs.stream()
163+
.min(Comparator.comparingInt(a -> a.jdk().getMajor()))
164+
.orElse(null);
165+
if (!"windows".equalsIgnoreCase(lowest.name().toString())) {
166+
model.platformConfigs.remove(lowest);
167+
model.platformConfigs.add(PlatformConfig.build(Platform.WINDOWS, lowest.jdk()));
168+
}
169+
}
170+
}
171+
172+
// Reorder by lowest JDK first
173+
model.platformConfigs = model.platformConfigs.stream()
174+
.sorted(Comparator.comparingInt(a -> a.jdk().getMajor()))
175+
.collect(Collectors.toList());
176+
155177
// We pass it the complete, updated configuration, and it will overwrite the old method call.
156178
// in future at some point remove jdk 17 from the configurations
157179
doAfterVisit(

‎plugin-modernizer-core/src/test/java/io/jenkins/tools/pluginmodernizer/core/recipes/UpdateJenkinsfileForJavaVersionTest.java‎

Lines changed: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,107 @@ void shouldUpgradeLegacyConfigAndAddJava25() {
3838
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
3939
configurations: [
4040
[platform: 'linux', jdk: 8],
41-
[platform: 'linux', jdk: 11],
4241
[platform: 'windows', jdk: 8],
42+
[platform: 'linux', jdk: 11],
4343
[platform: 'windows', jdk: 11],
4444
[platform: 'linux', jdk: 25],
4545
])
4646
""", sourceSpecs -> sourceSpecs.path(ArchetypeCommonFile.JENKINSFILE.getPath())));
4747
}
4848

49+
@Test
50+
void shouldRemoveOldJDKFromLegacyConfig() {
51+
rewriteRun(
52+
spec -> spec.recipe(new UpdateJenkinsfileForJavaVersion(25, List.of(8, 11, 17))),
53+
// language=groovy
54+
groovy("""
55+
buildPlugin(
56+
dontRemoveMe: 'true',
57+
forkCount: '1C',
58+
jdkVersions: ['8', '11'],
59+
platforms: ['linux', 'windows']
60+
)
61+
""", """
62+
/*
63+
See the documentation for more options:
64+
https://github.com/jenkins-infra/pipeline-library/
65+
*/
66+
buildPlugin(
67+
dontRemoveMe: 'true',
68+
forkCount: '1C', // run this number of tests in parallel for faster feedback. If the number terminates with a 'C', the value will be multiplied by the number of available CPU cores
69+
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
70+
configurations: [
71+
[platform: 'linux', jdk: 25],
72+
])
73+
""", sourceSpecs -> sourceSpecs.path(ArchetypeCommonFile.JENKINSFILE.getPath())));
74+
}
75+
76+
@Test
77+
void shouldRemoveOldJDKFromRecentConfigWithWindows() {
78+
rewriteRun(
79+
spec -> spec.recipe(new UpdateJenkinsfileForJavaVersion(25, List.of(8, 11, 17))),
80+
// language=groovy
81+
groovy("""
82+
/*
83+
See the documentation for more options:
84+
https://github.com/jenkins-infra/pipeline-library/
85+
*/
86+
buildPlugin(
87+
forkCount: '1C', // run this number of tests in parallel for faster feedback. If the number terminates with a 'C', the value will be multiplied by the number of available CPU cores
88+
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
89+
configurations: [
90+
[platform: 'linux', jdk: 21],
91+
[platform: 'windows', jdk: 17],
92+
[platform: 'linux', jdk: 25],
93+
])
94+
""", """
95+
/*
96+
See the documentation for more options:
97+
https://github.com/jenkins-infra/pipeline-library/
98+
*/
99+
buildPlugin(
100+
forkCount: '1C', // run this number of tests in parallel for faster feedback. If the number terminates with a 'C', the value will be multiplied by the number of available CPU cores
101+
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
102+
configurations: [
103+
[platform: 'windows', jdk: 21],
104+
[platform: 'linux', jdk: 25],
105+
])
106+
""", sourceSpecs -> sourceSpecs.path(ArchetypeCommonFile.JENKINSFILE.getPath())));
107+
}
108+
109+
@Test
110+
void shouldRemoveOldJDKFromRecentConfigWithoutWindows() {
111+
rewriteRun(
112+
spec -> spec.recipe(new UpdateJenkinsfileForJavaVersion(25, List.of(8, 11, 17))),
113+
// language=groovy
114+
groovy("""
115+
/*
116+
See the documentation for more options:
117+
https://github.com/jenkins-infra/pipeline-library/
118+
*/
119+
buildPlugin(
120+
forkCount: '1C', // run this number of tests in parallel for faster feedback. If the number terminates with a 'C', the value will be multiplied by the number of available CPU cores
121+
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
122+
configurations: [
123+
[platform: 'linux', jdk: 21],
124+
[platform: 'linux', jdk: 17],
125+
[platform: 'linux', jdk: 25],
126+
])
127+
""", """
128+
/*
129+
See the documentation for more options:
130+
https://github.com/jenkins-infra/pipeline-library/
131+
*/
132+
buildPlugin(
133+
forkCount: '1C', // run this number of tests in parallel for faster feedback. If the number terminates with a 'C', the value will be multiplied by the number of available CPU cores
134+
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
135+
configurations: [
136+
[platform: 'linux', jdk: 21],
137+
[platform: 'linux', jdk: 25],
138+
])
139+
""", sourceSpecs -> sourceSpecs.path(ArchetypeCommonFile.JENKINSFILE.getPath())));
140+
}
141+
49142
@Test
50143
void shouldUpgradeLegacyConfigAndAddJava25LimitJDK() {
51144
rewriteRun(
@@ -155,7 +248,7 @@ void shouldAddJava21ToModernConfigurationsAndRemoveOldJDK() {
155248
useContainerAgent: true,
156249
forkCount: '1C', // Set to `false` if you need to use Docker for containerized tests
157250
configurations: [
158-
[platform: 'linux', jdk: 11],
251+
[platform: 'windows', jdk: 11],
159252
[platform: 'linux', jdk: 21],
160253
])
161254
""", sourceSpecs -> sourceSpecs.path(ArchetypeCommonFile.JENKINSFILE.getPath())));

0 commit comments

Comments
 (0)