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

fix(controller): prepend script path to the script template args. Resolves #4481 #4492

Merged
merged 6 commits into from Nov 10, 2020
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
wrote tests for various configs of script template
Signed-off-by: ivancili <[email protected]>
  • Loading branch information
ivancili committed Nov 10, 2020
commit ce11e2a236e03e297cf521c3858b0a130cec76b4
81 changes: 81 additions & 0 deletions workflow/controller/workflowpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,87 @@ func TestScriptTemplateWithVolume(t *testing.T) {
assert.NoError(t, err)
}

var scriptTemplateWithArgsAndWithSource = `
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I'd never say this, but there is a first time for everything.

I think you've written TOO many tests.

I think you only need to test `extractMainCtrFromScriptTemplate.

Copy link
Author

@ghost ghost Nov 10, 2020

Choose a reason for hiding this comment

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

Haha yeah probably, no problem I'll delete those that don't test extractMainCtrFromScriptTemplate explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool - I'll mark this as "Draft" while you do that

Copy link
Author

Choose a reason for hiding this comment

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

Here you go, I left only 2 tests which cover extractMainCtrFromScriptTemplate. One for when script source is specified, and the other one when the source is empty.

name: script-with-args-and-with-source
script:
image: alpine:latest
command: [sh]
args: ["hello world"]
source: |
echo $@
`

// TestScriptTemplateWithArgsAndWithSource ensure we can run a script template
// when both args and script source are specified
func TestScriptTemplateWithArgsAndWithSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithArgsAndWithSource)
woc := newWoc()
_, err := woc.executeScript(tmpl.Name, "", tmpl, tmpl, &executeTemplateOpts{})
assert.NoError(t, err)
}

// TestScriptTemplateMainCtrArgsWhenArgsAndWhenSource ensure order of merged
// args and script path is correct when both args and script source are specified
func TestScriptTemplateMainCtrArgsWhenArgsAndWhenSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithArgsAndWithSource)
mainCtr := extractMainCtrFromScriptTemplate(tmpl)
assert.Equal(t, []string{"sh"}, mainCtr.Command)
assert.Equal(t, []string{common.ExecutorScriptSourcePath, "hello world"}, mainCtr.Args)
}

var scriptTemplateWithArgsAndWithoutSource = `
name: script-with-args-and-without-source
script:
image: alpine:latest
command: [echo]
args: ["hello world"]
`

// TestScriptTemplateWithArgsAndWithoutSource ensure we can run a script template
// when args are specified but script source is empty
func TestScriptTemplateWithArgsAndWithoutSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithArgsAndWithoutSource)
woc := newWoc()
_, err := woc.executeScript(tmpl.Name, "", tmpl, tmpl, &executeTemplateOpts{})
assert.NoError(t, err)
}

// TestScriptTemplateMainCtrArgsWhenArgsAndWhenNoSource ensure only args are passed
// to the resulting container when script source is empty
func TestScriptTemplateMainCtrArgsWhenArgsAndWhenNoSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithArgsAndWithoutSource)
mainCtr := extractMainCtrFromScriptTemplate(tmpl)
assert.Equal(t, []string{"echo"}, mainCtr.Command)
assert.Equal(t, []string{"hello world"}, mainCtr.Args)
}

var scriptTemplateWithoutArgsAndWithSource = `
name: script-without-args-and-with-source
script:
image: alpine:latest
command: [sh]
source: |
echo "hello world"
`

// TestScriptTemplateWithoutArgsAndWithSource ensure we can run a script template
// that has no args and but script source is specified
func TestScriptTemplateWithoutArgsAndWithSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithoutArgsAndWithSource)
woc := newWoc()
_, err := woc.executeScript(tmpl.Name, "", tmpl, tmpl, &executeTemplateOpts{})
assert.NoError(t, err)
}

// TestScriptTemplateMainCtrArgsWhenNoArgsAndWhenSource ensure only script path is passed
// as an arg to the resulting container when there are no args but script source is specified
func TestScriptTemplateMainCtrArgsWhenNoArgsAndWhenSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithoutArgsAndWithSource)
mainCtr := extractMainCtrFromScriptTemplate(tmpl)
assert.Equal(t, []string{"sh"}, mainCtr.Command)
assert.Equal(t, []string{common.ExecutorScriptSourcePath}, mainCtr.Args)
}

var scriptTemplateWithOptionalInputArtifactProvided = `
name: script-with-input-artifact
inputs:
Expand Down