----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66671/#review203555 -----------------------------------------------------------
For the tests `DestroyDuringUnsupportedLaunchLoop` and `DestroyAfterLaunchLoop`, I think the intention of both of them is to check the value of `container->destroyed` set in `ComposingContainerizerProcess::_launch()`. But in your patch https://reviews.apache.org/r/66668 , `container->destroyed` has been removed, i.e., the destroy future value always depends on the underlying containerizer's destroy which is what we have test in the `DestroyDuringSupportedLaunchLoop`. So do we still need 3 tests? Or we can just keep one? src/tests/containerizer/composing_containerizer_tests.cpp Line 83 (original), 82 (patched) <https://reviews.apache.org/r/66671/#comment285844> Not yours, it seems `slaveId` is not used in the test. Ditto for the other two tests. src/tests/containerizer/composing_containerizer_tests.cpp Line 103 (original), 102 (patched) <https://reviews.apache.org/r/66671/#comment285845> Not yours, it seems `resources` is not used in the test. Ditto for the other two tests. src/tests/containerizer/composing_containerizer_tests.cpp Line 123 (original), 122 (patched) <https://reviews.apache.org/r/66671/#comment285843> s/desroy/destroy/ - Qian Zhang On April 17, 2018, 11:23 p.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66671/ > ----------------------------------------------------------- > > (Updated April 17, 2018, 11:23 p.m.) > > > Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian > Zhang. > > > Bugs: MESOS-8737 > https://issues.apache.org/jira/browse/MESOS-8737 > > > Repository: mesos > > > Description > ------- > > This patch updates composing containerizer tests in order to be > consistent with the unification of `destroy()` and `wait()` return > types. > > > Diffs > ----- > > src/tests/containerizer/composing_containerizer_tests.cpp > 6964ac2f050396a733b04e650e59d6c29c59665d > > > Diff: https://reviews.apache.org/r/66671/diff/4/ > > > Testing > ------- > > internal CI > > > Thanks, > > Andrei Budnik > >
