----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49616/#review141095 -----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (lines 3572 - 3575) <https://reviews.apache.org/r/49616/#comment206488> We should use the templatized test fixture. s/TEST_F/TEST_P/ and use `GetParam()`. src/tests/hierarchical_allocator_tests.cpp (line 3587) <https://reviews.apache.org/r/49616/#comment206502> We actually don't need this count and it's misleading: it's not the count of offers. It's the count of frameworks that are receiving offers in this allocation round. To get the offer count we can use `offers.size()`. src/tests/hierarchical_allocator_tests.cpp (lines 3609 - 3616) <https://reviews.apache.org/r/49616/#comment206506> We can simply: ``` cout << "Using " << slaveCount << " agents and " << frameworkCount << " frameworks" << endl; vector<SlaveInfo> slaves(slaveCount); vector<FrameworkInfo> frameworks(frameworkCount); ``` src/tests/hierarchical_allocator_tests.cpp (line 3648) <https://reviews.apache.org/r/49616/#comment206492> Nit: `s/count/i/`, we don't seem to use `count` in any special way and `i` is the standard variable name in a for loop. src/tests/hierarchical_allocator_tests.cpp (lines 3650 - 3651) <https://reviews.apache.org/r/49616/#comment206494> Comment: // Recover resources with no filters because we want to test the effect of suppression alone. src/tests/hierarchical_allocator_tests.cpp (line 3654) <https://reviews.apache.org/r/49616/#comment206495> Comment: // Wait for all declined offers to be processed so the stopwatch only measures the allocation time. src/tests/hierarchical_allocator_tests.cpp (line 3660) <https://reviews.apache.org/r/49616/#comment206496> This `{` shouldn't be necessary. src/tests/hierarchical_allocator_tests.cpp (line 3669) <https://reviews.apache.org/r/49616/#comment206499> Instead of calling it a `round` we could be more explicit: "allocate() took 1.491969secs to make 2000 offers with 103 out of 200 frameworks suppressing offers" - Jiang Yan Xu On July 5, 2016, 2:48 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49616/ > ----------------------------------------------------------- > > (Updated July 5, 2016, 2:48 p.m.) > > > Review request for mesos, Joris Van Remoortere and Jiang Yan Xu. > > > Bugs: MESOS-5781 > https://issues.apache.org/jira/browse/MESOS-5781 > > > Repository: mesos > > > Description > ------- > > - Useful for high framework clusters which carry > many suppressed frameworks that are considered > during allocation. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 0498cd5e54b0e4b87a767585a77699653aa52179 > > Diff: https://reviews.apache.org/r/49616/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jacob Janco > >
