Re: Review Request 40995: Added test cases for role behavior.

2015-12-17 Thread Neil Conway
> On Dec. 18, 2015, 12:57 a.m., Adam B wrote: > > src/tests/role_tests.cpp, lines 79-83 > > > > > > `AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", > > response);` all fits on one line, and does

Re: Review Request 40995: Added test cases for role behavior.

2015-12-17 Thread Adam B
> On Dec. 17, 2015, 4:57 p.m., Adam B wrote: > > src/tests/role_tests.cpp, lines 79-83 > > > > > > `AWAIT_EXPECT_RESPONSE_HEADER_EQ(APPLICATION_JSON, "Content-Type", > > response);` all fits on one line, and does

Re: Review Request 40995: Added test cases for role behavior.

2015-12-17 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review111089 --- Ship it! Looks great! Just a couple of test macro suggestions

Re: Review Request 40995: Added test cases for role behavior.

2015-12-16 Thread Neil Conway
> On Dec. 15, 2015, 10:18 a.m., Adam B wrote: > > Thanks for validating the expected `--roles` behavior with tests, but I > > think you're mixing up the scheduler reservation/volume API (where only the > > framework's role is valid) with the operator API (where any whitelisted > > role is

Re: Review Request 40995: Added test cases for role behavior.

2015-12-16 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 16, 2015, 9:05 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 40995: Added test cases for role behavior.

2015-12-15 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review110460 --- Thanks for validating the expected `--roles` behavior with tests,

Re: Review Request 40995: Added test cases for role behavior.

2015-12-15 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 15, 2015, 8:56 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 40995: Added test cases for role behavior.

2015-12-14 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 14, 2015, 11:15 p.m.) Review request for mesos, Adam B,

Re: Review Request 40995: Added test cases for role behavior.

2015-12-10 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 10, 2015, 10:08 p.m.) Review request for mesos, Adam B,

Re: Review Request 40995: Added test cases for role behavior.

2015-12-09 Thread Neil Conway
> On Dec. 8, 2015, 2:55 p.m., Alexander Rukletsov wrote: > > src/tests/role_tests.cpp, lines 83-84 > > > > > > `MesosTest` exposes the `defaultAgentResourcesString` constant. I think > > you can get rid of these

Re: Review Request 40995: Added test cases for role behavior.

2015-12-08 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 8, 2015, 8:28 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 40995: Added test cases for role behavior.

2015-12-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review109324 --- One thing we also agreed is to disallow empty string roles. Do you

Re: Review Request 40995: Added test cases for role behavior.

2015-12-08 Thread Alexander Rukletsov
> On Dec. 8, 2015, 12:51 a.m., Guangya Liu wrote: > > src/tests/role_tests.cpp, lines 24-27 > > > > > > I think that we should always use std first? > > > > using std::vector; > > > > using

Re: Review Request 40995: Added test cases for role behavior.

2015-12-08 Thread Neil Conway
> On Dec. 8, 2015, 2:55 p.m., Alexander Rukletsov wrote: > > One thing we also agreed is to disallow empty string roles. Do you think it > > makes sense to extend this patch with a test for that or do it separately? I think we should do this separately, perhaps as part of fixing MESOS-2210.

Re: Review Request 40995: Added test cases for role behavior.

2015-12-08 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 8, 2015, 7:18 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 40995: Added test cases for role behavior.

2015-12-08 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 9, 2015, 5:53 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review109217 --- Ship it! LGTM! One question: Is the positive case (correct role

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Guangya Liu
> On 十二月 8, 2015, 12:51 a.m., Guangya Liu wrote: > > src/tests/role_tests.cpp, lines 24-27 > > > > > > I think that we should always use std first? > > > > using std::vector; > > > > using

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Guangya Liu
> On 十二月 8, 2015, 12:51 a.m., Guangya Liu wrote: > > src/tests/role_tests.cpp, lines 24-27 > > > > > > I think that we should always use std first? > > > > using std::vector; > > > > using

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Neil Conway
> On Dec. 8, 2015, 12:51 a.m., Guangya Liu wrote: > > src/tests/role_tests.cpp, lines 24-27 > > > > > > I think that we should always use std first? > > > > using std::vector; > > > > using

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Neil Conway
> On Dec. 8, 2015, 12:17 a.m., Joseph Wu wrote: > > LGTM! > > > > One question: Is the positive case (correct role -> successful operation) > > not interesting or just tested in other files? There are tests for successful cases in other files. Maybe not exhaustive though... - Neil

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review109234 --- src/tests/role_tests.cpp (lines 24 - 27)

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Neil Conway
> On Dec. 8, 2015, 12:51 a.m., Guangya Liu wrote: > > src/tests/role_tests.cpp, lines 24-27 > > > > > > I think that we should always use std first? > > > > using std::vector; > > > > using

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Yong Qiao Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review109278 --- Ship it! Ship It! - Yong Qiao Wang On Dec. 7, 2015, 9:22 p.m.,

Re: Review Request 40995: Added test cases for role behavior.

2015-12-07 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- (Updated Dec. 8, 2015, 5:28 a.m.) Review request for mesos, Adam B, Alexander

Review Request 40995: Added test cases for role behavior.

2015-12-04 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/ --- Review request for mesos, Adam B and Greg Mann. Repository: mesos

Re: Review Request 40995: Added test cases for role behavior.

2015-12-04 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review109077 --- Patch looks great! Reviews applied: [40995] Passed command: