veyron/runtimes/google/ipc: Reduce test flakiness.
Some tests were setup to always call ipc.Client.CloseSend and check
for the error. However, when there are no streaming arguments, it is possible
that the server closes the stream.Flow that is used for the RPC before the CloseSend
message is sent, and thus CloseSend will return an error.
I suspect that this "race" is the cause for much of the flakiness reported in:
https://code.google.com/p/envyor/issues/detail?id=257
Thanks to jsimsa@ for planting the idea for this race being possible
(https://veyron-review.googlesource.com/#/c/3934/)
Change-Id: I0446d48195d1bd9d8152861ab491d8c4cd40bcc4
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index 5ea1d3f..2d7a6a0 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -565,7 +565,17 @@
}
if shouldCloseSend {
vlog.VI(1).Infof("%s call.CloseSend", name(test))
- if err := call.CloseSend(); err != nil {
+ // When the method does not involve streaming
+ // arguments, the server gets all the arguments in
+ // StartCall and then sends a response without
+ // (unnecessarily) waiting for a CloseSend message from
+ // the client. If the server responds before the
+ // CloseSend call is made at the client, the CloseSend
+ // call will fail. Thus, only check for errors on
+ // CloseSend if there are streaming arguments to begin
+ // with (i.e., only if the server is expected to wait
+ // for the CloseSend notification).
+ if err := call.CloseSend(); err != nil && len(test.streamArgs) > 0 {
t.Errorf(`%s call.CloseSend got unexpected error "%v"`, name(test), err)
}
}
@@ -732,9 +742,6 @@
t.Errorf(`%s client.StartCall got unexpected error: "%v"`, name(test), err)
continue
}
- if err := call.CloseSend(); err != nil {
- t.Errorf(`%s call.CloseSend got unexpected error: "%v"`, name(test), err)
- }
results := makeResultPtrs(test.results)
err = call.Finish(results...)
if !matchesErrorPattern(err, test.finishErr) {
