----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26862/#review58545 -----------------------------------------------------------
src/slave/containerizer/docker.cpp <https://reviews.apache.org/r/26862/#comment99607> If we're going to take ownership than we need to make that explicit in the constructor please! Also, why a 'const Docker'? src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/26862/#comment99608> Can you please redirect people to the explanation in src/tests/containerizer.cpp which explains why we set up defaults with EXPECT_CALL/WillRepeatedly instead of ON_CALL/WillByDefault. As an example, see MockGarbageCollector() in src/tests/mesos.hpp. Also, did you copy this code from someplace that doesn't have a comment? Because it would be great to get that everywhere so people don't end up making that change (like I once did) only to have to undo it because of gmock annoyances. src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/26862/#comment99609> Hmm, I feel like I ask this question everytime, but can we not just s/MockDocker::_logs/Docker::logs/ here and it does the right thing? src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/26862/#comment99613> This doesn't look like it was used by any call sites. src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/26862/#comment99610> s/> >/>>/ src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/26862/#comment99611> Shouldn't this just be an automatic upcast so the explicit cast isn't necessary? src/tests/docker_containerizer_tests.cpp <https://reviews.apache.org/r/26862/#comment99612> I don't understand, isn't the point of this test to show that the container was at least stopped (since now we're not removing them until after the delay)? The same for the other tests where this was removed too? - Benjamin Hindman On Oct. 22, 2014, 12:46 a.m., Timothy Chen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26862/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 12:46 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos-git > > > Description > ------- > > Docker tests are flaky, mostly around getting expected output from the docker > container forwarded to stdout/stderr. > > This is due to Docker not always have the stdout/stderr output available for > docker logs if kill/rm is called. > > > Diffs > ----- > > src/docker/docker.hpp 9656f15 > src/docker/docker.cpp e09b51c > src/slave/containerizer/docker.hpp fbbd45d > src/slave/containerizer/docker.cpp 9a29489 > src/tests/docker_containerizer_tests.cpp 67d60a8 > src/tests/docker_tests.cpp 04139af > src/tests/environment.cpp 4dd78e7 > > Diff: https://reviews.apache.org/r/26862/diff/ > > > Testing > ------- > > make with gtest_repeat=-1 gtest_shuffle=1 > > > Thanks, > > Timothy Chen > >
