----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65232/#review195836 -----------------------------------------------------------
Fix it, then Ship it! Thanks for this fix Greg. This will help remove some unsafe boilerplate from tests. We should probably follow up with a tech-debt ticket removing unnecessary clock resumes in tests. src/tests/cluster.cpp Lines 117 (patched) <https://reviews.apache.org/r/65232/#comment275164> This would be the only symbol from `process` we pull into this scope. Let's stick to the current pattern of full names here for local consistency. src/tests/cluster.cpp Line 342 (original), 344 (patched) <https://reviews.apache.org/r/65232/#comment275165> Suggest to not touch this now, see above. Here and below in `Master::start`. - Benjamin Bannier On Jan. 19, 2018, 1:40 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65232/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2018, 1:40 a.m.) > > > Review request for mesos, Benjamin Bannier and Gaston Kleiman. > > > Repository: mesos > > > Description > ------- > > Because the `cgroups::destroy()` code path makes use of `delay()`, > the clock must not be paused in order for the destructor of the > test `Slave` to reliably destroy all remaining containers. > > This patch updates the destructor to check if the clock is paused > and, if so, resume it before destroying containers. > > > Diffs > ----- > > src/tests/cluster.cpp 568c9c7eda9d69e85bc32ce3259548b348ed9f25 > > > Diff: https://reviews.apache.org/r/65232/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Greg Mann > >