----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69158/#review212104 -----------------------------------------------------------
src/tests/slave_tests.cpp Lines 10731 (patched) <https://reviews.apache.org/r/69158/#comment297710> `v1::ResourceProviderInfo` src/tests/slave_tests.cpp Lines 10755 (patched) <https://reviews.apache.org/r/69158/#comment297709> We can use a const reference here: `const v1::ResourceProviderID&` src/tests/slave_tests.cpp Lines 10834 (patched) <https://reviews.apache.org/r/69158/#comment297711> If you think it is a good idea to create this alias, how about moving it to the beginning so the above two occurrences of `ContentType::PROTOBUF` can all be replaced? src/tests/slave_tests.cpp Lines 10895-10902 (patched) <https://reviews.apache.org/r/69158/#comment297712> Is the order guaranteed? If yes, it seems to me that it is sufficient to just verify that the scheduler receives `OPERATION_GONE_BY_OPERATOR` right? The order validation here makes the test a bit hard to follow, so I suggest just remove the check here, as well as the two futures. If the order is not guaranteed, then it seems we have a problem to resolve. src/tests/slave_tests.cpp Lines 10924-10926 (patched) <https://reviews.apache.org/r/69158/#comment297713> To make this check more reliable, we should add a `Clock::settle()` at the end of the test. Even so, it cannot 100% verify the property to test since HTTP messages could be in transmission when the test stops. src/tests/slave_tests.cpp Lines 10946 (patched) <https://reviews.apache.org/r/69158/#comment297714> Let's check that the operation status has an agent ID but no resource provider ID. - Chun-Hung Hsiao On Jan. 16, 2019, 10:51 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69158/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2019, 10:51 a.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8403 > https://issues.apache.org/jira/browse/MESOS-8403 > > > Repository: mesos > > > Description > ------- > > Added an integration test for resource provider removal. > > > Diffs > ----- > > src/tests/slave_tests.cpp dc711de96053ebecb6b71b98f14f22cb9b050c52 > > > Diff: https://reviews.apache.org/r/69158/diff/6/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
