Skip to content

[JENKINS-52943] Add descriptors for shell steps#92

Merged
dwnusbaum merged 4 commits intojenkinsci:masterfrom
halkeye:JENKINS-52943-task-descriptors
Dec 14, 2018
Merged

[JENKINS-52943] Add descriptors for shell steps#92
dwnusbaum merged 4 commits intojenkinsci:masterfrom
halkeye:JENKINS-52943-task-descriptors

Conversation

@halkeye
Copy link
Copy Markdown
Member

@halkeye halkeye commented Dec 14, 2018 •
edited by dwnusbaum
Loading

@halkeye
Copy link
Copy Markdown
Member Author

halkeye commented Dec 14, 2018

WIP because I'm trying to learn how to write a test for this.

I did test it manually though.

image

Generated by the script:

node('master'){
 sh 'echo "hello"'
 sh returnStdout: true, script: 'echo "hello"'
 sh returnStatus: true, script: 'echo "hello"'
}

jglick
jglick reviewed Dec 14, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will take a look at the test failure

@jglick
Copy link
Copy Markdown
Member

jglick commented Dec 14, 2018

Your problem is that this plugin was still testing against a very old version of workflow-cps. The API here was implemented in 2.31; older versions will not save step arguments. Minimally:

diff --git a/pom.xml b/pom.xml
index 1987911..4b53c92 100644
--- a/pom.xml
+++ b/pom.xml
@@ -93,7 +93,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-cps</artifactId>
-            <version>2.22</version>
+            <version>2.31</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -137,7 +137,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-scm-step</artifactId>
-            <version>2.3</version>
+            <version>2.4</version>
             <scope>test</scope>
         </dependency>
         <dependency>

(You could go higher, all the way up to current 2.61, though that might pull in some other newer dependencies too which would necessitate larger POM changes.)

@dwnusbaum dwnusbaum mentioned this pull request Dec 14, 2018
Merged
@halkeye
Merge pull request #1 from dwnusbaum/JENKINS-52943
0fa0cc6 
Update dependencies and make minor adjustments for tests to pass
@halkeye
Copy link
Copy Markdown
Member Author

halkeye commented Dec 14, 2018

thanks @dwnusbaum

@halkeye halkeye changed the title (WIP) [JENKINS-52943] Add descriptors for shell steps [JENKINS-52943] Add descriptors for shell steps Dec 14, 2018
dwnusbaum
dwnusbaum approved these changes Dec 14, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll merge it once the CI build passes.

@dwnusbaum dwnusbaum merged commit f02e95a into jenkinsci:master Dec 14, 2018
@dwnusbaum dwnusbaum mentioned this pull request Jan 4, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jglick jglick jglick left review comments

@dwnusbaum dwnusbaum dwnusbaum approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@halkeye @jglick @dwnusbaum