> On April 13, 2018, 9:43 a.m., Greg Mann wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 455-459 (patched) > > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line455> > > > > Is this enforced somewhere in validation code? Can we check for > > expected behavior when a GROW/SHRINK operation is submitted for a MOUNT > > volume, rather than simply returning?
I added validation in r/66050 so we drop shrink volume operation on MOUNT. There is no logical path for triggering `GROW` on `MOUNT` disk type so I'm not writing validation. For testing, we can either keep the current structure and check that `GROW`/`SHRINK` do not work on `MOUNT` (operation will be dropped), or take Chun's suggestion to not test them for `MOUNT`. Please let me know. > On April 13, 2018, 9:43 a.m., Greg Mann wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 492 (patched) > > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line492> > > > > Is this necessary? Since the clock is paused, is it really possible > > that we'll get more offers than expected? This is very common in our tests (there are more than 500 line matches with this comment through `git grep`). My guess is that `clock::advance` can guarantee at least one offer being triggered but there might be some race condition cases which would make the test flaky if we do not guard against this? > On April 13, 2018, 9:43 a.m., Greg Mann wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 549 (patched) > > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line549> > > > > Is this necessary? See above comment. > On April 13, 2018, 9:43 a.m., Greg Mann wrote: > > src/tests/persistent_volume_tests.cpp > > Lines 553-554 (patched) > > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line553> > > > > Is this necessary? Awaiting on the task status updates may be > > sufficient? See above comment. - Zhitao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66220/#review201109 ----------------------------------------------------------- On April 11, 2018, 2:19 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66220/ > ----------------------------------------------------------- > > (Updated April 11, 2018, 2:19 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations. > > > Diffs > ----- > > src/tests/persistent_volume_tests.cpp > 4edf781711d9efdb994114aeb6289b6af750b87a > > > Diff: https://reviews.apache.org/r/66220/diff/4/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
