----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69815/#review212337 -----------------------------------------------------------
src/tests/storage_local_resource_provider_tests.cpp Lines 4681-4683 (patched) <https://reviews.apache.org/r/69815/#comment298103> Could you maybe put the flow of the test somewhere, either here or in the test body. The test goes through a lot of states and is pretty long. src/tests/storage_local_resource_provider_tests.cpp Lines 4853 (patched) <https://reviews.apache.org/r/69815/#comment298097> Do we need this for synchronization? src/tests/storage_local_resource_provider_tests.cpp Lines 4886 (patched) <https://reviews.apache.org/r/69815/#comment298098> Could we add some output of e.g., the iteration for the assertions in this loop? Here and below. src/tests/storage_local_resource_provider_tests.cpp Lines 4887 (patched) <https://reviews.apache.org/r/69815/#comment298099> Can we assert than no more than one request is ready? src/tests/storage_local_resource_provider_tests.cpp Lines 4980 (patched) <https://reviews.apache.org/r/69815/#comment298101> Can we assert that only one request is ready at a time here? src/tests/storage_local_resource_provider_tests.cpp Lines 5001 (patched) <https://reviews.apache.org/r/69815/#comment298100> Since we probably already have some tests for successful deletetion, could we maybe add use some gRPC status that leads to failed operation as a smoke test? src/tests/storage_local_resource_provider_tests.cpp Lines 5011 (patched) <https://reviews.apache.org/r/69815/#comment298102> A comment here would probably make this easier to follow. - Benjamin Bannier On Jan. 25, 2019, 4:29 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69815/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2019, 4:29 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-9517 > https://issues.apache.org/jira/browse/MESOS-9517 > > > Repository: mesos > > > Description > ------- > > This patch adds a unit test to verify that SLRP will retry > `CreateVolume` and `DeleteVolume` CSI calls with a random exponential > backoff upon receiving `DEADLINE_EXCEEDED` or `UNAVAILABLE`. > > > Diffs > ----- > > src/tests/storage_local_resource_provider_tests.cpp > e8ed20f818ed7f1a3ce15758ea3c366520443377 > > > Diff: https://reviews.apache.org/r/69815/diff/4/ > > > Testing > ------- > > make check > > TODO: Will make the test more robust and add testing for `DESTROY_DISK`. > > > Thanks, > > Chun-Hung Hsiao > >
