yume/act
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: fix savestate in pre steps (#1466)

Browse source 
* refactor: fix savestate in pre steps

* fix pre steps collision

* fix tests

* remove

* enable tests

* Update pkg/runner/action.go

* Rename InterActionState to IntraActionState

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
ChristopherHX 2022-12-15 18:08:31 +01:00 committed by GitHub
parent a8e05cded6
commit b8d7e947cf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 90 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
pkg/model/step_result.go
View file

@ -42,5 +42,4 @@ type StepResult struct {
Outputs map[string]string `json:"outputs"`
Conclusion stepStatus `json:"conclusion"`
Outcome stepStatus `json:"outcome"`
State map[string]string
}

6
pkg/runner/action.go
View file

@ -374,9 +374,9 @@ func newStepContainer(ctx context.Context, step step, image string, cmd []string
}
func populateEnvsFromSavedState(env *map[string]string, step actionStep, rc *RunContext) {
stepResult := rc.StepResults[step.getStepModel().ID]
if stepResult != nil {
for name, value := range stepResult.State {
state, ok := rc.IntraActionState[step.getStepModel().ID]
if ok {
for name, value := range state {
envName := fmt.Sprintf("STATE_%s", name)
(*env)[envName] = value
}

7
pkg/runner/action_test.go
View file

@ -201,10 +201,11 @@ func TestActionRunner(t *testing.T) {
},
CurrentStep: "post-step",
StepResults: map[string]*model.StepResult{
"step": {},
},
IntraActionState: map[string]map[string]string{
"step": {
State: map[string]string{
"name": "state value",
},
"name": "state value",
},
},
},

17
pkg/runner/command.go
View file

@ -153,13 +153,16 @@ func unescapeKvPairs(kvPairs map[string]string) map[string]string {
}
func (rc *RunContext) saveState(ctx context.Context, kvPairs map[string]string, arg string) {
if rc.CurrentStep != "" {
stepResult := rc.StepResults[rc.CurrentStep]
if stepResult != nil {
if stepResult.State == nil {
stepResult.State = map[string]string{}
}
stepResult.State[kvPairs["name"]] = arg
stepID := rc.CurrentStep
if stepID != "" {
if rc.IntraActionState == nil {
rc.IntraActionState = map[string]map[string]string{}
}
state, ok := rc.IntraActionState[stepID]
if !ok {
state = map[string]string{}
rc.IntraActionState[stepID] = state
}
state[kvPairs["name"]] = arg
}
}

8
pkg/runner/command_test.go
View file

@ -177,11 +177,7 @@ func TestAddmaskUsemask(t *testing.T) {
func TestSaveState(t *testing.T) {
rc := &RunContext{
CurrentStep: "step",
StepResults: map[string]*model.StepResult{
"step": {
State: map[string]string{},
},
},
StepResults: map[string]*model.StepResult{},
}
ctx := context.Background()
@ -189,5 +185,5 @@ func TestSaveState(t *testing.T) {
handler := rc.commandHandler(ctx)
handler("::save-state name=state-name::state-value\n")
assert.Equal(t, "state-value", rc.StepResults["step"].State["state-name"])
assert.Equal(t, "state-value", rc.IntraActionState["step"]["state-name"])
}

1
pkg/runner/run_context.go
View file

@ -38,6 +38,7 @@ type RunContext struct {
ExtraPath []string
CurrentStep string
StepResults map[string]*model.StepResult
IntraActionState map[string]map[string]string
ExprEval ExpressionEvaluator
JobContainer container.ExecutionsEnvironment
OutputMappings map[MappableOutput]MappableOutput

41
pkg/runner/step.go
View file

@ -44,18 +44,6 @@ func (s stepStage) String() string {
return "Unknown"
}
func (s stepStage) getStepName(stepModel *model.Step) string {
switch s {
case stepStagePre:
return fmt.Sprintf("pre-%s", stepModel.ID)
case stepStageMain:
return stepModel.ID
case stepStagePost:
return fmt.Sprintf("post-%s", stepModel.ID)
}
return "unknown"
}
func runStepExecutor(step step, stage stepStage, executor common.Executor) common.Executor {
return func(ctx context.Context) error {
logger := common.Logger(ctx)
@ -63,13 +51,16 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
stepModel := step.getStepModel()
ifExpression := step.getIfExpression(ctx, stage)
rc.CurrentStep = stage.getStepName(stepModel)
rc.CurrentStep = stepModel.ID
rc.StepResults[rc.CurrentStep] = &model.StepResult{
stepResult := &model.StepResult{
Outcome: model.StepStatusSuccess,
Conclusion: model.StepStatusSuccess,
Outputs: make(map[string]string),
}
if stage == stepStageMain {
rc.StepResults[rc.CurrentStep] = stepResult
}
err := setupEnv(ctx, step)
if err != nil {
@ -78,15 +69,15 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
runStep, err := isStepEnabled(ctx, ifExpression, step, stage)
if err != nil {
rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusFailure
rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusFailure
stepResult.Conclusion = model.StepStatusFailure
stepResult.Outcome = model.StepStatusFailure
return err
}
if !runStep {
rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusSkipped
rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusSkipped
logger.WithField("stepResult", rc.StepResults[rc.CurrentStep].Outcome).Debugf("Skipping step '%s' due to '%s'", stepModel, ifExpression)
stepResult.Conclusion = model.StepStatusSkipped
stepResult.Outcome = model.StepStatusSkipped
logger.WithField("stepResult", stepResult.Outcome).Debugf("Skipping step '%s' due to '%s'", stepModel, ifExpression)
return nil
}
@ -118,25 +109,25 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
err = executor(ctx)
if err == nil {
logger.WithField("stepResult", rc.StepResults[rc.CurrentStep].Outcome).Infof(" \u2705 Success - %s %s", stage, stepString)
logger.WithField("stepResult", stepResult.Outcome).Infof(" \u2705 Success - %s %s", stage, stepString)
} else {
rc.StepResults[rc.CurrentStep].Outcome = model.StepStatusFailure
stepResult.Outcome = model.StepStatusFailure
continueOnError, parseErr := isContinueOnError(ctx, stepModel.RawContinueOnError, step, stage)
if parseErr != nil {
rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusFailure
stepResult.Conclusion = model.StepStatusFailure
return parseErr
}
if continueOnError {
logger.Infof("Failed but continue next step")
err = nil
rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusSuccess
stepResult.Conclusion = model.StepStatusSuccess
} else {
rc.StepResults[rc.CurrentStep].Conclusion = model.StepStatusFailure
stepResult.Conclusion = model.StepStatusFailure
}
logger.WithField("stepResult", rc.StepResults[rc.CurrentStep].Outcome).Errorf(" \u274C Failure - %s %s", stage, stepString)
logger.WithField("stepResult", stepResult.Outcome).Errorf(" \u274C Failure - %s %s", stage, stepString)
}
// Process Runner File Commands
orgerr := err

31
pkg/runner/step_action_local_test.go
View file

@ -107,13 +107,12 @@ func TestStepActionLocalTest(t *testing.T) {
func TestStepActionLocalPost(t *testing.T) {
table := []struct {
name string
stepModel *model.Step
actionModel *model.Action
initialStepResults map[string]*model.StepResult
expectedPostStepResult *model.StepResult
err error
mocks struct {
name string
stepModel *model.Step
actionModel *model.Action
initialStepResults map[string]*model.StepResult
err error
mocks struct {
env bool
exec bool
}
@ -138,11 +137,6 @@ func TestStepActionLocalPost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: &model.StepResult{
Conclusion: model.StepStatusSuccess,
Outcome: model.StepStatusSuccess,
Outputs: map[string]string{},
},
mocks: struct {
env bool
exec bool
@ -171,11 +165,6 @@ func TestStepActionLocalPost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: &model.StepResult{
Conclusion: model.StepStatusSuccess,
Outcome: model.StepStatusSuccess,
Outputs: map[string]string{},
},
mocks: struct {
env bool
exec bool
@ -204,11 +193,6 @@ func TestStepActionLocalPost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: &model.StepResult{
Conclusion: model.StepStatusSkipped,
Outcome: model.StepStatusSkipped,
Outputs: map[string]string{},
},
mocks: struct {
env bool
exec bool
@ -238,7 +222,6 @@ func TestStepActionLocalPost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: nil,
mocks: struct {
env bool
exec bool
@ -307,7 +290,7 @@ func TestStepActionLocalPost(t *testing.T) {
err := sal.post()(ctx)
assert.Equal(t, tt.err, err)
assert.Equal(t, tt.expectedPostStepResult, sal.RunContext.StepResults["post-step"])
assert.Equal(t, sal.RunContext.StepResults["post-step"], (*model.StepResult)(nil))
cm.AssertExpectations(t)
})
}

46
pkg/runner/step_action_remote_test.go
View file

@ -415,14 +415,14 @@ func TestStepActionRemotePreThroughActionToken(t *testing.T) {
func TestStepActionRemotePost(t *testing.T) {
table := []struct {
name string
stepModel *model.Step
actionModel *model.Action
initialStepResults map[string]*model.StepResult
expectedEnv map[string]string
expectedPostStepResult *model.StepResult
err error
mocks struct {
name string
stepModel *model.Step
actionModel *model.Action
initialStepResults map[string]*model.StepResult
IntraActionState map[string]map[string]string
expectedEnv map[string]string
err error
mocks struct {
env bool
exec bool
}
@ -445,19 +445,16 @@ func TestStepActionRemotePost(t *testing.T) {
Conclusion: model.StepStatusSuccess,
Outcome: model.StepStatusSuccess,
Outputs: map[string]string{},
State: map[string]string{
"key": "value",
},
},
},
IntraActionState: map[string]map[string]string{
"step": {
"key": "value",
},
},
expectedEnv: map[string]string{
"STATE_key": "value",
},
expectedPostStepResult: &model.StepResult{
Conclusion: model.StepStatusSuccess,
Outcome: model.StepStatusSuccess,
Outputs: map[string]string{},
},
mocks: struct {
env bool
exec bool
@ -486,11 +483,6 @@ func TestStepActionRemotePost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: &model.StepResult{
Conclusion: model.StepStatusSuccess,
Outcome: model.StepStatusSuccess,
Outputs: map[string]string{},
},
mocks: struct {
env bool
exec bool
@ -519,11 +511,6 @@ func TestStepActionRemotePost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: &model.StepResult{
Conclusion: model.StepStatusSkipped,
Outcome: model.StepStatusSkipped,
Outputs: map[string]string{},
},
mocks: struct {
env bool
exec bool
@ -553,7 +540,6 @@ func TestStepActionRemotePost(t *testing.T) {
Outputs: map[string]string{},
},
},
expectedPostStepResult: nil,
mocks: struct {
env bool
exec bool
@ -585,7 +571,8 @@ func TestStepActionRemotePost(t *testing.T) {
},
},
},
StepResults: tt.initialStepResults,
StepResults: tt.initialStepResults,
IntraActionState: tt.IntraActionState,
},
Step: tt.stepModel,
action: tt.actionModel,
@ -622,7 +609,8 @@ func TestStepActionRemotePost(t *testing.T) {
assert.Equal(t, value, sar.env[key])
}
}
assert.Equal(t, tt.expectedPostStepResult, sar.RunContext.StepResults["post-step"])
// Enshure that StepResults is nil in this test
assert.Equal(t, sar.RunContext.StepResults["post-step"], (*model.StepResult)(nil))
cm.AssertExpectations(t)
})
}

34
pkg/runner/testdata/GITHUB_STATE/push.yml vendored
View file

@ -15,8 +15,34 @@ jobs:
echo "::save-state name=mystate3::mystateval"
post: |
env
# Enable once https://github.com/nektos/act/issues/1459 is fixed
# [ "$STATE_mystate0" = "mystateval" ]
# [ "$STATE_mystate1" = "mystateval" ]
[ "$STATE_mystate0" = "mystateval" ]
[ "$STATE_mystate1" = "mystateval" ]
[ "$STATE_mystate2" = "mystateval" ]
[ "$STATE_mystate3" = "mystateval" ]
[ "$STATE_mystate3" = "mystateval" ]
test-id-collision-bug:
runs-on: ubuntu-latest
steps:
- uses: nektos/act-test-actions/script@main
id: script
with:
pre: |
env
echo mystate0=mystateval > $GITHUB_STATE
echo "::save-state name=mystate1::mystateval"
main: |
env
echo mystate2=mystateval > $GITHUB_STATE
echo "::save-state name=mystate3::mystateval"
post: |
env
[ "$STATE_mystate0" = "mystateval" ]
[ "$STATE_mystate1" = "mystateval" ]
[ "$STATE_mystate2" = "mystateval" ]
[ "$STATE_mystate3" = "mystateval" ]
- uses: nektos/act-test-actions/script@main
id: pre-script
with:
main: |
env
echo mystate0=mystateerror > $GITHUB_STATE
echo "::save-state name=mystate1::mystateerror"