----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55967/#review163632 -----------------------------------------------------------
Fix it, then Ship it! src/tests/hierarchical_allocator_tests.cpp (lines 944 - 945) <https://reviews.apache.org/r/55967/#comment235102> This seems like an odd test. Unforunate that we can't seem to use `Allocation` because of this. src/tests/hierarchical_allocator_tests.cpp (lines 3408 - 3411) <https://reviews.apache.org/r/55967/#comment235101> Should just use `foreachpair` twice here I think. A few places below as well. src/tests/hierarchical_allocator_tests.cpp (lines 3728 - 3730) <https://reviews.apache.org/r/55967/#comment235103> Operators should be placed in the previous lines I think? I think maybe it'd be better if we were to just pull out the `allocatedTaskResources`? ```cpp AllocatedResources allocatedTaskResources(task.resources(), "role1"); expected = Allocation::create( framework2.id(), {{"role1", {{slave.id(), updated.get() - allocatedTaskResources + volume}}}}); ``` src/tests/hierarchical_allocator_tests.cpp (lines 3810 - 3813) <https://reviews.apache.org/r/55967/#comment235104> `s/allocation/allocated/` since we have an `Allocation` type now? Also, could you have used `AllocatedResources` here? src/tests/hierarchical_allocator_tests.cpp (line 4180) <https://reviews.apache.org/r/55967/#comment235106> `{` on the newline. - Michael Park On Jan. 25, 2017, 4:46 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55967/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2017, 4:46 p.m.) > > > Review request for mesos, Guangya Liu and Michael Park. > > > Repository: mesos > > > Description > ------- > > Update the allocator unit tests to reflect MULTI_ROLE support. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 1edd0ecc8a93cd41532e1cf3641f67c780ab23a5 > > Diff: https://reviews.apache.org/r/55967/diff/ > > > Testing > ------- > > make check > > note that the rest of the tests do not pass until after my subsequent patches > > > Thanks, > > Benjamin Mahler > >
