-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39388/#review103182
-----------------------------------------------------------

Ship it!


See comments below :)


src/tests/containerizer/docker_containerizer_tests.cpp (line 504)
<https://reviews.apache.org/r/39388/#comment161157>

    Mind adding a comment about the test sequence? Why does the 'test 
$LIBPROCESS_IP = \"foobar"\' work, etc.



src/tests/containerizer/docker_containerizer_tests.cpp (lines 509 - 510)
<https://reviews.apache.org/r/39388/#comment161155>

    No biggie; but could we have done the creation of the mock docker inline in 
line 512? The first thing that came to my mind was that we were leaking the 
object.



src/tests/containerizer/docker_containerizer_tests.cpp (line 518)
<https://reviews.apache.org/r/39388/#comment161156>

    Can we make this assumption (do we have other places where we assume 
localhost is always 127.0.0.1)?


- Niklas Nielsen


On Oct. 17, 2015, 4:18 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2015, 4:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
>     https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> -------
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Reply via email to