diff --git a/depgraph/graph.go b/depgraph/graph.go index 62b39572f..4d66a9397 100644 --- a/depgraph/graph.go +++ b/depgraph/graph.go @@ -259,6 +259,10 @@ func (g *Graph) Walk(fn WalkFunc) error { seenMap := make(map[*Noun]chan struct{}) seenMap[g.Root] = make(chan struct{}) + // Keep track of what nodes errored. + var errMapL sync.RWMutex + errMap := make(map[*Noun]struct{}) + // Build the list of things to visit tovisit := make([]*Noun, 1, len(g.Nouns)) tovisit[0] = g.Root @@ -302,10 +306,23 @@ func (g *Graph) Walk(fn WalkFunc) error { case <-quitCh: return } + + // Check if any dependencies errored. If so, + // then return right away, we won't walk it. + errMapL.RLock() + _, errOk := errMap[dep.Target] + errMapL.RUnlock() + if errOk { + return + } } // Call our callback! if err := fn(current); err != nil { + errMapL.Lock() + errMap[current] = struct{}{} + errMapL.Unlock() + errCh <- err } }(current) diff --git a/depgraph/graph_test.go b/depgraph/graph_test.go index 9b943c8df..6e38b885d 100644 --- a/depgraph/graph_test.go +++ b/depgraph/graph_test.go @@ -388,21 +388,40 @@ c -> e`) func TestGraphWalk_error(t *testing.T) { nodes := ParseNouns(`a -> b -a -> c -b -> d -b -> e -c -> d -c -> e`) +b -> c +a -> d`) list := NounMapToList(nodes) g := &Graph{Name: "Test", Nouns: list} if err := g.Validate(); err != nil { t.Fatalf("err: %s", err) } - err := g.Walk(func(n *Noun) error { - return fmt.Errorf("foo") - }) - if err == nil { - t.Fatal("should error") + // We repeat this a lot because sometimes timing causes + // a false positive. + for i := 0; i < 100; i++ { + var lock sync.Mutex + var walked []string + err := g.Walk(func(n *Noun) error { + lock.Lock() + defer lock.Unlock() + + walked = append(walked, n.Name) + + if n.Name == "b" { + return fmt.Errorf("foo") + } + + return nil + }) + if err == nil { + t.Fatal("should error") + } + + sort.Strings(walked) + + expected := []string{"b", "c", "d"} + if !reflect.DeepEqual(walked, expected) { + t.Fatalf("bad: %#v", walked) + } } }