----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36314/#review91162 -----------------------------------------------------------
Ship it! LGTM! Some nits. src/tests/slave_tests.cpp (line 2258) <https://reviews.apache.org/r/36314/#comment144450> I don't think you need to use the MockExecutor here. src/tests/slave_tests.cpp (line 2262) <https://reviews.apache.org/r/36314/#comment144451> No need to use the temp variable. I would suggest the following: ``` slave::Flags flags = CreateSlaveFlage(); flags.resources = "cpus:2;mem:1024;disk:1024;ports:[31000-32000]"; ... AWAIT_READY(usage); EXPECT_EQ(Resources(usage.get().total()), Resources::parse(flags.resources).get()); ``` src/tests/slave_tests.cpp (line 2280) <https://reviews.apache.org/r/36314/#comment144452> This check seems to be redundent. I would suggest killing it. src/tests/slave_tests.cpp (line 2301) <https://reviews.apache.org/r/36314/#comment144459> Same. No need for MockExecutor. src/tests/slave_tests.cpp (lines 2305 - 2307) <https://reviews.apache.org/r/36314/#comment144460> No need for this temp variable. - Jie Yu On July 9, 2015, 1:09 p.m., Bartek Plotka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36314/ > ----------------------------------------------------------- > > (Updated July 9, 2015, 1:09 p.m.) > > > Review request for mesos, Jie Yu, Niklas Nielsen, Szymon Konefal, and Vinod > Kone. > > > Repository: mesos > > > Description > ------- > > Follow up tests for https://reviews.apache.org/r/36204/ > > > Diffs > ----- > > src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 > > Diff: https://reviews.apache.org/r/36314/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Bartek Plotka > >