[JENKINS-50636] Use @DataBoundSetter for optional pipeline params

[rmstyrczula]

This helps the Snippet Generator provide a smaller example
where many defaults are used, and prevents the Jenkins pipeline
linter from complaining about missing "required" parameters.

JIRA

No issue directly found, though the closest I could find is JENKINS-50636.

Misc

I started working on this due to build failures in my Jenkins pipeline when using a Jenkinsfile from SCM. This does not seem to happen with an inline pipeline job.

plot-build-failure.txt

What has been done

1. Optional constructor parameters have been removed and have been given @DataBoundSetters
2. Generic series has been removed and csv, xml, properties are concatenated during perform.

How to test

You can check if a Jenkinsfile will pass muster by using the built-in linter

The following Jenkinsfile works with my changes, but fail with errors similar to the attached plot-build-failure.txt.

(You can also just create a simple Pipeline job and paste the following)

pipeline {
  agent any
  stages {
    stage('write-data') {
      steps {
        sh '''
          echo '<my_data>' > data.xml
          echo '    <test value1="123.456"/>' >> data.xml
          echo '    <test value2="321.654"/>' >> data.xml
          echo '</my_data>' >> data.xml
        '''
      }
    }
  }
  post {
    always {
      plot csvFileName: 'plot-d519ae37-01b4-446b-a901-a98d0b28efe6.csv',
           group: 'My Data',
           title: 'Useful Title',
           style: 'line',
           yaxis: 'arbitrary',
           xmlSeries: [
             [file: 'data.xml',
              nodeType: 'NODESET',
              xpath: 'my_data/test/@*']
           ]
    }
  }
}

I tested this with Jenkins (LTS 2.107.3) inside Docker:

docker run -d --name jenkins -p 127.0.0.1:8080:8080 -p 50000:50000 -v jenkins_home:/var/jenkins_home jenkins/jenkins:lts

I installed the suggested plugins, and then uploaded the built hpi to test.

Checklist

• Git commits follow best practices
• Build passes in Jenkins
• Appropriate tests or explanation to why this change has no tests
• JIRA issue is well described (problem explanation, steps to reproduce, screenshots)
• For dependency updates: links to external changelogs and, if possible, full diffs

[vgaidarji]

Probably this field should be moved to the top where other Required fields are located.

[rmstyrczula]

This one is a little odd, since it's not directly specified by the user with the snippet generator. I'm assuming it's generated here through reflection when the step is created.

[vgaidarji]

No need to move, your assumption is correct. Maybe, in this case, explanation comment from where it comes will be better than Generate?.

[vgaidarji]

Verified that this one was actually never called from outside. Should be safe to remove.

[rmstyrczula]

I'm not super happy with the removal of this, as it splits the pipeline and freestyle jobs further from each other. I tried taking parts of the freestyle config.jelly, unfortunately it's not just simple replacement. I'm not familiar enough with freestyle jobs to say why that is.

[vgaidarji]

@rmstyrczula would be nice to have tests for different combinations.
It can be tricky, since it requires base Jenkins version update. But I guess most of the plugin users who use it in pipeline have their Jenkins more or less up to date (need to prove this somehow).

I've prepared a patch for you with a working test and pipeline you provided (it's a .patch file inside, GitHub doesn't support those formats, had to wrap into .zip)
0001-Add-tests.patch.zip
I've update the base Jenkins from 1.625.3 to 2.60.3. Would be nice to find minimum supported version where tests will work. Tests were failing for me with 1.625.3 because of missing class - http://javadoc.jenkins.io/jenkins/model/OptionalJobProperty.html. According to the docs it was added in 1.637 version. But there can be other dependencies as well. Would be nice if you can investigate that. Or at least provide required tests and then I can play myself with dependencies versions.

We need to have tests (at least):

• with mandatory parameters only
• test for XML series
• test for Properties series
• test for CSV series
• ...

[vgaidarji]

@rmstyrczula + would be nice to include JIRA ticket id in a commit message (check this PR for example). This will post updates to JIRA ticket when PR get's merged.

[rmstyrczula]

I'll look into adding your patched tests and if I can add my own.

Regarding the JIRA ticket, do you want me to reference JENKINS-50636?

edit: There is a deprecated jenkins image with many older tags. I'll check that out to test older versions

[rmstyrczula]

Through testing, it seems 2.0 is the minimum required version now. Testing on even the latest 1.x version, 1.656, has failures with the new PlotBuilder tests.

I added your patch, added some basic tests on top of it, and modified the original commit message to include the JIRA ticket ID.

Let me know if I've missed anything!

[vgaidarji]

@rmstyrczula good job! That's exactly what I've expected from those tests.
Regarding Jira ticket, could you please format them in a following way

• JENKINS-50636 Use @DataBoundSetter to support optional pipeline params
• JENKINS-50636 Configure environment for PlotBuilder pipeline tests

Divide 10d5c29 commit into 2 (if tricky, modifying commit message should be enough):

• JENKINS-50636 Reduce required Jenkins version (with detailed explanation why we changed the Jenkins version)
• JENKINS-50636 Cover PlotBuilder with tests

---

I've tested on freestyle/matrix/pipeline with parameters/pipeline with minimal required parameters jobs and everything works as expected 👍

---

Once you fix the commit messages, I'll merge and release this fix. Thank you so much!

[rmstyrczula]

I split out the pom update to its own commit, and put your patched tests plus the others in the last commit. Changed the wording as you requested.

[vgaidarji]

@rmstyrczula thank you so much for this fix 🎉.
2.1.0 version released (link to changelog). New version will be available soon via update center.