----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67009/#review203911 -----------------------------------------------------------
src/tests/resource_provider_manager_tests.cpp Lines 846 (patched) <https://reviews.apache.org/r/67009/#comment286291> Since the purpose of this test and the one below is the registrar, not the underlying storage, it seems to me that the in-memory store is sufficient for this test (there is no need to create a new `mesos::state::Storage` instance), and also makes this test rely on fewer prerequisits. WDYT? src/tests/resource_provider_manager_tests.cpp Lines 847 (patched) <https://reviews.apache.org/r/67009/#comment286289> Is there a reason why `os::getcwd()` is used here instead of `sandbox.get()`? Ditto below. src/tests/resource_provider_manager_tests.cpp Line 844 (original), 853 (patched) <https://reviews.apache.org/r/67009/#comment286293> Could you briefly explain why you're advocating this pattern? I personally prefer the original pattern. Ditto below. src/tests/resource_provider_manager_tests.cpp Lines 883-884 (patched) <https://reviews.apache.org/r/67009/#comment286294> We could do `ASSERT_SOME_NE(nullptr, registrar);` here and below. - Chun-Hung Hsiao On May 8, 2018, 3:16 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67009/ > ----------------------------------------------------------- > > (Updated May 8, 2018, 3:16 p.m.) > > > Review request for mesos and Chun-Hung Hsiao. > > > Bugs: MESOS-8837 > https://issues.apache.org/jira/browse/MESOS-8837 > > > Repository: mesos > > > Description > ------- > > Added tests of resource provider registrar recovery. > > > Diffs > ----- > > src/tests/resource_provider_manager_tests.cpp > c2045f2ba24f3e4b959115f23b706e733a75fea8 > > > Diff: https://reviews.apache.org/r/67009/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
