> On March 18, 2016, 11:24 p.m., Ben Mahler wrote: > > src/tests/slave_tests.cpp, line 3264 > > <https://reviews.apache.org/r/44994/diff/2/?file=1306236#file1306236line3264> > > > > You don't need a settle here, AWAIT_READY will settle if the clock is > > paused. > > Alexander Rukletsov wrote: > I feel that this is not obvious, that settle will happen in `AWAIT_READY` > if clock is paused. Moreover, when we don't await, we have to explicitly > settle, which—I think—makes it harder for folks to follow the test and reason > when settle is necessary, because sometimes a necessary settle is implicit > (inside await). Hence I tend to put explicit settles when I feel this makes > the code more obvious. Do you have a strong preference? > > Ben Mahler wrote: > Think about it this way, if you put a `settle()` before the > `AWAIT_READY`, then the implication is that you don't need `AWAIT_READY` at > all, you could just do `ASSERT_TRUE(f.isReady())` because you've done the > necessary settling explicitly. This is why it seems strange to have a > `settle` immediately preceed an `AWAIT_READY`, its the job of `AWAIT_READY` > to wait until the future is ready (which means doing the right thing when the > clock is paused, otherwise we would have to write a lot of `settle` calls).
I see, makes sense. Thanks Ben! - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44994/#review124308 ----------------------------------------------------------- On March 22, 2016, 5:07 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44994/ > ----------------------------------------------------------- > > (Updated March 22, 2016, 5:07 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-4949 > https://issues.apache.org/jira/browse/MESOS-4949 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 > src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 > > Diff: https://reviews.apache.org/r/44994/diff/ > > > Testing > ------- > > On Mac OS 10.10.4: > `make check` > `GTEST_FILTER="*ExecutorShutdownGracePeriod*" ./bin/mesos-tests.sh > --gtest_repeat=100 --gtest_break_on_failure` > > > Thanks, > > Alexander Rukletsov > >
