Some cleanup in taskapi (#271)

devbuddy · Jan 13, 2019 · 008719e · 008719e
1 parent 3bc145b
commit 008719e
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 42 deletions.
diff --git a/pkg/tasks/taskapi/task_action_builder.go b/pkg/tasks/taskapi/task_action_builder.go
@@ -1,18 +1,9 @@
 package taskapi
 
-type genericTaskActionCondition struct {
-	pre  func(*Context) *ActionResult
-	post func(*Context) *ActionResult
-}
-
 type genericTaskActionBuilder struct {
 	*genericTaskAction
 }
 
-func actionBuilder(description string, runFunc func(*Context) error) *genericTaskActionBuilder {
-	return &genericTaskActionBuilder{&genericTaskAction{desc: description, runFunc: runFunc}}
-}
-
 // On registers a new condition
 func (a *genericTaskActionBuilder) On(condition *genericTaskActionCondition) *genericTaskActionBuilder {
 	a.conditions = append(a.conditions, condition)
@@ -32,8 +23,3 @@ func (a *genericTaskActionBuilder) OnFileChange(path string) *genericTaskActionB
 	a.monitoredFiles = append(a.monitoredFiles, path)
 	return a
 }
-
-// Build returns a new task action with the behaviour specified by the builder
-func (a *genericTaskActionBuilder) Build() *genericTaskAction {
-	return a.genericTaskAction
-}
diff --git a/pkg/tasks/taskapi/task_action_generic.go b/pkg/tasks/taskapi/task_action_generic.go
@@ -16,6 +16,11 @@ type genericTaskAction struct {
 	runCalled bool
 }
 
+type genericTaskActionCondition struct {
+	pre  func(*Context) *ActionResult
+	post func(*Context) *ActionResult
+}
+
 func (a *genericTaskAction) Description() string {
 	return a.desc
 }

diff --git a/pkg/tasks/taskapi/task_action_generic_test.go b/pkg/tasks/taskapi/task_action_generic_test.go
@@ -10,14 +10,17 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func newBuilder(description string, runFunc func(*Context) error) *genericTaskActionBuilder {
+	return &genericTaskActionBuilder{&genericTaskAction{desc: description, runFunc: runFunc}}
+}
+
 func TestTaskActionGenericRun(t *testing.T) {
 	runCalls := 0
 
-	builder := actionBuilder("", func(ctx *Context) error {
+	action := newBuilder("", func(ctx *Context) error {
 		runCalls++
 		return nil
-	})
-	action := builder.Build()
+	}).genericTaskAction
 
 	action.Run(&Context{})
 	require.Equal(t, 1, runCalls)
@@ -26,25 +29,22 @@ func TestTaskActionGenericRun(t *testing.T) {
 func TestTaskActionGenericRunError(t *testing.T) {
 	dummyError := errors.New("dummy")
 
-	builder := actionBuilder("", func(ctx *Context) error {
+	action := newBuilder("", func(ctx *Context) error {
 		return dummyError
-	})
-	action := builder.Build()
+	}).genericTaskAction
 
 	err := action.Run(&Context{})
 	require.Equal(t, dummyError, err)
 }
 
 func TestTaskActionGenericDescription(t *testing.T) {
-	builder := actionBuilder("dummy desc", nil)
-	action := builder.Build()
+	action := newBuilder("dummy desc", nil)
 
 	require.Equal(t, "dummy desc", action.Description())
 }
 
 func TestTaskActionGenericNoConditions(t *testing.T) {
-	builder := actionBuilder("", func(ctx *Context) error { return nil })
-	action := builder.Build()
+	action := newBuilder("", func(ctx *Context) error { return nil }).genericTaskAction
 
 	result := action.Needed(&Context{})
 	require.NoError(t, result.Error)
@@ -68,7 +68,7 @@ func TestTaskActionGenericConditions(t *testing.T) {
 	result1 := ActionNeeded("pre reason")
 	result2 := ActionNeeded("post reason")
 
-	builder := actionBuilder("", func(ctx *Context) error { return nil })
+	builder := newBuilder("", func(ctx *Context) error { return nil })
 	builder.On(&genericTaskActionCondition{
 		pre:  func(ctx *Context) *ActionResult { pre1Calls++; return ActionNotNeeded() },
 		post: func(ctx *Context) *ActionResult { post1Calls++; return ActionNotNeeded() },
@@ -81,7 +81,7 @@ func TestTaskActionGenericConditions(t *testing.T) {
 		pre:  func(ctx *Context) *ActionResult { pre3Calls++; return ActionNotNeeded() },
 		post: func(ctx *Context) *ActionResult { post3Calls++; return ActionNotNeeded() },
 	})
-	action := builder.Build()
+	action := builder.genericTaskAction
 
 	result := action.Needed(&Context{})
 	require.Equal(t, result1, result)
@@ -108,13 +108,13 @@ func TestTaskActionGenericOnFunc(t *testing.T) {
 	calls := 0
 	results := []*ActionResult{ActionNeeded("reason 1"), ActionNotNeeded()}
 
-	builder := actionBuilder("", func(ctx *Context) error { return nil })
+	builder := newBuilder("", func(ctx *Context) error { return nil })
 	builder.OnFunc(func(ctx *Context) *ActionResult {
 		index := calls
 		calls++
 		return results[index]
 	})
-	action := builder.Build()
+	action := builder.genericTaskAction
 
 	result := action.Needed(&Context{})
 	require.NoError(t, result.Error)
@@ -138,7 +138,7 @@ func TestTaskActionGenericFileChange(t *testing.T) {
 
 	// Without file
 
-	action := actionBuilder("", runFunc).OnFileChange("testfile").Build()
+	action := newBuilder("", runFunc).OnFileChange("testfile").genericTaskAction
 
 	result := action.Needed(ctx)
 	require.NoError(t, result.Error)
@@ -154,7 +154,7 @@ func TestTaskActionGenericFileChange(t *testing.T) {
 
 	filet.File(t, tmpdir+"/testfile", "content-A")
 
-	action = actionBuilder("", runFunc).OnFileChange("testfile").Build()
+	action = newBuilder("", runFunc).OnFileChange("testfile").genericTaskAction
 
 	result = action.Needed(ctx)
 	require.NoError(t, result.Error)
@@ -169,7 +169,7 @@ func TestTaskActionGenericFileChange(t *testing.T) {
 
 	// The file did not change
 
-	action = actionBuilder("", runFunc).OnFileChange("testfile").Build()
+	action = newBuilder("", runFunc).OnFileChange("testfile").genericTaskAction
 
 	result = action.Needed(ctx)
 	require.NoError(t, result.Error)
@@ -179,7 +179,7 @@ func TestTaskActionGenericFileChange(t *testing.T) {
 
 	filet.File(t, tmpdir+"/testfile", "content-B")
 
-	action = actionBuilder("", runFunc).OnFileChange("testfile").Build()
+	action = newBuilder("", runFunc).OnFileChange("testfile").genericTaskAction
 
 	result = action.Needed(ctx)
 	require.NoError(t, result.Error)

diff --git a/pkg/tasks/taskapi/task_list.go b/pkg/tasks/taskapi/task_list.go
@@ -35,24 +35,23 @@ func NewTaskFromDefinition(definition interface{}) (task *Task, err error) {
 
 	taskDef := taskDefinitions[taskConfig.name]
 	if taskDef == nil {
-		taskDef = &TaskDefinition{
-			Name:   "Unknown",
-			Parser: parseUnknown,
-		}
+		taskDef = newUnknownTaskDefinition()
 	}
 
 	task = &Task{TaskDefinition: taskDef}
 	err = taskDef.Parser(taskConfig, task)
 	return
 }
 
-func parseUnknown(config *TaskConfig, task *Task) error {
-	builder := actionBuilder("", func(ctx *Context) error {
-		ctx.UI.TaskWarning(fmt.Sprintf("Unknown task: \"%s\"", config.name))
+func newUnknownTaskDefinition() *TaskDefinition {
+	parser := func(config *TaskConfig, task *Task) error {
+		task.AddActionWithBuilder("", func(ctx *Context) error {
+			ctx.UI.TaskWarning(fmt.Sprintf("Unknown task: \"%s\"", config.name))
+			return nil
+		})
 		return nil
-	})
-	task.AddAction(builder.Build())
-	return nil
+	}
+	return &TaskDefinition{Name: "Unknown", Parser: parser}
 }
 
 func GetFeaturesFromTasks(tasks []*Task) features.FeatureSet {