-----------------------------------------------------------
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
> 
>

Reply via email to