Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add junit test for stash file parameter #7

Merged
merged 34 commits into from
Jan 30, 2021
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9476558
upgrade release drafter and add dependabot for ghactions
olamy Jan 20, 2021
9bda7b9
add junit for stash file parameter
olamy Jan 21, 2021
9dc68fa
add more details on manual tests done
olamy Jan 21, 2021
542a861
add more docs and inline help
olamy Jan 22, 2021
83d4e02
mar inline help as done
olamy Jan 22, 2021
a3e2f56
add more tests including auto creation of build parameters with pipeline
olamy Jan 22, 2021
779d206
add ignored test for a coming feature
olamy Jan 22, 2021
1ba3a0b
use org.jenkins-ci.plugins:jackson2-api
olamy Jan 22, 2021
7f25496
Update README.md
olamy Jan 23, 2021
237e9ee
Update README.md
olamy Jan 23, 2021
ed7877d
Update README.md
olamy Jan 23, 2021
ab26f20
Update src/main/java/io/jenkins/plugins/file_parameters/Base64FilePar…
olamy Jan 23, 2021
36d2f2f
Update README.md
olamy Jan 23, 2021
828f0ba
Update src/test/java/io/jenkins/plugins/file_parameters/FileParameter…
olamy Jan 23, 2021
1b62df5
fix unit test
olamy Jan 23, 2021
9558459
Update src/test/java/io/jenkins/plugins/file_parameters/FileParameter…
olamy Jan 24, 2021
ece717d
more fixes after review
olamy Jan 24, 2021
e5ae911
Update README.md
olamy Jan 24, 2021
ceabdb8
Update src/main/java/io/jenkins/plugins/file_parameters/StashedFilePa…
olamy Jan 24, 2021
2c2d4f5
Apply suggestions from code review
olamy Jan 24, 2021
e677d30
fix test as simplified parameter syntax do not work
olamy Jan 24, 2021
292f870
add allowNoFile option to not fail if no parameter
olamy Jan 25, 2021
b960b79
option to tolerate undefined parameter has been done
olamy Jan 25, 2021
b2e08aa
all done for withFileParameter
olamy Jan 25, 2021
5c29809
add a comment on why this dependency with exclusions
olamy Jan 27, 2021
4920a15
Update src/test/java/io/jenkins/plugins/file_parameters/FileParameter…
olamy Jan 27, 2021
06333dc
Update src/test/java/io/jenkins/plugins/file_parameters/FileParameter…
olamy Jan 27, 2021
0d3acb6
Update src/main/resources/io/jenkins/plugins/file_parameters/Base64Fi…
olamy Jan 27, 2021
89ee670
Update src/main/resources/io/jenkins/plugins/file_parameters/StashedF…
olamy Jan 27, 2021
4535fd3
Update src/test/java/io/jenkins/plugins/file_parameters/FileParameter…
olamy Jan 27, 2021
be09527
remove extra empty lines
olamy Jan 27, 2021
71928ff
add allowNoFile help and move help
olamy Jan 27, 2021
12b668f
Update README.md
olamy Jan 27, 2021
405415f
stashed is for big files
olamy Jan 27, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ updates:
directory: "/"
schedule:
interval: "weekly"
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "weekly"
5 changes: 2 additions & 3 deletions .github/workflows/release-drafter.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
# Automates creation of Release Drafts using Release Drafter
# More Info: https://github.com/jenkinsci/.github/blob/master/.github/release-drafter.adoc

name: Release Drafter
on:
push:
branches:
- master

jobs:
update_release_draft:
runs-on: ubuntu-latest
steps:
# Drafts your next Release notes as Pull Requests are merged into "master"
- uses: release-drafter/release-drafter@v5
- uses: release-drafter/[email protected]
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
39 changes: 32 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,45 @@ Offers alternative types of file parameter that are compatible with Pipeline and

See [JENKINS-27413](https://issues.jenkins-ci.org/browse/JENKINS-27413) and [JENKINS-29289](https://issues.jenkins-ci.org/browse/JENKINS-29289) for background.

## Usage in declarative pipeline

You can now declare file parameters as is in declarative pipeline:

```groovy
pipeline {
agent any
parameters {
base64File 'FILE'
stashedFile 'FILE-STASH'
olamy marked this conversation as resolved.
Show resolved Hide resolved
}
stages {
stage('Example') {
steps {
withFileParameter('FILE') {
Copy link
Member

Choose a reason for hiding this comment

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

IIRC Declarative supports using block-scoped steps in an option block. Does it work here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep it works

Copy link
Member

Choose a reason for hiding this comment

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

But you prefer to demonstrate using a block-scoped step inside steps?

Copy link
Member Author

Choose a reason for hiding this comment

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

uhm I missed that one but I'm not sure what sort of syntax you mean?

Copy link
Member

Choose a reason for hiding this comment

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

Something like

pipeline {
  agent any
  parameters {
    base64File(name: 'FILE')
  }
  options {
    withFileParameter('FILE')
  }
  stages {
    stage('x') {
      steps {
        sh 'cat $FILE'
      }
    }
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

Bug in Declarative it seems: https://issues.jenkins.io/browse/JENKINS-64761

sh 'cat $FILE'
}
unstash 'FILE-STASH'
echo(/loaded '${readFile('FILE-STASH')}'/)
}
}
}
}
```

## Implementation status

- [X] Base64 file parameter (simple and suitable for small files)
- [X] implementation
- [ ] inline help text
- [X] inline help text
- [X] `withFileParameter` compatibility
- [X] manual test
- [X] automated test
- [ ] stashed file parameter (suitable for larger files used in Pipeline)
- [X] implementation
- [ ] inline help text
- [ ] `withFileParameter` compatibility
- [X] inline help text
- [X] `withFileParameter` compatibility (manual test)
Copy link
Member

Choose a reason for hiding this comment

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

I do not think it could possibly work currently:

throw new IOException(); // TODO StashManager.unstash to a temp dir

Copy link
Member Author

Choose a reason for hiding this comment

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

well not sure using such usage is very useful :) but true we can be error prone...

pipeline {
    agent any
    parameters {
        stashedFile(name: 'FILE')
    }
    stages {
        stage('Example') {
            steps {
                withFileParameter(name:'FILE') {
                    unstash "FILE"
                    sh 'cat $FILE'
                }
            }
        }
    }
}

Copy link
Member

Choose a reason for hiding this comment

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

If you are using withFileParameter then you are not supposed to need unstash (and vice-versa).

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- [X] `withFileParameter` compatibility (manual test)
- [ ] `withFileParameter` compatibility

- [X] manual test
- [ ] automated test
- [X] automated test
- [ ] sanity check against `artifact-manager-s3`
- [ ] archived file parameter (compatible with freestyle, and suitable if you want to ensure parameters are kept after the build ends)
- [ ] implementation
Expand All @@ -45,12 +70,12 @@ See [JENKINS-27413](https://issues.jenkins-ci.org/browse/JENKINS-27413) and [JEN
- [X] implementation
- [ ] manual test
- [ ] automated test
- [ ] `withFileParameter` wrapper step
- [X] `withFileParameter` wrapper step
- [X] implementation
- [X] inline help text
- [ ] manual test
- [X] manual test
- [X] automated test
- [ ] option to tolerate undefined parameter
- [X] option to tolerate undefined parameter
- [ ] `input` step submission
- [ ] design
- [ ] manual test
Expand Down
10 changes: 10 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-api</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>jackson2-api</artifactId>
<scope>test</scope>
</dependency>
olamy marked this conversation as resolved.
Show resolved Hide resolved
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-job</artifactId>
Expand All @@ -62,6 +67,11 @@
<artifactId>workflow-basic-steps</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkinsci.plugins</groupId>
<artifactId>pipeline-model-definition</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<licenses>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public final class Base64FileParameterDefinition extends AbstractFileParameterDe
@Override public String getDisplayName() {
return "Base64 File Parameter";
}

}



olamy marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import jenkins.tasks.SimpleBuildWrapper;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;

/**
* Saves a file parameter to a temporary local file.
Expand All @@ -49,22 +50,37 @@ public final class FileParameterWrapper extends SimpleBuildWrapper {

public final String name;

private boolean allowNoFile;

@DataBoundConstructor public FileParameterWrapper(String name) {
this.name = name;
}

public boolean isAllowNoFile() {
return allowNoFile;
}

@DataBoundSetter
public void setAllowNoFile(boolean allowNoFile) {
this.allowNoFile = allowNoFile;
}

@Override public void setUp(Context context, Run<?, ?> build, FilePath workspace, Launcher launcher, TaskListener listener, EnvVars initialEnvironment) throws IOException, InterruptedException {
ParametersAction pa = build.getAction(ParametersAction.class);
if (pa == null) {
throw new AbortException("No parameters");
}
ParameterValue pv = pa.getParameter(name);
if (pv == null) {
if (pv == null && !allowNoFile) {
throw new AbortException("No parameter named " + name);
}
if (!(pv instanceof AbstractFileParameterValue)) {
if (!(pv instanceof AbstractFileParameterValue) && !allowNoFile) {
throw new AbortException("Unsupported parameter type");
}
if (pv == null && allowNoFile) {
listener.getLogger().println("Skip file parameter as there is no parameter with name: '" + name + "'");
return;
}
FilePath tempDir = WorkspaceList.tempDir(workspace);
if (tempDir == null) {
throw new AbortException("Missing workspace or could not make temp dir");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public final class StashedFileParameterDefinition extends AbstractFileParameterD

// TODO equals/hashCode

@Symbol("stashed64File")
@Symbol("stashedFile")
@Extension public static final class DescriptorImpl extends ParameterDefinition.ParameterDescriptor {

@Override public String getDisplayName() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<div>
<p>File parameter compatible with Pipeline, how to use it in a declarative pipeline:</p>
olamy marked this conversation as resolved.
Show resolved Hide resolved
<pre>
pipeline {
agent any
parameters {
base64File(name: 'THEFILE')
}
stages {
stage('Example') {
steps {
withFileParameter('THEFILE') {
olamy marked this conversation as resolved.
Show resolved Hide resolved
echo(/loaded '${readFile(FILE)}' from $FILE/)
olamy marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
}
</pre>
</div>
<div>
Per default, there will be an error if there is no parameter for the build but you can ignore this error using the
parameter attribute <code>allowNoFile</code>. In this case your pipeline must take into account the file doesn't exists
<pre>
pipeline {
agent any
parameters {
base64File(name: 'THEFILE')
}
stages {
stage('Example') {
steps {
withFileParameter(name:'THEFILE', allowNoFile: true) {
echo(/loaded '${readFile(FILE)}' from $FILE/)
}
}
}
}
}
</pre>
</div>
olamy marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<div>
<p>File parameter compatible with Pipeline but this one stash the file, how to use it in a declarative pipeline:</p>
olamy marked this conversation as resolved.
Show resolved Hide resolved
<pre>
pipeline {
agent any
parameters {
stashedFile(name: 'FILE-STASH')
}
stages {
stage('Example') {
steps {
unstash "FILE-STASH"
echo(/loaded '${readFile("./FILE-STASH")}'/)
olamy marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
</pre>
</div>
Loading