> On June 20, 2018, 12:43 p.m., Benjamin Bannier wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Line 529 (original), 534 (patched) > > <https://reviews.apache.org/r/67667/diff/1/?file=2042686#file2042686line534> > > > > Just wondering what actually requires `ROOT` in this test. I see we > > enable filesystem isolation, but is that required?
The SLRP launches the CSI plugin in a standalone container, and it seems safer to enable the isolation. In contrast, I could disable this isolation and manually do an `unmountAll` during teardown. Do you prefer the latter? If yes I can have a followup patch to do this. > On June 20, 2018, 12:43 p.m., Benjamin Bannier wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 556 (patched) > > <https://reviews.apache.org/r/67667/diff/1/?file=2042686#file2042686line557> > > > > Let's pass this into `StartMaster`. Oops. > On June 20, 2018, 12:43 p.m., Benjamin Bannier wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 621 (patched) > > <https://reviews.apache.org/r/67667/diff/1/?file=2042686#file2042686line629> > > > > Formatting. Oops again :( Thanks for catching this! > On June 20, 2018, 12:43 p.m., Benjamin Bannier wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 634-638 (patched) > > <https://reviews.apache.org/r/67667/diff/1/?file=2042686#file2042686line642> > > > > Let's add at least one `Clock::settle` here for consistency even though > > it is implicitly called in e.g., `AWAIT_READY`. You know my opinion on this ;) But sure I'll do it for consistency. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67667/#review205059 ----------------------------------------------------------- On June 20, 2018, 5:33 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67667/ > ----------------------------------------------------------- > > (Updated June 20, 2018, 5:33 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-8995 > https://issues.apache.org/jira/browse/MESOS-8995 > > > Repository: mesos > > > Description > ------- > > The test is renamed to `ProfileAppeared` to reflect the fact that the > SLRP passively observed a profile showing up, not actively adding a > profile. > > The test is changed from intercepting `UpdateSlaveMessage`s to checking > offers to make it more like an end-to-end test. Also with the > `DiskProfileServer` helper we can control the clock better. > > > Diffs > ----- > > src/tests/storage_local_resource_provider_tests.cpp > 1afe8a8e0413ef225e952cd9cd6376e5d82774e5 > > > Diff: https://reviews.apache.org/r/67667/diff/1/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
