----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44290/#review121721 -----------------------------------------------------------
Looks good. Just some minor cleanup comments. src/tests/scheduler_http_api_tests.cpp (line 290) <https://reviews.apache.org/r/44290/#comment183519> We should also explicitly check that the "Mesos-Stream-Id" header is not equal to "" in addition to it being present. src/tests/scheduler_http_api_tests.cpp (line 329) <https://reviews.apache.org/r/44290/#comment183509> s/Bad Request/BadRequest Here and the other 2 more occurences please. src/tests/scheduler_http_api_tests.cpp (line 852) <https://reviews.apache.org/r/44290/#comment183511> s/subscribeCall/call Also can you scope all the calls? { Call call; call.set_type(Call::SUBSCRIBE); ... } { Call call; call.set_type(Call::TEARDOWN); } src/tests/scheduler_http_api_tests.cpp (line 894) <https://reviews.apache.org/r/44290/#comment183521> s/teardownCall/call Can we also scope this? src/tests/scheduler_http_api_tests.cpp (line 922) <https://reviews.apache.org/r/44290/#comment183515> Can we move this before L938 i.e. in the next scope? src/tests/scheduler_http_api_tests.cpp (line 973) <https://reviews.apache.org/r/44290/#comment183516> Can we define another `Call` here explicitly and not re-use the old instance? src/tests/scheduler_http_api_tests.cpp (line 1010) <https://reviews.apache.org/r/44290/#comment183518> s/teardownCall/call - Anand Mazumdar On March 2, 2016, 8:59 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44290/ > ----------------------------------------------------------- > > (Updated March 2, 2016, 8:59 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-3583 > https://issues.apache.org/jira/browse/MESOS-3583 > > > Repository: mesos > > > Description > ------- > > Added tests involving HTTP scheduler stream IDs. > > Three new tests have been added in this patch: > SchedulerHttpApiTest.TeardownWithoutStreamId, > SchedulerHttpApiTest.TeardownWrongStreamId, and > SchedulerHttpApiTest.SubscribeWithStreamId. > > > Diffs > ----- > > src/tests/scheduler_http_api_tests.cpp > 428e12646d80b45daec30cfe607b97f36170fdf5 > > Diff: https://reviews.apache.org/r/44290/diff/ > > > Testing > ------- > > `make check` was used to test on both OSX and CentOS 7.1 > > The new tests were run 1000 times to look for flakiness; no failures were > observed. > > > Thanks, > > Greg Mann > >
