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

2016-03-03 Thread Greg Mann 

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

2016-03-02 Thread Vinod Kone 

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


// Send a TEARDOWN call without stream ID.



src/tests/scheduler_http_api_tests.cpp (line 1039)


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.

2016-03-02 Thread Greg Mann 

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

2016-03-02 Thread Anand Mazumdar 

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


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)


s/Bad Request/BadRequest

Here and the other 2 more occurences please.



src/tests/scheduler_http_api_tests.cpp (line 852)


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)


s/teardownCall/call

Can we also scope this?



src/tests/scheduler_http_api_tests.cpp (line 922)


Can we move this before L938 i.e. in the next scope?



src/tests/scheduler_http_api_tests.cpp (line 973)


Can we define another `Call` here explicitly and not re-use the old 
instance?



src/tests/scheduler_http_api_tests.cpp (line 1010)


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

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

2016-03-02 Thread Greg Mann 

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