> On Jan. 31, 2017, 7:25 a.m., Michael Park wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 3414-3417 > > <https://reviews.apache.org/r/55967/diff/1/?file=1616016#file1616016line3414> > > > > Should just use `foreachpair` twice here I think. A few places below as > > well.
Yeah we could, just seemed a bit cleaner to avoid introducing another resources variable. > On Jan. 31, 2017, 7:25 a.m., Michael Park wrote: > > src/tests/hierarchical_allocator_tests.cpp, lines 3737-3739 > > <https://reviews.apache.org/r/55967/diff/1/?file=1616016#file1616016line3737> > > > > 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}}}}); > > ``` Sounds good! Turns out I don't need to pull it out since now the adjustment of the operation mutates it rather than returning a copy. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55967/#review163632 ----------------------------------------------------------- On Jan. 26, 2017, 12:46 a.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55967/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2017, 12:46 a.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 > >
