> On June 8, 2018, 4:12 p.m., Andrew Schwartzmeyer wrote: > > src/tests/containerizer/docker_containerizer_tests.cpp > > Line 300 (original), 339 (patched) > > <https://reviews.apache.org/r/67457/diff/1/?file=2035342#file2035342line339> > > > > What changed here to need another argument? > > Akash Gupta wrote: > It's just an optimization. The `exists()` function works like this: > ``` > bool exists(docker, id, desiredState) { > while(time < timeout) { > inspectResult = docker->inspect() > > if (inspectResult.success()) { > if (desiredState == EXISTS) { return true; } > if (desiredState == RUNNING && inspectResult.containerIsRunning()) > { return true; } > } > > wait(SOME_TIME); > updateTime(&time, SOME_TIME); > } > return false; > } > ``` > > Basically it retries an unsuccessfult result until you get a timeout. > But, a lot of the tests add a `ASSERT_FALSE(docker, container, > ContainerState::{RUNNING,EXISTS}, false)` at the end to check if the > container is *not* running or existing. Since false is only returned after > `timeout`, each test would spend `timeout` seconds just doing the above loop. > This was especially bad on Windows, since the timeouts need to be longer, so > each test was waiting like 15+ seconds just for `exists()` to return false. > So I just added another variable and controls the retry behavior, so that the > tests don't waste so much time.
Gotcha! I like it; good improvement. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67457/#review204507 ----------------------------------------------------------- On June 5, 2018, 1:40 p.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67457/ > ----------------------------------------------------------- > > (Updated June 5, 2018, 1:40 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. > > > Bugs: MESOS-7342 > https://issues.apache.org/jira/browse/MESOS-7342 > > > Repository: mesos > > > Description > ------- > > With some Docker bug fixes and the IOCP backend, the remaining docker > have been ported. The only remaining Docker tests that aren't ported > are either due to limitations on Windows Containers or unimplemented > features in Mesos (Persistent volume and hooks). > > > Diffs > ----- > > src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 > src/tests/containerizer/docker_containerizer_tests.cpp > 194308bf9d09a3562e683c49e0da4c9a6463d66e > > > Diff: https://reviews.apache.org/r/67457/diff/1/ > > > Testing > ------- > > > Thanks, > > Akash Gupta > >
