diff --git a/cyclic.go b/cyclic.go index f7c9387d..1fb395ae 100644 --- a/cyclic.go +++ b/cyclic.go @@ -1,29 +1,34 @@ package task -// HasCyclicDep checks if a task tree has any cyclic dependency -func (e *Executor) HasCyclicDep() bool { +// CheckCyclicDep checks if a task tree has any cyclic dependency +func (e *Executor) CheckCyclicDep() error { visits := make(map[string]struct{}, len(e.Tasks)) - var checkCyclicDep func(string, *Task) bool - checkCyclicDep = func(name string, t *Task) bool { + var checkCyclicDep func(string, *Task) error + checkCyclicDep = func(name string, t *Task) error { if _, ok := visits[name]; ok { - return false + return ErrCyclicDepDetected } visits[name] = struct{}{} defer delete(visits, name) for _, d := range t.Deps { - if !checkCyclicDep(d.Task, e.Tasks[d.Task]) { - return false + // FIXME: ignoring by now. should return an error instead? + task, ok := e.Tasks[d.Task] + if !ok { + continue + } + if err := checkCyclicDep(d.Task, task); err != nil { + return err } } - return true + return nil } for k, v := range e.Tasks { - if !checkCyclicDep(k, v) { - return true + if err := checkCyclicDep(k, v); err != nil { + return err } } - return false + return nil } diff --git a/cyclic_test.go b/cyclic_test.go index c0c00c61..602378cd 100644 --- a/cyclic_test.go +++ b/cyclic_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/go-task/task" + + "github.com/stretchr/testify/assert" ) func TestCyclicDepCheck(t *testing.T) { @@ -18,9 +20,7 @@ func TestCyclicDepCheck(t *testing.T) { }, } - if !isCyclic.HasCyclicDep() { - t.Error("Task should be cyclic") - } + assert.Equal(t, task.ErrCyclicDepDetected, isCyclic.CheckCyclicDep(), "task should be cyclic") isNotCyclic := &task.Executor{ Tasks: task.Tasks{ @@ -34,7 +34,23 @@ func TestCyclicDepCheck(t *testing.T) { }, } - if isNotCyclic.HasCyclicDep() { - t.Error("Task should not be cyclic") + assert.NoError(t, isNotCyclic.CheckCyclicDep()) + + inexixtentTask := &task.Executor{ + Tasks: task.Tasks{ + "task-a": &task.Task{ + Deps: []*task.Dep{&task.Dep{Task: "invalid-task"}}, + }, + }, } + + // FIXME: by now Task should ignore non existent tasks + // in the future we should improve the detection of + // tasks called with interpolation? + // task: + // deps: + // - task: "task{{.VARIABLE}}" + // vars: + // VARIABLE: something + assert.NoError(t, inexixtentTask.CheckCyclicDep()) } diff --git a/errors.go b/errors.go index bb207674..2325d2ed 100644 --- a/errors.go +++ b/errors.go @@ -6,8 +6,8 @@ import ( ) var ( - // ErrCyclicDependencyDetected is returned when a cyclic dependency was found in the Taskfile - ErrCyclicDependencyDetected = errors.New("task: cyclic dependency detected") + // ErrCyclicDepDetected is returned when a cyclic dependency was found in the Taskfile + ErrCyclicDepDetected = errors.New("task: cyclic dependency detected") // ErrTaskfileAlreadyExists is returned on creating a Taskfile if one already exists ErrTaskfileAlreadyExists = errors.New("task: A Taskfile already exists") ) diff --git a/task.go b/task.go index d9d7c003..b3b8702a 100644 --- a/task.go +++ b/task.go @@ -61,8 +61,8 @@ type Task struct { // Run runs Task func (e *Executor) Run(args ...string) error { - if e.HasCyclicDep() { - return ErrCyclicDependencyDetected + if err := e.CheckCyclicDep(); err != nil { + return err } if e.Stdin == nil {