----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53385/#review154713 -----------------------------------------------------------
src/tests/containerizer/docker_containerizer_tests.cpp (lines 3915 - 3917) <https://reviews.apache.org/r/53385/#comment224416> Backtick enum values. src/tests/containerizer/docker_containerizer_tests.cpp (line 3921) <https://reviews.apache.org/r/53385/#comment224380> I'm surprised to find out that we don't have `DockerExecutorTest` fixture! src/tests/containerizer/docker_containerizer_tests.cpp (line 3974) <https://reviews.apache.org/r/53385/#comment224418> 15 seconds src/tests/containerizer/docker_containerizer_tests.cpp (lines 4008 - 4011) <https://reviews.apache.org/r/53385/#comment224420> If you use `MockDockerContainerizer`, you should explicitly perform container operations. Here for example, you should launch the task when the request comes in: ``` Future<ContainerID> containerId; EXPECT_CALL(containerizer, launch(_, _, _, _, _, _, _, _)) .WillOnce(DoAll(FutureArg<0>(&containerId), Invoke(&containerizer, &MockDockerContainerizer::_launch))); ``` src/tests/containerizer/docker_containerizer_tests.cpp (lines 4043 - 4045) <https://reviews.apache.org/r/53385/#comment224421> Again, here you need to explicitly wait for container to terminate and cleanup containers, something like this (from a similar test): ``` Future<Option<ContainerTermination>> termination = containerizer.wait(containerId.get()); driver.stop(); driver.join(); AWAIT_READY(termination); EXPECT_SOME(termination.get()); agent.get()->terminate(); agent->reset(); Future<std::list<Docker::Container>> containers = docker->ps(true, slave::DOCKER_NAME_PREFIX); AWAIT_READY(containers); // Clean up all mesos launched docker containers. foreach (const Docker::Container& container, containers.get()) { AWAIT_READY_FOR(docker->rm(container.id, true), Seconds(30)); } ``` - Alexander Rukletsov On Nov. 3, 2016, 3:52 p.m., Gastón Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53385/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2016, 3:52 p.m.) > > > Review request for mesos, Alexander Rukletsov and Benjamin Mahler. > > > Bugs: MESOS-6457 > https://issues.apache.org/jira/browse/MESOS-6457 > > > Repository: mesos > > > Description > ------- > > Added a test to ensure MESOS-6457 is fixed for the Docker executor. > > > Diffs > ----- > > src/tests/containerizer/docker_containerizer_tests.cpp > 810488d1476cadbbd5a4a7dcecaeec55739ab71f > > Diff: https://reviews.apache.org/r/53385/diff/ > > > Testing > ------- > > This test fails without the executor changes in the previous patch and > succeeds with them. Tested on both OS X and Linux. > > > Thanks, > > Gastón Kleiman > >
