From 757de17219ac67a127794c3d36660dc1471cec6d Mon Sep 17 00:00:00 2001 From: Raihaan Shouhell Date: Tue, 22 Dec 2020 11:11:34 +0800 Subject: [PATCH] Improve tests and cleanup code (#80) * Improve tests and cleanup code * Fix upper bound deps Co-authored-by: res0nance --- README.md | 4 +- pom.xml | 29 ++++- .../plugins/parameterizedscheduler/Cron.java | 14 +-- .../DescriptorImpl.java | 7 +- .../ParameterParser.java | 10 +- .../ParameterizedCronTab.java | 5 +- .../ParameterizedCronTabList.java | 2 +- .../ParameterizedTimerTrigger.java | 17 +-- .../ParameterizedCronTabListTest.java | 5 +- .../ParameterizedSchedulerTest.java | 100 ++++++++++++++++++ 10 files changed, 154 insertions(+), 39 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedSchedulerTest.java diff --git a/README.md b/README.md index 59f14df..a7fbb23 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ Example Output in Jenkins ## Scripted Pipeline Example ``` -properties( +properties([ parameters([ string(name: 'PLANET', defaultValue: 'Earth', description: 'Which planet are we on?'), string(name: 'GREETING', defaultValue: 'Hello', description: 'How shall we greet?') @@ -95,5 +95,5 @@ properties( parameterizedCron('*/2 * * * * %GREETING=Hola;PLANET=Pluto'), parameterizedCron('*/3 * * * * %PLANET=Mars') ]) -) +]) ``` diff --git a/pom.xml b/pom.xml index 4337fbc..1521c70 100644 --- a/pom.xml +++ b/pom.xml @@ -61,14 +61,39 @@ + + org.jenkins-ci.plugins.workflow + workflow-job + + + org.jenkins-ci.plugins + structs + + + org.jenkins-ci.plugins.workflow + workflow-cps + test + + + org.jenkins-ci.plugins.workflow + workflow-multibranch + test + + + org.jenkinsci.plugins + pipeline-model-definition + test + org.mockito mockito-core test + - org.jenkins-ci.plugins.workflow - workflow-job + org.jenkins-ci.plugins + jackson2-api + test diff --git a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/Cron.java b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/Cron.java index b9854e0..8d59419 100644 --- a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/Cron.java +++ b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/Cron.java @@ -32,20 +32,14 @@ public long getInitialDelay() { @Override protected void doRun() throws Exception { - Jenkins instance = Jenkins.getInstance(); + Jenkins instance = Jenkins.get(); - if (instance == null) { // soon to be removed, on 2.4 IIRC - throw new IllegalStateException("Jenkins instance cannot really be null at this point"); - } - - for (AbstractProject project : instance.getAllItems(AbstractProject.class)) { + for (AbstractProject project : instance.allItems(AbstractProject.class)) { checkTriggers(project.getName(), project.getTriggers().values(), new GregorianCalendar()); } - if (instance.getPlugin("workflow-job") != null) { - for (WorkflowJob workflowJob : instance.getAllItems(WorkflowJob.class)) { - checkTriggers(workflowJob.getName(), workflowJob.getTriggers().values(), new GregorianCalendar()); - } + for (WorkflowJob workflowJob : instance.allItems(WorkflowJob.class)) { + checkTriggers(workflowJob.getName(), workflowJob.getTriggers().values(), new GregorianCalendar()); } } diff --git a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/DescriptorImpl.java b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/DescriptorImpl.java index 1f5f883..4f54b77 100644 --- a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/DescriptorImpl.java +++ b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/DescriptorImpl.java @@ -9,7 +9,6 @@ import hudson.triggers.TriggerDescriptor; import hudson.util.FormValidation; import org.jenkinsci.Symbol; -import jenkins.model.Jenkins; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.QueryParameter; @@ -31,13 +30,9 @@ public DescriptorImpl() { public boolean isApplicable(Item item) { boolean result = false; - Jenkins jenkins = Jenkins.getInstance(); - if (item instanceof AbstractProject) { result = ((AbstractProject) item).isParameterized(); - } else if (jenkins != null && - jenkins.getPlugin("workflow-job") != null && - item instanceof WorkflowJob) { + } else if (item instanceof WorkflowJob) { result = ((WorkflowJob) item).isParameterized(); } return result; diff --git a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterParser.java b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterParser.java index a867e43..b0e7f8a 100644 --- a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterParser.java +++ b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterParser.java @@ -3,6 +3,7 @@ import hudson.model.ParametersDefinitionProperty; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -10,7 +11,6 @@ import org.jenkinsci.plugins.parameterizedscheduler.Messages; import com.google.common.base.Splitter; -import com.google.common.collect.Maps; public class ParameterParser { /** @@ -27,7 +27,7 @@ public class ParameterParser { */ public Map parse(String nameValuePairFormattedString) { if (StringUtils.isBlank(nameValuePairFormattedString)) { - return Maps. newHashMap(); + return Collections.emptyMap(); } String clean = nameValuePairFormattedString.trim(); if (nameValuePairFormattedString.endsWith(PAIR_SEPARATOR)) { @@ -39,8 +39,8 @@ public Map parse(String nameValuePairFormattedString) { public String checkSanity(String cronTabSpec, ParametersDefinitionProperty parametersDefinitionProperty) { String[] cronTabLines = cronTabSpec.split("\\r?\\n"); - for (int i = 0; i < cronTabLines.length; i++) { - String[] split = cronTabLines[i].split(PARAMETER_SEPARATOR); + for (String cronTabLine : cronTabLines) { + String[] split = cronTabLine.split(PARAMETER_SEPARATOR); if (split.length > 2) { return Messages.ParameterizedTimerTrigger_MoreThanOnePercent(); } @@ -48,7 +48,7 @@ public String checkSanity(String cronTabSpec, ParametersDefinitionProperty param try { Map parsedParameters = parse(split[1]); List parameterDefinitionNames = parametersDefinitionProperty.getParameterDefinitionNames(); - List parsedKeySet = new ArrayList(parsedParameters.keySet()); + List parsedKeySet = new ArrayList<>(parsedParameters.keySet()); parsedKeySet.removeAll(parameterDefinitionNames); if (!parsedKeySet.isEmpty()) { return Messages.ParameterizedTimerTrigger_UndefinedParameter(parsedKeySet, parameterDefinitionNames); diff --git a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTab.java b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTab.java index a310360..ff179e0 100644 --- a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTab.java +++ b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTab.java @@ -2,10 +2,9 @@ import java.util.Calendar; import java.util.Collections; +import java.util.HashMap; import java.util.Map; -import com.google.common.collect.Maps; - import antlr.ANTLRException; import hudson.scheduler.CronTab; import hudson.scheduler.CronTabList; @@ -38,7 +37,7 @@ public ParameterizedCronTab(CronTab cronTab, Map parameters) { public static ParameterizedCronTab create(String line, int lineNumber, Hash hash) throws ANTLRException { String[] lineParts = line.split("%"); CronTab cronTab = new CronTab(lineParts[0].trim(), lineNumber, hash); - Map parameters = Maps.newHashMap(); + Map parameters = new HashMap<>(); if (lineParts.length == 2) { parameters = new ParameterParser().parse(lineParts[1]); } diff --git a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabList.java b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabList.java index 5c5b7bd..2626161 100644 --- a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabList.java +++ b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabList.java @@ -29,7 +29,7 @@ public static ParameterizedCronTabList create(String cronTabSpecification) throw } public static ParameterizedCronTabList create(String cronTabSpecification, Hash hash) throws ANTLRException { - List result = new ArrayList(); + List result = new ArrayList<>(); int lineNumber = 0; for (String line : cronTabSpecification.split("\\r?\\n")) { lineNumber++; diff --git a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedTimerTrigger.java b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedTimerTrigger.java index f3334f8..d5d9000 100644 --- a/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedTimerTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedTimerTrigger.java @@ -1,6 +1,13 @@ package org.jenkinsci.plugins.parameterizedscheduler; -import hudson.model.*; +import hudson.model.AbstractProject; +import hudson.model.Cause; +import hudson.model.CauseAction; +import hudson.model.Job; +import hudson.model.ParameterDefinition; +import hudson.model.ParameterValue; +import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; import hudson.scheduler.Hash; import hudson.triggers.Trigger; @@ -11,8 +18,6 @@ import java.util.logging.Level; import java.util.logging.Logger; -import hudson.triggers.TriggerDescriptor; -import jenkins.model.Jenkins; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.kohsuke.stapler.DataBoundConstructor; @@ -46,12 +51,11 @@ public void run() { * @param parameterValues * @return the ParameterValues as set from the crontab row or their defaults */ - @SuppressWarnings("unchecked") private List configurePropertyValues(Map parameterValues) { assert job != null : "job must not be null if this was 'started'"; ParametersDefinitionProperty paramDefProp = (ParametersDefinitionProperty) job .getProperty(ParametersDefinitionProperty.class); - ArrayList defValues = new ArrayList(); + List defValues = new ArrayList<>(); /* Scan for all parameter with an associated default values */ for (ParameterDefinition paramDefinition : paramDefProp.getParameterDefinitions()) { @@ -71,7 +75,6 @@ private List configurePropertyValues(Map paramet public void checkCronTabsAndRun(Calendar calendar) { LOGGER.fine("checking and maybe running at " + calendar); ParameterizedCronTab cronTab = cronTabList.check(calendar); - Jenkins jenkins = Jenkins.getInstance(); if (cronTab != null) { Map parameterValues = cronTab.getParameterValues(); @@ -79,7 +82,7 @@ public void checkCronTabsAndRun(Calendar calendar) { assert job != null : "job must not be null, if this was 'started'"; if (job instanceof AbstractProject) { ((AbstractProject) job).scheduleBuild2(0, (Cause)null, causeAction(parameterValues), parametersAction); - } else if (jenkins != null && jenkins.getPlugin("workflow-job") != null && job instanceof WorkflowJob) { + } else if (job instanceof WorkflowJob) { ((WorkflowJob) job).scheduleBuild2(0, causeAction(parameterValues), parametersAction); } } diff --git a/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabListTest.java b/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabListTest.java index f3421f4..e6feac6 100644 --- a/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabListTest.java +++ b/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedCronTabListTest.java @@ -1,6 +1,7 @@ package org.jenkinsci.plugins.parameterizedscheduler; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -9,8 +10,6 @@ import java.util.GregorianCalendar; import java.util.Map; -import org.jenkinsci.plugins.parameterizedscheduler.ParameterizedCronTab; -import org.jenkinsci.plugins.parameterizedscheduler.ParameterizedCronTabList; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -31,7 +30,7 @@ public void create() throws Exception { ParameterizedCronTabList testObject = ParameterizedCronTabList.create("* * * * *%foo=bar"); assertTrue(testObject.checkSanity(), testObject.checkSanity().startsWith("Do you really mean \"every minute\"")); ParameterizedCronTab actualCronTab = testObject.check(new GregorianCalendar()); - assertTrue(actualCronTab != null); + assertNotNull(actualCronTab); Map expected = Maps.newHashMap(); expected.put("foo", "bar"); diff --git a/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedSchedulerTest.java b/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedSchedulerTest.java new file mode 100644 index 0000000..f74673f --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/parameterizedscheduler/ParameterizedSchedulerTest.java @@ -0,0 +1,100 @@ +package org.jenkinsci.plugins.parameterizedscheduler; + +import hudson.model.FreeStyleProject; +import hudson.model.Job; +import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.StringParameterDefinition; +import hudson.triggers.Trigger; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; + +public class ParameterizedSchedulerTest { + + @Rule + public JenkinsRule r = new JenkinsRule(); + + @Test + public void freestyle() throws Exception { + FreeStyleProject p = r.createFreeStyleProject(); + p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("foo", "lol"))); + assertThat(p.getLastCompletedBuild(), is(nullValue())); + Trigger t = new ParameterizedTimerTrigger("* * * * *%foo=bar"); + t.start(p, true); + p.addTrigger(t); + new Cron().doRun(); + assertThat(p.isInQueue(), is(true)); + r.waitUntilNoActivity(); + assertThat(p.getLastCompletedBuild(), is(notNullValue())); + assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar")); + } + + @Test + public void pipeline() throws Exception { + WorkflowJob p = r.createProject(WorkflowJob.class); + p.setDefinition(new CpsFlowDefinition("", true)); + WorkflowRun wfr = p.scheduleBuild2(0).get(); + p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("foo", "lol"))); + Trigger t = new ParameterizedTimerTrigger("* * * * *%foo=bar"); + t.start(p, true); + p.addTrigger(t); + new Cron().doRun(); + r.waitUntilNoActivity(); + assertThat(p.getLastCompletedBuild(), is(not(wfr))); + assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar")); + } + + @Test + public void scripted() throws Exception { + WorkflowJob p = r.createProject(WorkflowJob.class); + p.setDefinition(new CpsFlowDefinition("properties([\n" + + " parameters([\n" + + " string(name: 'foo', defaultValue: 'lol')\n" + + " ]),\n" + + " pipelineTriggers([\n" + + " parameterizedCron('* * * * *%foo=bar')\n" + + " ])\n" + + "])", true)); + WorkflowRun wfr = r.buildAndAssertSuccess(p); + new Cron().doRun(); + r.waitUntilNoActivity(); + assertThat(p.getLastCompletedBuild(), is(not(wfr))); + assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar")); + } + + @Test + public void declarative() throws Exception { + WorkflowJob p = r.createProject(WorkflowJob.class); + p.setDefinition(new CpsFlowDefinition("pipeline {\n" + + " agent any\n" + + " parameters {\n" + + " string(name: 'foo', defaultValue: 'lol')\n" + + " }\n" + + " triggers {\n" + + " parameterizedCron('* * * * *%foo=bar')\n" + + " }\n" + + " stages {\n" + + " stage('Test') {\n" + + " steps {\n" + + " echo 'test'\n" + + " }\n" + + " }\n" + + " }\n" + + "}", true)); + WorkflowRun wfr = r.buildAndAssertSuccess(p); + new Cron().doRun(); + r.waitUntilNoActivity(); + assertThat(p.getLastCompletedBuild(), is(not(wfr))); + assertThat((String) p.getLastCompletedBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), is("bar")); + } +}