Skip to content

Commit 91a1074

Browse files
SanderSimsonSanderSimson
committed
fix: run catch blocks only for failed steps
Signed-off-by: Sander Simson <sander.simson@wise.com>
1 parent a197055 commit 91a1074

4 files changed

Lines changed: 148 additions & 5 deletions

File tree

‎pkg/runner/runner.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (r *runner) run(ctx context.Context, m mainstart, nsOptions v1alpha2.Namesp
188188
Id: i + 1,
189189
}
190190
tc := tc.WithBinding("step", info)
191-
if stop := r.runStep(ctx, t.Cleanup, t.Fail, t.Failed, tc, step, report); stop {
191+
if stop := r.runStep(ctx, t.Cleanup, t.Fail, tc, step, report); stop {
192192
return
193193
}
194194
}
@@ -239,7 +239,6 @@ func (r *runner) runStep(
239239
ctx context.Context,
240240
cleanup func(func()),
241241
fail func(),
242-
failed func() bool,
243242
tc enginecontext.TestContext,
244243
step v1alpha1.TestStep,
245244
testReport *model.TestReport,
@@ -248,6 +247,7 @@ func (r *runner) runStep(
248247
Name: step.Name,
249248
StartTime: time.Now(),
250249
}
250+
stepFailed := false
251251
defer func() {
252252
report.EndTime = time.Now()
253253
testReport.Add(report)
@@ -327,7 +327,7 @@ func (r *runner) runStep(
327327
}
328328
if catch := tc.Catch(); len(catch) != 0 {
329329
defer func() {
330-
if failed() {
330+
if stepFailed {
331331
logging.Log(ctx, logging.Catch, logging.BeginStatus, nil, color.BoldFgCyan)
332332
defer func() {
333333
logging.Log(ctx, logging.Catch, logging.EndStatus, nil, color.BoldFgCyan)
@@ -352,6 +352,7 @@ func (r *runner) runStep(
352352
for i, operation := range step.Try {
353353
continueOnError, outputsTc, err := r.runOperation(ctx, tc, operation, i, cleaner, report)
354354
if err != nil {
355+
stepFailed = true
355356
fail()
356357
if !continueOnError {
357358
return true

‎pkg/runner/step_test.go‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package runner
33
import (
44
"context"
55
"path/filepath"
6+
"strings"
67
"testing"
78
"time"
89

@@ -785,7 +786,6 @@ func TestStepProcessor_Run(t *testing.T) {
785786
}
786787
_failed := false
787788
fail := func() { _failed = true }
788-
failed := func() bool { return _failed }
789789
cleanup := func(func()) {}
790790
ctx := context.Background()
791791
ctx = logging.WithLogger(ctx, &fakeLogger.Logger{})
@@ -802,9 +802,63 @@ func TestStepProcessor_Run(t *testing.T) {
802802
Exec: &config.Spec.Timeouts.Exec,
803803
})
804804
runner := runner{}
805-
got := runner.runStep(ctx, cleanup, fail, failed, tcontext, tc.stepSpec, &model.TestReport{})
805+
got := runner.runStep(ctx, cleanup, fail, tcontext, tc.stepSpec, &model.TestReport{})
806806
assert.Equal(t, tc.want, got)
807807
assert.Equal(t, tc.expectedFail, _failed)
808808
})
809809
}
810810
}
811+
812+
func TestRunner_CatchIsLocalToStep(t *testing.T) {
813+
logger := &fakeLogger.Logger{}
814+
ctx := logging.WithLogger(context.Background(), logger)
815+
failed := false
816+
fail := func() {
817+
failed = true
818+
}
819+
cleanup := func(func()) {}
820+
continueOnError := true
821+
execTimeout := metav1.Duration{Duration: 5 * time.Second}
822+
823+
tc := enginecontext.MakeContext(
824+
clock.RealClock{},
825+
apis.NewBindings(),
826+
mocks.Registry{},
827+
).WithTimeouts(v1alpha1.Timeouts{
828+
Exec: &execTimeout,
829+
})
830+
831+
failingStep := v1alpha1.TestStep{
832+
TestStepSpec: v1alpha1.TestStepSpec{
833+
Try: []v1alpha1.Operation{{
834+
OperationBase: v1alpha1.OperationBase{
835+
ContinueOnError: &continueOnError,
836+
},
837+
Command: &v1alpha1.Command{
838+
Entrypoint: "/bin/sh",
839+
Args: []string{"-c", "exit 1"},
840+
},
841+
}},
842+
Catch: []v1alpha1.CatchFinally{{
843+
Command: &v1alpha1.Command{Entrypoint: "/bin/sh", Args: []string{"-c", "echo failed-step-catch"}},
844+
}},
845+
},
846+
}
847+
successfulStep := v1alpha1.TestStep{
848+
TestStepSpec: v1alpha1.TestStepSpec{
849+
Try: []v1alpha1.Operation{{
850+
Command: &v1alpha1.Command{Entrypoint: "/bin/sh", Args: []string{"-c", "exit 0"}},
851+
}},
852+
Catch: []v1alpha1.CatchFinally{{
853+
Command: &v1alpha1.Command{Entrypoint: "/bin/sh", Args: []string{"-c", "echo successful-step-catch"}},
854+
}},
855+
},
856+
}
857+
858+
r := runner{}
859+
r.runStep(ctx, cleanup, fail, tc, failingStep, &model.TestReport{})
860+
r.runStep(ctx, cleanup, fail, tc, successfulStep, &model.TestReport{})
861+
862+
assert.True(t, failed)
863+
assert.Equal(t, 1, strings.Count(strings.Join(logger.Logs, "\n"), "CATCH: BEGIN"))
864+
}

‎test/continue/chainsaw-test.yaml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
apiVersion: chainsaw.kyverno.io/v1alpha1
2+
kind: Test
3+
metadata:
4+
name: chainsaw-test-catch-continue-on-error
5+
spec:
6+
steps:
7+
- name: Failed step should run its catch block
8+
try:
9+
- continueOnError: true
10+
script:
11+
content: |
12+
# Fail deliberately so the first step's catch block is expected to run.
13+
exit 1
14+
catch:
15+
- script:
16+
content: |
17+
# Emit a distinctive marker for the catch block belonging to the failed step.
18+
echo 'chainsaw-test: catch from failed step'
19+
20+
- name: Successful step A should not run its catch block
21+
try:
22+
- continueOnError: true
23+
script:
24+
content: |
25+
# Succeed deliberately so this step's catch block is expected to stay silent.
26+
exit 0
27+
catch:
28+
- script:
29+
content: |
30+
# Emit a marker that should never appear because the step succeeds.
31+
echo 'chainsaw-test: catch from successful step A'
32+
33+
- name: Successful step B should not run its catch block
34+
try:
35+
- continueOnError: true
36+
script:
37+
content: |
38+
# Succeed deliberately so this step's catch block is expected to stay silent.
39+
exit 0
40+
catch:
41+
- script:
42+
content: |
43+
# Emit a marker that should never appear because the step succeeds.
44+
echo 'chainsaw-test: catch from successful step B'

‎test/stop/chainsaw-test.yaml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
apiVersion: chainsaw.kyverno.io/v1alpha1
2+
kind: Test
3+
metadata:
4+
name: chainsaw-test-catch-continue-on-error
5+
spec:
6+
steps:
7+
- name: Failed step should run its catch block
8+
try:
9+
- continueOnError: true
10+
script:
11+
content: |
12+
# Fail deliberately so the first step's catch block is expected to run.
13+
exit 1
14+
catch:
15+
- script:
16+
content: |
17+
# Emit a distinctive marker for the catch block belonging to the failed step.
18+
echo 'chainsaw-test: catch from failed step'
19+
20+
- name: Successful step A should not run its catch block
21+
try:
22+
- continueOnError: true
23+
script:
24+
content: |
25+
# Succeed deliberately so this step's catch block is expected to stay silent.
26+
exit 0
27+
catch:
28+
- script:
29+
content: |
30+
# Emit a marker that should never appear because the step succeeds.
31+
echo 'chainsaw-test: catch from successful step A'
32+
33+
- name: Successful step B should not run its catch block
34+
try:
35+
- continueOnError: true
36+
script:
37+
content: |
38+
# Succeed deliberately so this step's catch block is expected to stay silent.
39+
exit 0
40+
catch:
41+
- script:
42+
content: |
43+
# Emit a marker that should never appear because the step succeeds.
44+
echo 'chainsaw-test: catch from successful step B'

0 commit comments

Comments
 (0)