fix(controller): prepend script path to the script template args. Resolves #4481 #4492
Conversation
ghost
marked this pull request as ready for review
There was a problem hiding this comment.
@alexec The function extractMainCtrFromScriptTemplate that I wrote could also be used by the test TestScriptTemplateWithoutVolumeOptionalArtifact, which was previously written inside workflowpod_test.go. Let me know if you want me to update the PR with those changes.
| @@ -69,6 +69,87 @@ func TestScriptTemplateWithVolume(t *testing.T) { | |||
| assert.NoError(t, err) | |||
| } | |||
|
|
|||
| var scriptTemplateWithArgsAndWithSource = ` | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Haha yeah probably, no problem I'll delete those that don't test extractMainCtrFromScriptTemplate explicitly.
There was a problem hiding this comment.
cool - I'll mark this as "Draft" while you do that
There was a problem hiding this comment.
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.
|
So I deleted those unnecessary tests that you pointed out, and now some existing tests that were passing before are failing. And everything is passing locally. |
alexec
changed the title
ghost
marked this pull request as ready for review
Signed-off-by: ivancili <[email protected]>
Signed-off-by: ivancili <[email protected]>
Signed-off-by: ivancili <[email protected]>
Signed-off-by: ivancili <[email protected]>
Signed-off-by: ivancili <[email protected]>
Signed-off-by: ivancili <[email protected]>
…olves argoproj#4481 (argoproj#4492) Signed-off-by: ivancili <[email protected]> Signed-off-by: Alex Capras <[email protected]>
Signed-off-by: ivancili [email protected]
Checklist:
Resolves #4481