----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64992/#review195055 -----------------------------------------------------------
src/examples/test_csi_plugin.cpp Lines 591-595 (original), 591-597 (patched) <https://reviews.apache.org/r/64992/#comment274181> Could you leave a comment here explaining what we're doing and why? i.e., we are ensuring we only report capacity for the default-constructed MountVolume capability since this module only simulates a volume. src/tests/storage_local_resource_provider_tests.cpp Lines 154 (patched) <https://reviews.apache.org/r/64992/#comment274184> You can use `.getOrElse` here instead. src/tests/storage_local_resource_provider_tests.cpp Lines 245 (patched) <https://reviews.apache.org/r/64992/#comment274190> Is there a reason not to move this to the top of the test body? I think that improves readability - placing the pause here implies that there's a reason we're pausing at this precise point, but AFAICT there is not. src/tests/storage_local_resource_provider_tests.cpp Lines 258 (patched) <https://reviews.apache.org/r/64992/#comment274191> I think this can be removed. src/tests/storage_local_resource_provider_tests.cpp Lines 282 (patched) <https://reviews.apache.org/r/64992/#comment274192> I think this could be moved to just after L267, which makes the reason for the resume/pause a bit more explicit. src/tests/storage_local_resource_provider_tests.cpp Lines 292 (patched) <https://reviews.apache.org/r/64992/#comment274193> This can be removed. - Greg Mann On Jan. 9, 2018, 7:17 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64992/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2018, 7:17 p.m.) > > > Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu. > > > Bugs: MESOS-8407 > https://issues.apache.org/jira/browse/MESOS-8407 > > > Repository: mesos > > > Description > ------- > > This patch adds the following tests: > > `NoResource`: RP updates its state with no resource, and can recover > from a checkpointed state that contains no resource. > > `ZeroSizedDisk`: CSI plugin reports a pre-existing volume with > zero capacity. > > `SmallDisk`: CSI plugin reports a storage pool and a pre-existing volume > with sizes < 1MB. > > `NewProfile`: A new profile is added after RP updates its state. > > > Diffs > ----- > > src/examples/test_csi_plugin.cpp f6b2c98c44366d5c752aa44bdbf8ae0374a7765c > src/tests/storage_local_resource_provider_tests.cpp > bbfe95e9818f25fdd5405db3ad2fe355e023f743 > > > Diff: https://reviews.apache.org/r/64992/diff/3/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
