-----------------------------------------------------------
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
> 
>

Reply via email to