-
Notifications
You must be signed in to change notification settings - Fork 81
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(steps): provide functions args in function conditions values #343
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
name: hello-with-conditions | ||
description: Say hello with conditions | ||
action: | ||
type: echo::hello::world | ||
configuration: | ||
name: '{{.function_args.name}}' | ||
custom_states: | ||
- SAID_HELLO | ||
- NOT_EXPECTED | ||
conditions: | ||
- type: skip | ||
if: | ||
- value: '{{.function_args.skip | default `false`}}' | ||
operator: EQ | ||
expected: 'true' | ||
then: | ||
this: PRUNE | ||
message: 'No hello {{.function_args.name}} !' | ||
- type: check | ||
if: | ||
- value: '{{.function_args.name}}' | ||
operator: NE | ||
expected: '{{.function_args.expected}}' | ||
then: | ||
this: NOT_EXPECTED | ||
message: 'Expected {{.function_args.expected}}, got {{.function_args.name}}' | ||
- type: check | ||
if: | ||
- value: '{{.function_args.name}}' | ||
operator: EQ | ||
expected: '{{.function_args.expected}}' | ||
then: | ||
this: SAID_HELLO | ||
message: 'Said hello to {{.function_args.name}}' |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -477,6 +477,23 @@ func PreRun(st *Step, values *values.Values, ss StateSetter, executedSteps map[s | |
return | ||
} | ||
|
||
// Check if we have a function as runner or not. | ||
runner, err := getRunner(st.Action.Type) | ||
if err != nil { | ||
ss(st.Name, StateServerError, err.Error()) | ||
return | ||
} | ||
|
||
_, isFunction := runner.(*functions.Function) | ||
if isFunction { | ||
var functionInput map[string]interface{} | ||
if err := utils.JSONnumberUnmarshal(bytes.NewBuffer(st.Action.Configuration), &functionInput); err != nil { | ||
ss(st.Name, StateServerError, err.Error()) | ||
return | ||
} | ||
values.SetFunctionsArgs(functionInput) | ||
} | ||
|
||
for _, sc := range conditions { | ||
if sc.Type != condition.SKIP { | ||
continue | ||
|
@@ -521,6 +538,23 @@ func AfterRun(st *Step, values *values.Values, ss StateSetter) { | |
return | ||
} | ||
|
||
// Check if we have a function as runner or not. | ||
runner, err := getRunner(st.Action.Type) | ||
if err != nil { | ||
ss(st.Name, StateServerError, err.Error()) | ||
return | ||
} | ||
|
||
_, isFunction := runner.(*functions.Function) | ||
if isFunction { | ||
var functionInput map[string]interface{} | ||
if err := utils.JSONnumberUnmarshal(bytes.NewBuffer(st.Action.Configuration), &functionInput); err != nil { | ||
ss(st.Name, StateServerError, err.Error()) | ||
return | ||
} | ||
values.SetFunctionsArgs(functionInput) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, we previously used Everytime a condition is matching, we need to alter the I guess not cloning the values is fine, but the drawback of function_args not working in conditions if they are nested seems a huge drawback that we should address before merging this one |
||
} | ||
|
||
for _, sc := range conditions { | ||
if sc.Type != condition.CHECK { | ||
continue | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
name: functionConditions | ||
description: Test function conditions | ||
title_format: "[test] Test function conditions" | ||
auto_runnable: true | ||
steps: | ||
asExpected: | ||
description: Say hello to foobar | ||
action: | ||
type: hello-with-conditions | ||
configuration: | ||
name: foobar | ||
expected: foobar | ||
|
||
notExpected: | ||
description: Say hello to foo but expect bar | ||
action: | ||
type: hello-with-conditions | ||
configuration: | ||
name: foo | ||
expected: bar | ||
|
||
skipped: | ||
description: Say hello to foo but skipping step before it starts | ||
action: | ||
type: hello-with-conditions | ||
configuration: | ||
name: foo | ||
expected: foor | ||
skip: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this condition in the test : function
nested2
has correctly been called withfoobar
, but it's failing in the test.This comes from the fact that the pull request is only using the function args of the "main" step (the one declared in the template): if we start to have nested functions, templating start to fails, because only the
function_args
on the "main" step is populated in the values.I also found why this test was failing if we used
values.Clone
, follow the second comment