> On June 10, 2017, 3:22 a.m., Benjamin Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp > > Lines 4606-4607 (patched) > > <https://reviews.apache.org/r/58510/diff/1/?file=1693724#file1693724line4606> > > > > 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?
Since we made changes for both quota and fair share stage of allocation, I'm using quota to make sure we cover both logic paths. I updated the test comments, please take a look. > On June 10, 2017, 3:22 a.m., Benjamin Mahler wrote: > > src/tests/reservation_tests.cpp > > Lines 2399-2401 (patched) > > <https://reviews.apache.org/r/58510/diff/1/?file=1693725#file1693725line2399> > > > > 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?) Dropped the test. - Jay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58510/#review177509 ----------------------------------------------------------- On June 13, 2017, 11:40 p.m., Jay Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58510/ > ----------------------------------------------------------- > > (Updated June 13, 2017, 11:40 p.m.) > > > Review request for mesos and Michael Park. > > > Bugs: MESOS-7149 > https://issues.apache.org/jira/browse/MESOS-7149 > > > Repository: mesos > > > Description > ------- > > Added a test for hierarchical reservation. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 631e800e6566847e015ea2638cc1c2fe673855be > > > Diff: https://reviews.apache.org/r/58510/diff/2/ > > > Testing > ------- > > make check > > comment out > https://github.com/apache/mesos/blob/master/src/common/roles.cpp#L71-L74 and > `./bin/mesos-tests.sh > --gtest_filter="HierarchicalAllocatorTest.DISABLED_OfferAncestorReservationsToDescendantChild" > --gtest_also_run_disabled_tests` > > > Thanks, > > Jay Guo > >
