----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66569/#review201179 -----------------------------------------------------------
src/tests/persistent_volume_tests.cpp Lines 691-695 (patched) <https://reviews.apache.org/r/66569/#comment282236> Since this test just verifies that the grow and shrink operations cannot be combined with other operations, I was wondering if there's a way to check this without worry about the different behaviors produced by PATH or MOUNT. Alternatively, if you take my suggestion to create another `PersistentVolumeResizeTest` fixture in the previous patch, we can move this test into that fixture. src/tests/persistent_volume_tests.cpp Lines 702 (patched) <https://reviews.apache.org/r/66569/#comment282405> No need for this. src/tests/persistent_volume_tests.cpp Lines 728 (patched) <https://reviews.apache.org/r/66569/#comment282407> No need for this if the clock is already paused. src/tests/persistent_volume_tests.cpp Lines 732 (patched) <https://reviews.apache.org/r/66569/#comment282227> Let's move this to the beginning of this test to make it clear that this test pauses the clock. src/tests/persistent_volume_tests.cpp Lines 775-781 (patched) <https://reviews.apache.org/r/66569/#comment282411> As discussed, I'm not sure if this is the semantics we want to enforce. If we end up rejecting the whole `ACCEPT` call, then we could simplify the test by: 1. Starting with a persistent volume, some free disk and some CPU. 2. Open receiving an offer, accept it with reserving the CPU and growing the persistent volume. Different type of resources are used here to make sure it's not just for preventing pipelining. 3. Verify that the `ACCEPT` call fails by checking that the next offer contains the original resources. src/tests/persistent_volume_tests.cpp Lines 791 (patched) <https://reviews.apache.org/r/66569/#comment282412> No need for this. src/tests/persistent_volume_tests.cpp Lines 814-861 (patched) <https://reviews.apache.org/r/66569/#comment282413> How about moving the `SHRINK_VOLUME` test into another test? Let's keep the test focused. - Chun-Hung Hsiao On April 11, 2018, 9:19 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66569/ > ----------------------------------------------------------- > > (Updated April 11, 2018, 9:19 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Greg Mann. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > Added a test to verify that grow and shrink cannot be combined. > > > Diffs > ----- > > src/tests/persistent_volume_tests.cpp > 4edf781711d9efdb994114aeb6289b6af750b87a > > > Diff: https://reviews.apache.org/r/66569/diff/1/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
