Merge "veyron/services/mgmt/node/impl: fix flakiness in testNodeManager"
diff --git a/services/mgmt/node/impl/node_service.go b/services/mgmt/node/impl/node_service.go
index 6d0f12d..ebb604f 100644
--- a/services/mgmt/node/impl/node_service.go
+++ b/services/mgmt/node/impl/node_service.go
@@ -238,7 +238,7 @@
if err != nil {
return verror2.Make(ErrOperationFailed, ctx)
}
- // Check that invoking Update() succeeds.
+ // Check that invoking Revert() succeeds.
childName = naming.Join(childName, "nm")
nmClient := node.NodeClient(childName)
linkOld, pathOld, err := i.getCurrentFileInfo()
@@ -258,13 +258,17 @@
}
// Check that the new node manager updated the current symbolic link.
if !linkOld.ModTime().Before(linkNew.ModTime()) {
- vlog.Errorf("new node manager test failed")
+ vlog.Errorf("New node manager test failed")
return verror2.Make(ErrOperationFailed, ctx)
}
// Ensure that the current symbolic link points to the same script.
if pathNew != pathOld {
updateLink(pathOld, i.config.CurrentLink)
- vlog.Errorf("new node manager test failed")
+ vlog.Errorf("New node manager test failed")
+ return verror2.Make(ErrOperationFailed, ctx)
+ }
+ if err := handle.Wait(childWaitTimeout); err != nil {
+ vlog.Errorf("New node manager failed to exit cleanly: %v", err)
return verror2.Make(ErrOperationFailed, ctx)
}
return nil
diff --git a/services/mgmt/node/impl/util.go b/services/mgmt/node/impl/util.go
index c3c2bdc..feeb1c3 100644
--- a/services/mgmt/node/impl/util.go
+++ b/services/mgmt/node/impl/util.go
@@ -19,6 +19,7 @@
// TODO(caprita): Set these timeout in a more principled manner.
const (
childReadyTimeout = 20 * time.Second
+ childWaitTimeout = 20 * time.Second
ipcContextTimeout = time.Minute
)
diff --git a/services/mgmt/node/impl/util_test.go b/services/mgmt/node/impl/util_test.go
index 1c19f37..1d0e119 100644
--- a/services/mgmt/node/impl/util_test.go
+++ b/services/mgmt/node/impl/util_test.go
@@ -171,12 +171,12 @@
return server, endpoint.String()
}
-// resolveExpectError verifies that the given name is not in the mounttable.
+// resolveExpectNotFound verifies that the given name is not in the mounttable.
func resolveExpectNotFound(t *testing.T, name string) {
if results, err := globalRT.Namespace().Resolve(globalRT.NewContext(), name); err == nil {
- t.Fatalf("Resolve(%v) succeeded with results %v when it was expected to fail", name, results)
+ t.Fatalf("%s: Resolve(%v) succeeded with results %v when it was expected to fail", loc(1), name, results)
} else if expectErr := naming.ErrNoSuchName.ID; !verror2.Is(err, expectErr) {
- t.Fatalf("Resolve(%v) failed with error %v, expected error ID %v", name, err, expectErr)
+ t.Fatalf("%s: Resolve(%v) failed with error %v, expected error ID %v", loc(1), name, err, expectErr)
}
}
