----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53455/#review154962 -----------------------------------------------------------
Fix it, then Ship it! LGTM. Might have preferred a few smaller patches (such as one to combine the `TestContainerizer` constructors), but I'm not going to block the review on that. src/tests/containerizer.hpp (line 137) <https://reviews.apache.org/r/53455/#comment224761> s/mocks/mock/ src/tests/containerizer.cpp (lines 246 - 251) <https://reviews.apache.org/r/53455/#comment224776> This function seems to be un-used (or I'm just not seeing it). - Joseph Wu On Nov. 3, 2016, 11:15 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53455/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2016, 11:15 p.m.) > > > Review request for mesos and Joseph Wu. > > > Bugs: MESOS-6544 and MESOS-6545 > https://issues.apache.org/jira/browse/MESOS-6544 > https://issues.apache.org/jira/browse/MESOS-6545 > > > Repository: mesos > > > Description > ------- > > The TestContainerizer is currently not backed by a Process and > does not do any explicit synchronization and so is not thread safe. > > Most tests currently cannot trip the concurrency issues, but > this surfaced recently in MESOS-6544. > > > Diffs > ----- > > src/tests/containerizer.hpp d418ecebee155f894187e24549f42497372d8a5f > src/tests/containerizer.cpp c2208660352e5cf62cec5c7b92ec67fea62aea49 > > Diff: https://reviews.apache.org/r/53455/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Mahler > >
