> On Jan. 21, 2016, 7:17 a.m., Ben Mahler wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 449-451 > > <https://reviews.apache.org/r/42355/diff/2/?file=1202267#file1202267line449> > > > > Why mention expiration between two consecutive allocations here? The > > way I had been thinking about a test here was that we ensure that a filter > > always expires after the next batch allocation. > > > > So, is the fact that there are two batch allocations here is just an > > implementation detail as far as testing is concerned? > > > > Would you mind referencing MESOS-4302 so that the next person to look > > at this test can follow the history a little more easily?
I always try to show the reader why we actually cared and wrote the test. I believe a breif description of the problem helps the reader to better understand motivation behind the chosen behaviour. However I agree that such background information should not substitute the exact description. I'll try to rephrase. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42355/#review115583 ----------------------------------------------------------- On Jan. 19, 2016, 11:32 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42355/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2016, 11:32 p.m.) > > > Review request for mesos, Ben Mahler and Joris Van Remoortere. > > > Bugs: MESOS-4302 > https://issues.apache.org/jira/browse/MESOS-4302 > > > Repository: mesos > > > Description > ------- > > Without the timeout, we rely on filter expiration only. This guarantees > that filter removal is scheduled after `allocate()` if the allocator is > backlogged given default parameters are used. Additionally we ensure the > filter timeout is at least as big as the allocation interval. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 48acde69b1a2f305b568a7e322a58708063dd30a > src/tests/hierarchical_allocator_tests.cpp > 9362dd306497ba01e0f387c3862456cdcac6f863 > > Diff: https://reviews.apache.org/r/42355/diff/ > > > Testing > ------- > > On Mac OS 10.10.4: > > `make check` > > `GTEST_FILTER="HierarchicalAllocatorTest.FilterTimeout" ./bin/mesos-tests.sh > --gtest_repeat=100 --gtest_break_on_failure` passes with the patch and fails > without. > > `GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh > --gtest_repeat=100 --gtest_break_on_failure` > > > Thanks, > > Alexander Rukletsov > >
