----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53511/#review155388 -----------------------------------------------------------
Fix it, then Ship it! Very clean update to the test! I left a comment about whether we can do this similar parameterization on the response side, perhaps you'll want to leave a TODO for this if you don't want to take it on now? 3rdparty/libprocess/src/tests/decoder_tests.cpp (lines 43 - 44) <https://reviews.apache.org/r/53511/#comment225273> Seems a little odd to have DecoderTest as a request specific typed test, and then DecoderTest is also still the name for the response tests. Wouldn't this break if we made the same cleanup for the response decoder tests? How about s/Decoder/RequestDecoder/ for these names? Looks like you want to add a TODO to do this similar parameterization for the response tests? Or is there a reason we can't? 3rdparty/libprocess/src/tests/decoder_tests.cpp (lines 77 - 83) <https://reviews.apache.org/r/53511/#comment225284> Let's invoke the function immediately so that it can only be called once: ``` Future<string> body = [&request]() { ... }(); ``` Or if you want the function, how about avoiding the capture since this function has no need to be tied to a specific request? ``` auto getBody = [](Request* request) { ... }; AWAIT_EXPECT_EQ(string(), getBody(request)); ``` Although in this case, calling getBody twice would be problematic in the case of a pipe body, so the first option seems better to me, unless we needed to call this for different requests. - Benjamin Mahler On Nov. 5, 2016, 1:32 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53511/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2016, 1:32 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-6466 > https://issues.apache.org/jira/browse/MESOS-6466 > > > Repository: mesos > > > Description > ------- > > This allows us to not duplicate tests for the streaming request > decoder. > > > Diffs > ----- > > 3rdparty/libprocess/src/tests/decoder_tests.cpp > 4535614312373b0515025f09f9f8495f9ce353a3 > > Diff: https://reviews.apache.org/r/53511/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > >
