----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/#review177509 -----------------------------------------------------------
Thanks for adding tests! src/tests/hierarchical_allocator_tests.cpp Lines 4598 (patched) <https://reviews.apache.org/r/58510/#comment251104> "descendant roles"? Ditto for the test name src/tests/hierarchical_allocator_tests.cpp Lines 4606-4607 (patched) <https://reviews.apache.org/r/58510/#comment251105> I was a bit confused about why this test was using quota, then I figured out that this is how you wanted to ensure that the allocation goes to the descendant. We should document this at the top of the test, or here, to describe that we leverage quota to test this. Simpler to understand would be to not use quota and add two agents, expecting one agent to have been allocated to the ancestor and descendant, each. Does that not work? src/tests/reservation_tests.cpp Lines 2399-2401 (patched) <https://reviews.apache.org/r/58510/#comment251106> These two tests are very similar, the only difference seems to be that one tests that the reservations are allocated using "fair sharing" between the roles (or at least if you removed quota, that would be the case), and the other just tests that it is allocatable to the role? That seems like a reasonable split to ensure that there is a ReservationTest covering this, but it's only accurate if you remove the quota from the first test (otherwise these tests are essentially the same?) - Benjamin Mahler On April 18, 2017, 3:45 p.m., Jay Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58510/ > ----------------------------------------------------------- > > (Updated April 18, 2017, 3:45 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-7149 > https://issues.apache.org/jira/browse/MESOS-7149 > > > Repository: mesos > > > Description > ------- > > Added two tests for hierarchical reservation. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad > > > Diff: https://reviews.apache.org/r/58510/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jay Guo > >
