From 9abc87b41634cab2ad9325287fbed9135c2664c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Brauer?= <bjoern.brauer@new-work.se>
Date: Thu, 10 Feb 2022 17:54:58 +0100
Subject: [PATCH] fix: always execute closeContainer() executor (#988)
* fix: always execute closeContainer() executor
During our earlier refactoring in #984 we accidentally changed the
behaviour in such a way that the `closeContainer()` executor was never
called.
This commit restores the earlier behaviour.
Ref:
* https://github.com/nektos/act/pull/984/files#diff-c057d66dc9657d8428e290c69871596e2b567bb8fecad62a99cab54398131a84L294
* https://github.com/nektos/act/pull/984/files#diff-ea9d5c93d769ef9b268932dd9990363e97fc3bec8a00114979d049bead5dd718R68
* test: add testcases to ensure job containers are started/stopped
This commit adds tests to ensure that the executors of `startContainer`,
`stopContainer`, `interpolateOutputs` and `closeContainer` are always
called in the correct order.
---
pkg/runner/job_executor.go | 6 +--
pkg/runner/job_executor_test.go | 92 +++++++++++++++++++++++++--------
2 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go
index c6068ae..3d86a8e 100644
--- a/pkg/runner/job_executor.go
+++ b/pkg/runner/job_executor.go
@@ -48,6 +48,7 @@ func newJobExecutor(info jobInfo) common.Executor {
return nil
})
}
+
steps = append(steps, func(ctx context.Context) error {
err := info.stopContainer()(ctx)
if err != nil {
@@ -64,8 +65,5 @@ func newJobExecutor(info jobInfo) common.Executor {
return nil
})
- return common.NewPipelineExecutor(steps...).Finally(info.interpolateOutputs()).Finally(func(ctx context.Context) error {
- info.closeContainer()
- return nil
- })
+ return common.NewPipelineExecutor(steps...).Finally(info.interpolateOutputs()).Finally(info.closeContainer())
}
diff --git a/pkg/runner/job_executor_test.go b/pkg/runner/job_executor_test.go
index 0444aef..e22bc82 100644
--- a/pkg/runner/job_executor_test.go
+++ b/pkg/runner/job_executor_test.go
@@ -62,32 +62,71 @@ func (jpm *jobInfoMock) result(result string) {
func TestNewJobExecutor(t *testing.T) {
table := []struct {
- name string
- steps []*model.Step
- result string
- hasError bool
+ name string
+ steps []*model.Step
+ executedSteps []string
+ result string
+ hasError bool
}{
{
- "zeroSteps",
- []*model.Step{},
- "success",
- false,
+ name: "zeroSteps",
+ steps: []*model.Step{},
+ executedSteps: []string{
+ "startContainer",
+ "stopContainer",
+ "interpolateOutputs",
+ "closeContainer",
+ },
+ result: "success",
+ hasError: false,
},
{
- "stepWithoutPrePost",
- []*model.Step{{
+ name: "stepWithoutPrePost",
+ steps: []*model.Step{{
ID: "1",
}},
- "success",
- false,
+ executedSteps: []string{
+ "startContainer",
+ "step1",
+ "stopContainer",
+ "interpolateOutputs",
+ "closeContainer",
+ },
+ result: "success",
+ hasError: false,
},
{
- "stepWithFailure",
- []*model.Step{{
+ name: "stepWithFailure",
+ steps: []*model.Step{{
ID: "1",
}},
- "failure",
- true,
+ executedSteps: []string{
+ "startContainer",
+ "step1",
+ "stopContainer",
+ "interpolateOutputs",
+ "closeContainer",
+ },
+ result: "failure",
+ hasError: true,
+ },
+ {
+ name: "multipleSteps",
+ steps: []*model.Step{{
+ ID: "1",
+ }, {
+ ID: "2",
+ }},
+ executedSteps: []string{
+ "startContainer",
+ "step1",
+ "step2",
+ "stopContainer",
+ "interpolateOutputs",
+ "closeContainer",
+ },
+ result: "success",
+ hasError: false,
},
}
@@ -95,41 +134,50 @@ func TestNewJobExecutor(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
ctx := common.WithJobErrorContainer(context.Background())
jpm := &jobInfoMock{}
+ executorOrder := make([]string, 0)
jpm.On("startContainer").Return(func(ctx context.Context) error {
+ executorOrder = append(executorOrder, "startContainer")
return nil
})
jpm.On("steps").Return(tt.steps)
for _, stepMock := range tt.steps {
- jpm.On("newStepExecutor", stepMock).Return(func(ctx context.Context) error {
- if tt.hasError {
- return fmt.Errorf("error")
- }
- return nil
- })
+ func(stepMock *model.Step) {
+ jpm.On("newStepExecutor", stepMock).Return(func(ctx context.Context) error {
+ executorOrder = append(executorOrder, "step"+stepMock.ID)
+ if tt.hasError {
+ return fmt.Errorf("error")
+ }
+ return nil
+ })
+ }(stepMock)
}
jpm.On("interpolateOutputs").Return(func(ctx context.Context) error {
+ executorOrder = append(executorOrder, "interpolateOutputs")
return nil
})
jpm.On("matrix").Return(map[string]interface{}{})
jpm.On("stopContainer").Return(func(ctx context.Context) error {
+ executorOrder = append(executorOrder, "stopContainer")
return nil
})
jpm.On("result", tt.result)
jpm.On("closeContainer").Return(func(ctx context.Context) error {
+ executorOrder = append(executorOrder, "closeContainer")
return nil
})
executor := newJobExecutor(jpm)
err := executor(ctx)
assert.Nil(t, err)
+ assert.Equal(t, tt.executedSteps, executorOrder)
})
}
}