----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52539/#review151706 -----------------------------------------------------------
src/tests/containerizer/filesystem_isolator_tests.cpp <https://reviews.apache.org/r/52539/#comment220178> This is my main concern on this patch. We bundle the `filesystem/linux` isolator with the `docker/runtime` isolator for all unit tests in this file because of getting rid of the helper `createContainerizer()`. Even though we know that is doable, I still consider whether or not we should do that, because: 1. In Mesos, the `docker/runtime` isolator depends on the `filesystem/linux`. Then, should we have the `filesystem/linux` tests relying on `docker/runtime`? isn't a stand alone test environment better? 2. We already know `filesystem/linux` tests slow. And now, except copying some files from the host rootfs, we also need to tar and untar it. Have the provisioner, docker store and local puller get involved. And go through the docker runtime isolator logic. Seems a little expansive to me. src/tests/containerizer/filesystem_isolator_tests.cpp (lines 89 - 109) <https://reviews.apache.org/r/52539/#comment220168> Consider a followup patch to move this to tests/mesos.hpp? src/tests/containerizer/filesystem_isolator_tests.cpp (line 119) <https://reviews.apache.org/r/52539/#comment220169> Use `const` if sounds good to you. Maybe we can chat a little bit to find the balance? - Gilbert Song On Oct. 4, 2016, 6:15 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52539/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2016, 6:15 p.m.) > > > Review request for mesos, Benjamin Hindman and Gilbert Song. > > > Repository: mesos > > > Description > ------- > > Refactored and simplified LinuxFilesystemIsolatorTests. > > > Diffs > ----- > > src/tests/containerizer/filesystem_isolator_tests.cpp > 5b6a1d409293e8d16f6bd78bc9c6f5a524c5e0fe > src/tests/mesos.hpp a05e3e0fad92169dc550f1a1a8b704074716d99e > > Diff: https://reviews.apache.org/r/52539/diff/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Jie Yu > >
