----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70836/#review215897 -----------------------------------------------------------
src/tests/slave_tests.cpp Lines 11785-11786 (patched) <https://reviews.apache.org/r/70836/#comment302835> Do we want to add e.g., tasks into this test later? As written it would fit better into the API tests (though they have pretty bad scope creep). src/tests/slave_tests.cpp Lines 11796 (patched) <https://reviews.apache.org/r/70836/#comment302833> Why do we wait for the `UpdateSlaveMessage`? I'd expect `SlaveRegisteredMessage` would be enough for the test as written. If not please add a comment. src/tests/slave_tests.cpp Lines 11852 (patched) <https://reviews.apache.org/r/70836/#comment302836> I'd expect at least one of these to be an `EXPECT` since this block doesn't just check preconditions for the following expecation. Maybe ``` EXPECT_EQ(drainInfo, devolve(responseMessage->get_agent().drain_info())); ``` src/tests/slave_tests.cpp Lines 11874 (patched) <https://reviews.apache.org/r/70836/#comment302834> Let's not do this; it is done automatically and even if it were needed this would only be invoked if none of the above assertions had triggered (so it would be a potentially leaking, incorrect pattern). - Benjamin Bannier On June 11, 2019, 10:52 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70836/ > ----------------------------------------------------------- > > (Updated June 11, 2019, 10:52 p.m.) > > > Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and > Joseph Wu. > > > Bugs: MESOS-9818 > https://issues.apache.org/jira/browse/MESOS-9818 > > > Repository: mesos > > > Description > ------- > > Added test for DrainInfo in agent API outputs. > > > Diffs > ----- > > src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 > > > Diff: https://reviews.apache.org/r/70836/diff/1/ > > > Testing > ------- > > `make check` > `bin/mesos-tests.sh --gtest_filter="*DrainInfoInAPIOutputs*" > --gtest_repeat=-1 --gtest_break_on_failure` > > > Thanks, > > Greg Mann > >
