Skip to content

Commit 757de17

Browse files
authored
Improve tests and cleanup code (#80)
* Improve tests and cleanup code * Fix upper bound deps Co-authored-by: res0nance <[email protected]>
1 parent e9c8abe commit 757de17

10 files changed

Lines changed: 154 additions & 39 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ Example Output in Jenkins
8686
## Scripted Pipeline Example
8787

8888
```
89-
properties(
89+
properties([
9090
parameters([
9191
string(name: 'PLANET', defaultValue: 'Earth', description: 'Which planet are we on?'),
9292
string(name: 'GREETING', defaultValue: 'Hello', description: 'How shall we greet?')
@@ -95,5 +95,5 @@ properties(
9595
parameterizedCron('*/2 * * * * %GREETING=Hola;PLANET=Pluto'),
9696
parameterizedCron('*/3 * * * * %PLANET=Mars')
9797
])
98-
)
98+
])
9999
```

pom.xml

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,39 @@
6161
</pluginRepositories>
6262

6363
<dependencies>
64+
<dependency>
65+
<groupId>org.jenkins-ci.plugins.workflow</groupId>
66+
<artifactId>workflow-job</artifactId>
67+
</dependency>
68+
<dependency>
69+
<groupId>org.jenkins-ci.plugins</groupId>
70+
<artifactId>structs</artifactId>
71+
</dependency>
72+
<dependency>
73+
<groupId>org.jenkins-ci.plugins.workflow</groupId>
74+
<artifactId>workflow-cps</artifactId>
75+
<scope>test</scope>
76+
</dependency>
77+
<dependency>
78+
<groupId>org.jenkins-ci.plugins.workflow</groupId>
79+
<artifactId>workflow-multibranch</artifactId>
80+
<scope>test</scope>
81+
</dependency>
82+
<dependency>
83+
<groupId>org.jenkinsci.plugins</groupId>
84+
<artifactId>pipeline-model-definition</artifactId>
85+
<scope>test</scope>
86+
</dependency>
6487
<dependency>
6588
<groupId>org.mockito</groupId>
6689
<artifactId>mockito-core</artifactId>
6790
<scope>test</scope>
6891
</dependency>
92+
<!-- Upper bound dependencies -->
6993
<dependency>
70-
<groupId>org.jenkins-ci.plugins.workflow</groupId>
71-
<artifactId>workflow-job</artifactId>
94+
<groupId>org.jenkins-ci.plugins</groupId>
95+
<artifactId>jackson2-api</artifactId>
96+
<scope>test</scope>
7297
</dependency>
7398
</dependencies>
7499

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/Cron.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,14 @@ public long getInitialDelay() {
3232

3333
@Override
3434
protected void doRun() throws Exception {
35-
Jenkins instance = Jenkins.getInstance();
35+
Jenkins instance = Jenkins.get();
3636

37-
if (instance == null) { // soon to be removed, on 2.4 IIRC
38-
throw new IllegalStateException("Jenkins instance cannot really be null at this point");
39-
}
40-
41-
for (AbstractProject<?, ?> project : instance.getAllItems(AbstractProject.class)) {
37+
for (AbstractProject<?, ?> project : instance.allItems(AbstractProject.class)) {
4238
checkTriggers(project.getName(), project.getTriggers().values(), new GregorianCalendar());
4339
}
4440

45-
if (instance.getPlugin("workflow-job") != null) {
46-
for (WorkflowJob workflowJob : instance.getAllItems(WorkflowJob.class)) {
47-
checkTriggers(workflowJob.getName(), workflowJob.getTriggers().values(), new GregorianCalendar());
48-
}
41+
for (WorkflowJob workflowJob : instance.allItems(WorkflowJob.class)) {
42+
checkTriggers(workflowJob.getName(), workflowJob.getTriggers().values(), new GregorianCalendar());
4943
}
5044
}
5145

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/DescriptorImpl.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import hudson.triggers.TriggerDescriptor;
1010
import hudson.util.FormValidation;
1111
import org.jenkinsci.Symbol;
12-
import jenkins.model.Jenkins;
1312
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
1413
import org.kohsuke.stapler.AncestorInPath;
1514
import org.kohsuke.stapler.QueryParameter;
@@ -31,13 +30,9 @@ public DescriptorImpl() {
3130
public boolean isApplicable(Item item) {
3231
boolean result = false;
3332

34-
Jenkins jenkins = Jenkins.getInstance();
35-
3633
if (item instanceof AbstractProject) {
3734
result = ((AbstractProject) item).isParameterized();
38-
} else if (jenkins != null &&
39-
jenkins.getPlugin("workflow-job") != null &&
40-
item instanceof WorkflowJob) {
35+
} else if (item instanceof WorkflowJob) {
4136
result = ((WorkflowJob) item).isParameterized();
4237
}
4338
return result;

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterParser.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
import hudson.model.ParametersDefinitionProperty;
44

55
import java.util.ArrayList;
6+
import java.util.Collections;
67
import java.util.List;
78
import java.util.Map;
89

910
import org.apache.commons.lang.StringUtils;
1011
import org.jenkinsci.plugins.parameterizedscheduler.Messages;
1112

1213
import com.google.common.base.Splitter;
13-
import com.google.common.collect.Maps;
1414

1515
public class ParameterParser {
1616
/**
@@ -27,7 +27,7 @@ public class ParameterParser {
2727
*/
2828
public Map<String, String> parse(String nameValuePairFormattedString) {
2929
if (StringUtils.isBlank(nameValuePairFormattedString)) {
30-
return Maps.<String, String> newHashMap();
30+
return Collections.emptyMap();
3131
}
3232
String clean = nameValuePairFormattedString.trim();
3333
if (nameValuePairFormattedString.endsWith(PAIR_SEPARATOR)) {
@@ -39,16 +39,16 @@ public Map<String, String> parse(String nameValuePairFormattedString) {
3939

4040
public String checkSanity(String cronTabSpec, ParametersDefinitionProperty parametersDefinitionProperty) {
4141
String[] cronTabLines = cronTabSpec.split("\\r?\\n");
42-
for (int i = 0; i < cronTabLines.length; i++) {
43-
String[] split = cronTabLines[i].split(PARAMETER_SEPARATOR);
42+
for (String cronTabLine : cronTabLines) {
43+
String[] split = cronTabLine.split(PARAMETER_SEPARATOR);
4444
if (split.length > 2) {
4545
return Messages.ParameterizedTimerTrigger_MoreThanOnePercent();
4646
}
4747
if (split.length == 2) {
4848
try {
4949
Map<String, String> parsedParameters = parse(split[1]);
5050
List<String> parameterDefinitionNames = parametersDefinitionProperty.getParameterDefinitionNames();
51-
List<String> parsedKeySet = new ArrayList<String>(parsedParameters.keySet());
51+
List<String> parsedKeySet = new ArrayList<>(parsedParameters.keySet());
5252
parsedKeySet.removeAll(parameterDefinitionNames);
5353
if (!parsedKeySet.isEmpty()) {
5454
return Messages.ParameterizedTimerTrigger_UndefinedParameter(parsedKeySet, parameterDefinitionNames);

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTab.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22

33
import java.util.Calendar;
44
import java.util.Collections;
5+
import java.util.HashMap;
56
import java.util.Map;
67

7-
import com.google.common.collect.Maps;
8-
98
import antlr.ANTLRException;
109
import hudson.scheduler.CronTab;
1110
import hudson.scheduler.CronTabList;
@@ -38,7 +37,7 @@ public ParameterizedCronTab(CronTab cronTab, Map<String, String> parameters) {
3837
public static ParameterizedCronTab create(String line, int lineNumber, Hash hash) throws ANTLRException {
3938
String[] lineParts = line.split("%");
4039
CronTab cronTab = new CronTab(lineParts[0].trim(), lineNumber, hash);
41-
Map<String, String> parameters = Maps.newHashMap();
40+
Map<String, String> parameters = new HashMap<>();
4241
if (lineParts.length == 2) {
4342
parameters = new ParameterParser().parse(lineParts[1]);
4443
}

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabList.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public static ParameterizedCronTabList create(String cronTabSpecification) throw
2929
}
3030

3131
public static ParameterizedCronTabList create(String cronTabSpecification, Hash hash) throws ANTLRException {
32-
List<ParameterizedCronTab> result = new ArrayList<ParameterizedCronTab>();
32+
List<ParameterizedCronTab> result = new ArrayList<>();
3333
int lineNumber = 0;
3434
for (String line : cronTabSpecification.split("\\r?\\n")) {
3535
lineNumber++;

src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedTimerTrigger.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
package org.jenkinsci.plugins.parameterizedscheduler;
22

3-
import hudson.model.*;
3+
import hudson.model.AbstractProject;
4+
import hudson.model.Cause;
5+
import hudson.model.CauseAction;
6+
import hudson.model.Job;
7+
import hudson.model.ParameterDefinition;
8+
import hudson.model.ParameterValue;
9+
import hudson.model.ParametersAction;
10+
import hudson.model.ParametersDefinitionProperty;
411
import hudson.scheduler.Hash;
512
import hudson.triggers.Trigger;
613

@@ -11,8 +18,6 @@
1118
import java.util.logging.Level;
1219
import java.util.logging.Logger;
1320

14-
import hudson.triggers.TriggerDescriptor;
15-
import jenkins.model.Jenkins;
1621
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
1722
import org.kohsuke.stapler.DataBoundConstructor;
1823

@@ -46,12 +51,11 @@ public void run() {
4651
* @param parameterValues
4752
* @return the ParameterValues as set from the crontab row or their defaults
4853
*/
49-
@SuppressWarnings("unchecked")
5054
private List<ParameterValue> configurePropertyValues(Map<String, String> parameterValues) {
5155
assert job != null : "job must not be null if this was 'started'";
5256
ParametersDefinitionProperty paramDefProp = (ParametersDefinitionProperty) job
5357
.getProperty(ParametersDefinitionProperty.class);
54-
ArrayList<ParameterValue> defValues = new ArrayList<ParameterValue>();
58+
List<ParameterValue> defValues = new ArrayList<>();
5559

5660
/* Scan for all parameter with an associated default values */
5761
for (ParameterDefinition paramDefinition : paramDefProp.getParameterDefinitions()) {
@@ -71,15 +75,14 @@ private List<ParameterValue> configurePropertyValues(Map<String, String> paramet
7175
public void checkCronTabsAndRun(Calendar calendar) {
7276
LOGGER.fine("checking and maybe running at " + calendar);
7377
ParameterizedCronTab cronTab = cronTabList.check(calendar);
74-
Jenkins jenkins = Jenkins.getInstance();
7578

7679
if (cronTab != null) {
7780
Map<String, String> parameterValues = cronTab.getParameterValues();
7881
ParametersAction parametersAction = new ParametersAction(configurePropertyValues(parameterValues));
7982
assert job != null : "job must not be null, if this was 'started'";
8083
if (job instanceof AbstractProject) {
8184
((AbstractProject) job).scheduleBuild2(0, (Cause)null, causeAction(parameterValues), parametersAction);
82-
} else if (jenkins != null && jenkins.getPlugin("workflow-job") != null && job instanceof WorkflowJob) {
85+
} else if (job instanceof WorkflowJob) {
8386
((WorkflowJob) job).scheduleBuild2(0, causeAction(parameterValues), parametersAction);
8487
}
8588
}

src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabListTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.jenkinsci.plugins.parameterizedscheduler;
22

33
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.assertNotNull;
45
import static org.junit.Assert.assertNull;
56
import static org.junit.Assert.assertSame;
67
import static org.junit.Assert.assertTrue;
@@ -9,8 +10,6 @@
910
import java.util.GregorianCalendar;
1011
import java.util.Map;
1112

12-
import org.jenkinsci.plugins.parameterizedscheduler.ParameterizedCronTab;
13-
import org.jenkinsci.plugins.parameterizedscheduler.ParameterizedCronTabList;
1413
import org.junit.Test;
1514
import org.junit.runner.RunWith;
1615
import org.mockito.Mock;
@@ -31,7 +30,7 @@ public void create() throws Exception {
3130
ParameterizedCronTabList testObject = ParameterizedCronTabList.create("* * * * *%foo=bar");
3231
assertTrue(testObject.checkSanity(), testObject.checkSanity().startsWith("Do you really mean \"every minute\""));
3332
ParameterizedCronTab actualCronTab = testObject.check(new GregorianCalendar());
34-
assertTrue(actualCronTab != null);
33+
assertNotNull(actualCronTab);
3534

3635
Map<String, String> expected = Maps.newHashMap();
3736
expected.put("foo", "bar");
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package org.jenkinsci.plugins.parameterizedscheduler;
2+
3+
import hudson.model.FreeStyleProject;
4+
import hudson.model.Job;
5+
import hudson.model.ParametersAction;
6+
import hudson.model.ParametersDefinitionProperty;
7+
import hudson.model.StringParameterDefinition;
8+
import hudson.triggers.Trigger;
9+
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
10+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
11+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
12+
import org.junit.Rule;
13+
import org.junit.Test;
14+
import org.jvnet.hudson.test.JenkinsRule;
15+
16+
import static org.hamcrest.MatcherAssert.assertThat;
17+
import static org.hamcrest.Matchers.is;
18+
import static org.hamcrest.Matchers.not;
19+
import static org.hamcrest.Matchers.notNullValue;
20+
import static org.hamcrest.Matchers.nullValue;
21+
22+
public class ParameterizedSchedulerTest {
23+
24+
@Rule
25+
public JenkinsRule r = new JenkinsRule();
26+
27+
@Test
28+
public void freestyle() throws Exception {
29+
FreeStyleProject p = r.createFreeStyleProject();
30+
p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("foo", "lol")));
31+
assertThat(p.getLastCompletedBuild(), is(nullValue()));
32+
Trigger<Job> t = new ParameterizedTimerTrigger("* * * * *%foo=bar");
33+
t.start(p, true);
34+
p.addTrigger(t);
35+
new Cron().doRun();
36+
assertThat(p.isInQueue(), is(true));
37+
r.waitUntilNoActivity();
38+
assertThat(p.getLastCompletedBuild(), is(notNullValue()));
39+
assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar"));
40+
}
41+
42+
@Test
43+
public void pipeline() throws Exception {
44+
WorkflowJob p = r.createProject(WorkflowJob.class);
45+
p.setDefinition(new CpsFlowDefinition("", true));
46+
WorkflowRun wfr = p.scheduleBuild2(0).get();
47+
p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("foo", "lol")));
48+
Trigger<Job> t = new ParameterizedTimerTrigger("* * * * *%foo=bar");
49+
t.start(p, true);
50+
p.addTrigger(t);
51+
new Cron().doRun();
52+
r.waitUntilNoActivity();
53+
assertThat(p.getLastCompletedBuild(), is(not(wfr)));
54+
assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar"));
55+
}
56+
57+
@Test
58+
public void scripted() throws Exception {
59+
WorkflowJob p = r.createProject(WorkflowJob.class);
60+
p.setDefinition(new CpsFlowDefinition("properties([\n" +
61+
" parameters([\n" +
62+
" string(name: 'foo', defaultValue: 'lol')\n" +
63+
" ]),\n" +
64+
" pipelineTriggers([\n" +
65+
" parameterizedCron('* * * * *%foo=bar')\n" +
66+
" ])\n" +
67+
"])", true));
68+
WorkflowRun wfr = r.buildAndAssertSuccess(p);
69+
new Cron().doRun();
70+
r.waitUntilNoActivity();
71+
assertThat(p.getLastCompletedBuild(), is(not(wfr)));
72+
assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar"));
73+
}
74+
75+
@Test
76+
public void declarative() throws Exception {
77+
WorkflowJob p = r.createProject(WorkflowJob.class);
78+
p.setDefinition(new CpsFlowDefinition("pipeline {\n" +
79+
" agent any\n" +
80+
" parameters {\n" +
81+
" string(name: 'foo', defaultValue: 'lol')\n" +
82+
" }\n" +
83+
" triggers {\n" +
84+
" parameterizedCron('* * * * *%foo=bar')\n" +
85+
" }\n" +
86+
" stages {\n" +
87+
" stage('Test') {\n" +
88+
" steps {\n" +
89+
" echo 'test'\n" +
90+
" }\n" +
91+
" }\n" +
92+
" }\n" +
93+
"}", true));
94+
WorkflowRun wfr = r.buildAndAssertSuccess(p);
95+
new Cron().doRun();
96+
r.waitUntilNoActivity();
97+
assertThat(p.getLastCompletedBuild(), is(not(wfr)));
98+
assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar"));
99+
}
100+
}

0 commit comments

Comments
 (0)