Fix removeEdge test failures
The removeEdge test could fail intermittently with the wrong order. The precondition of a 1->2->3 order wasn't met, because there was no edge from 1->3, so 3->1->2 was also a valid ordering. The other failure was a bookkeeping error, were the recorded order may not match the visited order. What happened in this case was the gateCh was closed by V2, allowing V3 to run which could beat V2 to recording its visit. Now the visit is recorded as part of the vertex walk, and the gate is released as the final operation. The order is deterministic now, so remove the brute-force test loop.
This commit is contained in:
parent
d01b0b0647
commit
bfa6ab4617
100
dag/walk_test.go
100
dag/walk_test.go
|
@ -215,63 +215,63 @@ func TestWalker_newEdge(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestWalker_removeEdge(t *testing.T) {
|
||||
// Run it a bunch of times since it is timing dependent
|
||||
for i := 0; i < 50; i++ {
|
||||
var g AcyclicGraph
|
||||
g.Add(1)
|
||||
g.Add(2)
|
||||
g.Add(3)
|
||||
g.Connect(BasicEdge(1, 2))
|
||||
g.Connect(BasicEdge(3, 2))
|
||||
var g AcyclicGraph
|
||||
g.Add(1)
|
||||
g.Add(2)
|
||||
g.Add(3)
|
||||
g.Connect(BasicEdge(1, 2))
|
||||
g.Connect(BasicEdge(1, 3))
|
||||
g.Connect(BasicEdge(3, 2))
|
||||
|
||||
// Record function
|
||||
var order []interface{}
|
||||
recordF := walkCbRecord(&order)
|
||||
// Record function
|
||||
var order []interface{}
|
||||
recordF := walkCbRecord(&order)
|
||||
|
||||
// The way this works is that our original graph forces
|
||||
// the order of 1 => 3 => 2. During the execution of 1, we
|
||||
// remove the edge forcing 3 before 2. Then, during the execution
|
||||
// of 3, we wait on a channel that is only closed by 2, implicitly
|
||||
// forcing 2 before 3 via the callback (and not the graph). If
|
||||
// 2 cannot execute before 3 (edge removal is non-functional), then
|
||||
// this test will timeout.
|
||||
var w *Walker
|
||||
gateCh := make(chan struct{})
|
||||
cb := func(v Vertex) error {
|
||||
if v == 1 {
|
||||
g.RemoveEdge(BasicEdge(3, 2))
|
||||
w.Update(&g)
|
||||
// The way this works is that our original graph forces
|
||||
// the order of 1 => 3 => 2. During the execution of 1, we
|
||||
// remove the edge forcing 3 before 2. Then, during the execution
|
||||
// of 3, we wait on a channel that is only closed by 2, implicitly
|
||||
// forcing 2 before 3 via the callback (and not the graph). If
|
||||
// 2 cannot execute before 3 (edge removal is non-functional), then
|
||||
// this test will timeout.
|
||||
var w *Walker
|
||||
gateCh := make(chan struct{})
|
||||
cb := func(v Vertex) error {
|
||||
switch v {
|
||||
case 1:
|
||||
g.RemoveEdge(BasicEdge(3, 2))
|
||||
w.Update(&g)
|
||||
|
||||
case 2:
|
||||
// this visit isn't completed until we've recorded it
|
||||
// Once the visit is official, we can then close the gate to
|
||||
// let 3 continue.
|
||||
defer close(gateCh)
|
||||
|
||||
case 3:
|
||||
select {
|
||||
case <-gateCh:
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
return fmt.Errorf("timeout 3 waiting for 2")
|
||||
}
|
||||
|
||||
if v == 2 {
|
||||
close(gateCh)
|
||||
}
|
||||
|
||||
if v == 3 {
|
||||
select {
|
||||
case <-gateCh:
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
return fmt.Errorf("timeout 3 waiting for 2")
|
||||
}
|
||||
}
|
||||
|
||||
return recordF(v)
|
||||
}
|
||||
|
||||
// Add the initial vertices
|
||||
w = &Walker{Callback: cb}
|
||||
w.Update(&g)
|
||||
return recordF(v)
|
||||
}
|
||||
|
||||
// Wait
|
||||
if err := w.Wait(); err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
// Add the initial vertices
|
||||
w = &Walker{Callback: cb}
|
||||
w.Update(&g)
|
||||
|
||||
// Check
|
||||
expected := []interface{}{1, 2, 3}
|
||||
if !reflect.DeepEqual(order, expected) {
|
||||
t.Fatalf("bad: %#v", order)
|
||||
}
|
||||
// Wait
|
||||
if err := w.Wait(); err != nil {
|
||||
t.Fatalf("err: %s", err)
|
||||
}
|
||||
|
||||
// Check
|
||||
expected := []interface{}{1, 2, 3}
|
||||
if !reflect.DeepEqual(order, expected) {
|
||||
t.Fatalf("bad: %#v", order)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue