Test: add more tests in discover and custom

Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com>

pkg/workflow/tasks/custom/task.go

@@ -90,7 +90,7 @@ type taskRunner struct {
 	name         string
 	run          func(ctx wfContext.Context, options *wfTypes.TaskRunOptions) (common.StepStatus, *wfTypes.Operation, error)
 	checkPending func(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool
-	skip         func(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
+	skip         func(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
 }
 
 // Name return step name.
@@ -108,8 +108,8 @@ func (tr *taskRunner) Pending(ctx wfContext.Context, stepStatus map[string]commo
 	return tr.checkPending(ctx, stepStatus)
 }
 
-func (tr *taskRunner) Skip(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
-	return tr.skip(ctx, dependsOnPhase, stepStatus)
+func (tr *taskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+	return tr.skip(dependsOnPhase, stepStatus)
 }
 
 // nolint:gocyclo
@@ -154,15 +154,13 @@ func (t *TaskLoader) makeTaskGenerator(templ string) (wfTypes.TaskGenerator, err
 		tRunner.checkPending = func(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool {
 			return CheckPending(ctx, wfStep, stepStatus)
 		}
-		tRunner.skip = func(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+		tRunner.skip = func(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
 			if EnableSuspendFailedWorkflow {
 				return exec.status(), false
 			}
-			skip := SkipTaskRunner(ctx, &SkipOptions{
+			skip := SkipTaskRunner(&SkipOptions{
 				If:             wfStep.If,
-				DependsOn:      wfStep.DependsOn,
 				DependsOnPhase: dependsOnPhase,
-				StepStatus:     stepStatus,
 			})
 			if skip {
 				exec.Skip("")
@@ -469,13 +467,11 @@ func NewTaskLoader(lt LoadTaskTemplate, pkgDiscover *packages.PackageDiscover, h
 // SkipOptions is the options of skip task runner
 type SkipOptions struct {
 	If             string
-	DependsOn      []string
 	DependsOnPhase common.WorkflowStepPhase
-	StepStatus     map[string]common.StepStatus
 }
 
 // SkipTaskRunner will decide whether to skip task runner.
-func SkipTaskRunner(ctx wfContext.Context, options *SkipOptions) bool {
+func SkipTaskRunner(options *SkipOptions) bool {
 	switch options.If {
 	case "always":
 		return false

pkg/workflow/tasks/custom/task_test.go

@@ -481,6 +481,41 @@ func TestPendingDependsOnCheck(t *testing.T) {
 	r.Equal(run.Pending(wfCtx, ss), false)
 }
 
+func TestSkip(t *testing.T) {
+	r := require.New(t)
+	discover := providers.NewProviders()
+	discover.Register("test", map[string]providers.Handler{
+		"ok": func(ctx wfContext.Context, v *value.Value, act types.Action) error {
+			return nil
+		},
+	})
+	step := v1beta1.WorkflowStep{
+		Name: "skip",
+		Type: "ok",
+	}
+	pCtx := process.NewContext(process.ContextData{
+		AppName:         "myapp",
+		CompName:        "mycomp",
+		Namespace:       "default",
+		AppRevisionName: "myapp-v1",
+	})
+	tasksLoader := NewTaskLoader(mockLoadTemplate, nil, discover, 0, pCtx)
+	gen, err := tasksLoader.GetTaskGenerator(context.Background(), step.Type)
+	r.NoError(err)
+	runner, err := gen(step, &types.GeneratorOptions{})
+	r.NoError(err)
+	status, skip := runner.Skip(common.WorkflowStepPhaseFailedAfterRetries, nil)
+	r.Equal(skip, true)
+	r.Equal(status.Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Reason, StatusReasonSkip)
+	runner2, err := gen(v1beta1.WorkflowStep{
+		If:   "always",
+		Name: "test",
+	}, &types.GeneratorOptions{ID: "124"})
+	_, skip = runner2.Skip(common.WorkflowStepPhaseFailedAfterRetries, nil)
+	r.Equal(skip, false)
+}
+
 func newWorkflowContextForTest(t *testing.T) wfContext.Context {
 	r := require.New(t)
 	cm := corev1.ConfigMap{}

pkg/workflow/tasks/discover.go

@@ -153,7 +153,7 @@ func (tr *suspendTaskRunner) Pending(ctx wfContext.Context, stepStatus map[strin
 	return custom.CheckPending(ctx, tr.step, stepStatus)
 }
 
-func (tr *suspendTaskRunner) Skip(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+func (tr *suspendTaskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
 	status := common.StepStatus{
 		ID:    tr.id,
 		Name:  tr.step.Name,
@@ -163,11 +163,9 @@ func (tr *suspendTaskRunner) Skip(ctx wfContext.Context, dependsOnPhase common.W
 	if custom.EnableSuspendFailedWorkflow {
 		return status, false
 	}
-	skip := custom.SkipTaskRunner(ctx, &custom.SkipOptions{
+	skip := custom.SkipTaskRunner(&custom.SkipOptions{
 		If:             tr.step.If,
-		DependsOn:      tr.step.DependsOn,
 		DependsOnPhase: dependsOnPhase,
-		StepStatus:     stepStatus,
 	})
 	if skip {
 		status.Phase = common.WorkflowStepPhaseSkipped
@@ -193,7 +191,7 @@ func (tr *stepGroupTaskRunner) Pending(ctx wfContext.Context, stepStatus map[str
 	return custom.CheckPending(ctx, tr.step, stepStatus)
 }
 
-func (tr *stepGroupTaskRunner) Skip(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+func (tr *stepGroupTaskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
 	status := common.StepStatus{
 		ID:   tr.id,
 		Name: tr.step.Name,
@@ -202,11 +200,9 @@ func (tr *stepGroupTaskRunner) Skip(ctx wfContext.Context, dependsOnPhase common
 	if custom.EnableSuspendFailedWorkflow {
 		return status, false
 	}
-	skip := custom.SkipTaskRunner(ctx, &custom.SkipOptions{
+	skip := custom.SkipTaskRunner(&custom.SkipOptions{
 		If:             tr.step.If,
-		DependsOn:      tr.step.DependsOn,
 		DependsOnPhase: dependsOnPhase,
-		StepStatus:     stepStatus,
 	})
 	if skip {
 		status.Phase = common.WorkflowStepPhaseSkipped

pkg/workflow/tasks/discover_test.go

@@ -18,22 +18,15 @@ package tasks
 
 import (
 	"context"
-	"encoding/json"
 	"testing"
 
-	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/yaml"
 
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 
-	"github.com/oam-dev/kubevela/pkg/cue/model/value"
 	"github.com/oam-dev/kubevela/pkg/cue/process"
-	wfContext "github.com/oam-dev/kubevela/pkg/workflow/context"
 	"github.com/oam-dev/kubevela/pkg/workflow/tasks/custom"
 	"github.com/oam-dev/kubevela/pkg/workflow/types"
 )
@@ -90,10 +83,35 @@ func TestSuspendStep(t *testing.T) {
 	}
 	gen, err := discover.GetTaskGenerator(context.Background(), "suspend")
 	r.NoError(err)
-	runner, err := gen(v1beta1.WorkflowStep{Name: "test"}, &types.GeneratorOptions{ID: "124"})
+	runner, err := gen(v1beta1.WorkflowStep{
+		Name:      "test",
+		DependsOn: []string{"depend"},
+	}, &types.GeneratorOptions{ID: "124"})
 	r.NoError(err)
 	r.Equal(runner.Name(), "test")
-	r.Equal(runner.Pending(nil, nil), false)
+
+	// test pending
+	r.Equal(runner.Pending(nil, nil), true)
+	ss := map[string]common.StepStatus{
+		"depend": {
+			Phase: common.WorkflowStepPhaseSucceeded,
+		},
+	}
+	r.Equal(runner.Pending(nil, ss), false)
+
+	// test skip
+	status, skip := runner.Skip(common.WorkflowStepPhaseFailedAfterRetries, nil)
+	r.Equal(skip, true)
+	r.Equal(status.Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Reason, custom.StatusReasonSkip)
+	runner2, err := gen(v1beta1.WorkflowStep{
+		If:   "always",
+		Name: "test",
+	}, &types.GeneratorOptions{ID: "124"})
+	_, skip = runner2.Skip(common.WorkflowStepPhaseFailedAfterRetries, nil)
+	r.Equal(skip, false)
+
+	// test run
 	status, act, err := runner.Run(nil, nil)
 	r.NoError(err)
 	r.Equal(act.Suspend, true)
@@ -135,11 +153,37 @@ func TestStepGroupStep(t *testing.T) {
 	r.NoError(err)
 	gen, err := discover.GetTaskGenerator(context.Background(), "stepGroup")
 	r.NoError(err)
-	runner, err := gen(v1beta1.WorkflowStep{Name: "test"}, &types.GeneratorOptions{ID: "124", SubTaskRunners: []types.TaskRunner{subRunner}})
+	runner, err := gen(v1beta1.WorkflowStep{
+		Name:      "test",
+		DependsOn: []string{"depend"},
+	}, &types.GeneratorOptions{ID: "124", SubTaskRunners: []types.TaskRunner{subRunner}})
 	r.NoError(err)
 	r.Equal(runner.Name(), "test")
-	r.Equal(runner.Pending(nil, nil), false)
 
+	// test pending
+	r.Equal(runner.Pending(nil, nil), true)
+	ss := map[string]common.StepStatus{
+		"depend": {
+			Phase: common.WorkflowStepPhaseSucceeded,
+		},
+	}
+	r.Equal(runner.Pending(nil, ss), false)
+
+	// test skip
+	stepStatus := make(map[string]common.StepStatus)
+	status, skip := runner.Skip(common.WorkflowStepPhaseFailedAfterRetries, stepStatus)
+	r.Equal(skip, false)
+	r.Equal(stepStatus["test"].Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Reason, custom.StatusReasonSkip)
+	runner2, err := gen(v1beta1.WorkflowStep{
+		If:   "always",
+		Name: "test",
+	}, &types.GeneratorOptions{ID: "124"})
+	_, skip = runner2.Skip(common.WorkflowStepPhaseFailedAfterRetries, stepStatus)
+	r.Equal(skip, false)
+
+	// test run
 	testCases := []struct {
 		name          string
 		engine        *testEngine
@@ -262,66 +306,3 @@ func TestStepGroupStep(t *testing.T) {
 		})
 	}
 }
-
-func TestPendingDependsOnCheck(t *testing.T) {
-	wfCtx := newWorkflowContextForTest(t)
-	r := require.New(t)
-	step := v1beta1.WorkflowStep{
-		Name:      "pending",
-		Type:      "suspend",
-		DependsOn: []string{"depend"},
-	}
-	discover := &taskDiscover{
-		builtins: map[string]types.TaskGenerator{
-			"suspend": suspend,
-		},
-	}
-	gen, err := discover.GetTaskGenerator(context.Background(), step.Type)
-	r.NoError(err)
-	run, err := gen(step, &types.GeneratorOptions{})
-	r.NoError(err)
-	r.Equal(run.Pending(wfCtx, nil), true)
-	ss := map[string]common.StepStatus{
-		"depend": {
-			Phase: common.WorkflowStepPhaseSucceeded,
-		},
-	}
-	r.Equal(run.Pending(wfCtx, ss), false)
-}
-
-func newWorkflowContextForTest(t *testing.T) wfContext.Context {
-	r := require.New(t)
-	cm := corev1.ConfigMap{}
-	testCaseJson, err := yaml.YAMLToJSON([]byte(testCaseYaml))
-	r.NoError(err)
-	err = json.Unmarshal(testCaseJson, &cm)
-	r.NoError(err)
-
-	cli := &test.MockClient{
-		MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error {
-			o, ok := obj.(*corev1.ConfigMap)
-			if ok {
-				*o = cm
-			}
-			return nil
-		},
-		MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
-			return nil
-		},
-	}
-	wfCtx, err := wfContext.NewContext(cli, "default", "app-v1", "testuid")
-	r.NoError(err)
-	v, _ := value.NewValue(`name: "app"`, nil, "")
-	r.NoError(wfCtx.SetVar(v, types.ContextKeyMetadata))
-	return wfCtx
-}
-
-var (
-	testCaseYaml = `apiVersion: v1
-data:
-  components: '{"server":"{\"Scopes\":null,\"StandardWorkload\":\"{\\\"apiVersion\\\":\\\"v1\\\",\\\"kind\\\":\\\"Pod\\\",\\\"metadata\\\":{\\\"labels\\\":{\\\"app\\\":\\\"nginx\\\"}},\\\"spec\\\":{\\\"containers\\\":[{\\\"env\\\":[{\\\"name\\\":\\\"APP\\\",\\\"value\\\":\\\"nginx\\\"}],\\\"image\\\":\\\"nginx:1.14.2\\\",\\\"imagePullPolicy\\\":\\\"IfNotPresent\\\",\\\"name\\\":\\\"main\\\",\\\"ports\\\":[{\\\"containerPort\\\":8080,\\\"protocol\\\":\\\"TCP\\\"}]}]}}\",\"Traits\":[\"{\\\"apiVersion\\\":\\\"v1\\\",\\\"kind\\\":\\\"Service\\\",\\\"metadata\\\":{\\\"name\\\":\\\"my-service\\\"},\\\"spec\\\":{\\\"ports\\\":[{\\\"port\\\":80,\\\"protocol\\\":\\\"TCP\\\",\\\"targetPort\\\":8080}],\\\"selector\\\":{\\\"app\\\":\\\"nginx\\\"}}}\"]}"}'
-kind: ConfigMap
-metadata:
-  name: app-v1
-`
-)

pkg/workflow/types/types.go

@@ -32,7 +32,7 @@ type TaskRunner interface {
 	Name() string
 	Pending(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool
 	Run(ctx wfContext.Context, options *TaskRunOptions) (common.StepStatus, *Operation, error)
-	Skip(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
+	Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
 }
 
 // TaskDiscover is the interface to obtain the TaskGenerator。

pkg/workflow/workflow.go

@@ -563,7 +563,7 @@ func (e *engine) steps(taskRunners []wfTypes.TaskRunner, dag bool) error {
 		var status common.StepStatus
 		operation := &wfTypes.Operation{}
 		skip := false
-		status, skip = runner.Skip(wfCtx, e.findDependPhase(taskRunners, index, dag), e.stepStatus)
+		status, skip = runner.Skip(e.findDependPhase(taskRunners, index, dag), e.stepStatus)
 		if !skip {
 			status, operation, err = runner.Run(wfCtx, options)
 			if err != nil {

pkg/workflow/workflow_test.go

@@ -1381,7 +1381,7 @@ var pending bool
 
 func makeRunner(name, tpy, ifDecl string, dependsOn []string, subTaskRunners []wfTypes.TaskRunner) wfTypes.TaskRunner {
 	var run func(ctx wfContext.Context, options *wfTypes.TaskRunOptions) (common.StepStatus, *wfTypes.Operation, error)
-	skip := func(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+	skip := func(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
 		status := common.StepStatus{
 			Name: name,
 			Type: tpy,
@@ -1389,11 +1389,9 @@ func makeRunner(name, tpy, ifDecl string, dependsOn []string, subTaskRunners []w
 		if custom.EnableSuspendFailedWorkflow {
 			return status, false
 		}
-		skip := custom.SkipTaskRunner(ctx, &custom.SkipOptions{
+		skip := custom.SkipTaskRunner(&custom.SkipOptions{
 			If:             ifDecl,
-			DependsOn:      dependsOn,
 			DependsOnPhase: dependsOnPhase,
-			StepStatus:     stepStatus,
 		})
 		if skip {
 			status.Phase = common.WorkflowStepPhaseSkipped
@@ -1521,7 +1519,7 @@ type testTaskRunner struct {
 	name         string
 	run          func(ctx wfContext.Context, options *wfTypes.TaskRunOptions) (common.StepStatus, *wfTypes.Operation, error)
 	checkPending func(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool
-	skip         func(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
+	skip         func(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
 }
 
 // Name return step name.
@@ -1539,8 +1537,8 @@ func (tr *testTaskRunner) Pending(ctx wfContext.Context, stepStatus map[string]c
 	return tr.checkPending(ctx, stepStatus)
 }
 
-func (tr *testTaskRunner) Skip(ctx wfContext.Context, dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
-	return tr.skip(ctx, dependsOnPhase, stepStatus)
+func (tr *testTaskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+	return tr.skip(dependsOnPhase, stepStatus)
 }
 
 func cleanStepTimeStamp(wfStatus *common.WorkflowStatus) {