----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25270/#review53531 -----------------------------------------------------------
Ship it! src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/25270/#comment93221> Awesome test! Can we add a comment above that explains in text how this differs from the other bridged test? In particular, it would be nice for people to understand that you have two tests here, both testing bridged networking, one which tests for an executor, the other which tests for a task and also makes sure the port mapping works. Thanks Tim! src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/25270/#comment93227> Let's move this down to where it's actually used. ;-) src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/25270/#comment93218> Separating these stanzas would make them easier to read (as we do when setting up other protobufs). src/tests/docker_tests.cpp <https://reviews.apache.org/r/25270/#comment93225> Just a random question, what happens if /mnt/mesos/sandbox doesn't exist? src/tests/docker_tests.cpp <https://reviews.apache.org/r/25270/#comment93223> Please split these up like above! src/tests/docker_tests.cpp <https://reviews.apache.org/r/25270/#comment93224> Please use AWAIT_EXPECT_FAILED, otherwise when this code actually becomes asynchronous this test is going to start non-deterministically failing. - Benjamin Hindman On Sept. 11, 2014, 5:48 p.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25270/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2014, 5:48 p.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy St. Clair. > > > Bugs: MESOS-1621 > https://issues.apache.org/jira/browse/MESOS-1621 > > > Repository: mesos-git > > > Description > ------- > > Review: https://reviews.apache.org/r/25270 > > > Diffs > ----- > > include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 > src/docker/docker.cpp af51ac9058382aede61b09e06e312ad2ce6de03e > src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 > src/tests/docker_containerizer_tests.cpp > 8654f9c787bd207f6a7b821651e0c083bea9dc8a > src/tests/docker_tests.cpp 826a8c1ef1b3089d416e5775fa2cf4e5cb0c26d1 > > Diff: https://reviews.apache.org/r/25270/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Timothy Chen > >
