-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42241/#review127792
-----------------------------------------------------------




src/tests/hook_tests.cpp (line 391)
<https://reviews.apache.org/r/42241/#comment191185>

    I would argue that the "right thing to do" here is to explicitly call 
`containerizer.destroy`. `TestContainerizer` does not track whether an 
underlying executor has exited or not, hence we should manually destroy the 
container *after* executor shuts down. What you propose to do, is to wait until 
the agent forcibly shuts down the executor, which is fine, but not necessary, 
and hence may be misleading. Does it make sense?



src/tests/hook_tests.cpp (line 392)
<https://reviews.apache.org/r/42241/#comment191180>

    we don't need `settle` here because AWAIT_READY does it for us. However, we 
should resume the clock since in the future we may have a check that the clock 
is unpaused when test finishes.


- Alexander Rukletsov


On Feb. 19, 2016, 4:36 a.m., Jian Qiu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42241/
> -----------------------------------------------------------
> 
> (Updated Feb. 19, 2016, 4:36 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Timothy Chen.
> 
> 
> Bugs: MESOS-4174
>     https://issues.apache.org/jira/browse/MESOS-4174
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Speed up HookTest.VerifySlaveLaunchExecutorHook.
> 
> 
> Diffs
> -----
> 
>   src/tests/hook_tests.cpp 59a00ea722a17bbc82b14c69bda826f68cbac6e9 
> 
> Diff: https://reviews.apache.org/r/42241/diff/
> 
> 
> Testing
> -------
> 
> Before
> HookTest.VerifySlaveLaunchExecutorHook (5061 ms)
> 
> After
> HookTest.VerifySlaveLaunchExecutorHook (132 ms)
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Reply via email to