GNOME Bugzilla – Bug 755124
tests/proxy-continuous: Don't assume that SoupMessage:got-chunk will correspond exactly to the chunks written by the server
Last modified: 2015-09-17 12:39:02 UTC
Currently tests/proxy-continuous has a SoupServer that writes chunks like: 1 1 1 1\n 2 2 2 2\n ... And it expects that _call_continuous_cb will receive them in chunks like: 1 1 1 1\n 2 2 2 2\n ... There is no guarantee that this will be the case. It can receive: 1 1 1 1\n 2 2 2 2\n ... Usually this is not an issue, but some people are hitting this problem because I got a downstream report where the test failed due to this. I suggest simplifying the test case to send a constant stream of integers (without the whitespaces), because then it is easier for the client to parse the responses without making any assumptions of about the chunks.
Created attachment 311492 [details] [review] tests/proxy-continuous: Server chunks can be different from client ones
Review of attachment 311492 [details] [review]: Looks good to me, couple of minor comments. (I set commit-now, I actually am not sure what the status of librest is wrt various freezes. I'd say it does not follow GNOME release cycles). ::: tests/proxy-continuous.c @@ +44,3 @@ SoupBuffer *buf; + gint i; + gint8 data[SIZE_CHUNK]; guint i; guint8 data[SIZE_CHUNK]; @@ +56,1 @@ soup_message_body_append_buffer (msg->response_body, buf); soup_message_body_append () would allow to skip the 'buf' temporary buffer (but this is a preexisting issue as the previous code could have used soup_message_body_append_take ()) @@ +89,3 @@ gpointer userdata) { + gint i; preference for guint i; here too
Created attachment 311547 [details] [review] tests/proxy-continuous: Server chunks can be different from client ones
(In reply to Christophe Fergeau from comment #2) > Review of attachment 311492 [details] [review] [review]: > > Thanks for the review Christophe. > > ::: tests/proxy-continuous.c > @@ +44,3 @@ > SoupBuffer *buf; > + gint i; > + gint8 data[SIZE_CHUNK]; > > guint i; > guint8 data[SIZE_CHUNK]; Fixed. I also changed server_count and client_count to be guint8 to avoid any compiler warnings related to assigning / comparing different integer types. > @@ +56,1 @@ > soup_message_body_append_buffer (msg->response_body, buf); > > soup_message_body_append () would allow to skip the 'buf' temporary buffer > (but this is a preexisting issue as the previous code could have used > soup_message_body_append_take ()) I will fix this in a separate patch. > @@ +89,3 @@ > gpointer userdata) > { > + gint i; > > preference for guint i; here too Fixed.
Created attachment 311549 [details] [review] tests/proxy-continuous: Simplify the code
Review of attachment 311547 [details] [review]: Looks good, thanks.
Review of attachment 311549 [details] [review]: ACK.
Thanks for those quick reviews! I have pushed them to master.