> On Oct. 19, 2015, 11:22 p.m., Niklas Nielsen wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp, line 504 > > <https://reviews.apache.org/r/39388/diff/2/?file=1100524#file1100524line504> > > > > Mind adding a comment about the test sequence? Why does the 'test > > $LIBPROCESS_IP = \"foobar"\' work, etc.
Yep, will do. > On Oct. 19, 2015, 11:22 p.m., Niklas Nielsen wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp, lines 509-510 > > <https://reviews.apache.org/r/39388/diff/2/?file=1100524#file1100524line509> > > > > 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. Probably, I'll look into it a bit more. I followed the pattern of existing tests so I'll perhaps update those correspondingly. > On Oct. 19, 2015, 11:22 p.m., Niklas Nielsen wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp, line 518 > > <https://reviews.apache.org/r/39388/diff/2/?file=1100524#file1100524line518> > > > > Can we make this assumption (do we have other places where we assume > > localhost is always 127.0.0.1)? The choice of `127.0.0.1` is arbitrary here. We could've assigned it to `x`, and performed `test $LIBPROCESS_IP = x` instead. Actually, I'll pull it out to a variable and leave a comment in line with what I described here. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39388/#review103182 ----------------------------------------------------------- On Oct. 17, 2015, 11: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, 11: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 > >
