You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/03/02 21:59:42 UTC

Review Request 44290: Added tests involving HTTP scheduler stream IDs.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44290/
-----------------------------------------------------------

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

Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44290/
-----------------------------------------------------------

(Updated March 3, 2016, 5:15 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

Addressed comments.


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 (updated)
-----

  src/tests/scheduler_http_api_tests.cpp a8d660a0fbebf29f865dfbfb07eceb0ce7cdd39f 

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

Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44290/#review121764
-----------------------------------------------------------


Fix it, then Ship it!





src/tests/scheduler_http_api_tests.cpp (line 899)
<https://reviews.apache.org/r/44290/#comment183582>

    // Send a TEARDOWN call without stream ID.



src/tests/scheduler_http_api_tests.cpp (line 1039)
<https://reviews.apache.org/r/44290/#comment183583>

    nice test!


- Vinod Kone


On March 2, 2016, 10:20 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, 10:20 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
> 
>

Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44290/
-----------------------------------------------------------

(Updated March 2, 2016, 10:20 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

Addressed comments.


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 (updated)
-----

  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

Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
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
> 
>