----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46140/#review131715 -----------------------------------------------------------
Fix it, then Ship it! I'll make the change for you for this patch. Please update the subsequent patches according to the comments in this patch. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 26) <https://reviews.apache.org/r/46140/#comment195747> This is not needed as the test source if wrapped in OS_LINUX in the makefile. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 51) <https://reviews.apache.org/r/46140/#comment195746> We avoid using 'using namespace' now. Please include them explicitly. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 83) <https://reviews.apache.org/r/46140/#comment195748> Ditto on removing it. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 243) <https://reviews.apache.org/r/46140/#comment195749> indentation. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 259) <https://reviews.apache.org/r/46140/#comment195750> offers->size() src/tests/containerizer/docker_volume_isolator_tests.cpp (line 263) <https://reviews.apache.org/r/46140/#comment195751> Is this used? src/tests/containerizer/docker_volume_isolator_tests.cpp (lines 266 - 268) <https://reviews.apache.org/r/46140/#comment195754> Why we need option here. I would simply not use it since we don't check that anyway. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 289) <https://reviews.apache.org/r/46140/#comment195755> you should use && src/tests/containerizer/docker_volume_isolator_tests.cpp (lines 310 - 311) <https://reviews.apache.org/r/46140/#comment195756> Why not checking driver name as well? src/tests/containerizer/docker_volume_isolator_tests.cpp (line 320) <https://reviews.apache.org/r/46140/#comment195759> Align the args please. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 326) <https://reviews.apache.org/r/46140/#comment195758> Align the args please. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 332) <https://reviews.apache.org/r/46140/#comment195757> why not offer.id()? src/tests/containerizer/docker_volume_isolator_tests.cpp (lines 334 - 339) <https://reviews.apache.org/r/46140/#comment195763> The expectation should be placed before driver.launchTasks. Otherwise, it's possible that the expectation is not setup while the event has already been triggered. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 342) <https://reviews.apache.org/r/46140/#comment195764> You can replace .get().xxx with ->xxx src/tests/containerizer/docker_volume_isolator_tests.cpp (line 349) <https://reviews.apache.org/r/46140/#comment195765> Ditto on .get(). Please fix all of them. src/tests/containerizer/docker_volume_isolator_tests.cpp (line 366) <https://reviews.apache.org/r/46140/#comment195766> Kill this line. - Jie Yu On May 4, 2016, 8:54 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46140/ > ----------------------------------------------------------- > > (Updated May 4, 2016, 8:54 a.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-5266 > https://issues.apache.org/jira/browse/MESOS-5266 > > > Repository: mesos > > > Description > ------- > > Added test "ROOT_CommandTaskNoRootfsWithVolumes". > > > Diffs > ----- > > src/Makefile.am 2ba82a78975f2f54bee8f93bc04b7816ce0887c1 > src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp > 070d52018e82ed3e46fb1b79714ffc4716f6a306 > src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46140/diff/ > > > Testing > ------- > > make > make check > GLOG_v=1 ./bin/mesos-tests.sh > --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithVolumes" > --verbose > > > Thanks, > > Guangya Liu > >
