----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63843/#review192103 -----------------------------------------------------------
src/tests/slave_tests.cpp Lines 8831-8839 (patched) <https://reviews.apache.org/r/63843/#comment270128> Any reason these EXPECT are in the scope? src/tests/slave_tests.cpp Lines 8900-8923 (patched) <https://reviews.apache.org/r/63843/#comment270129> It'll be nice to move this logic here to MockResourceProvider::operationFailed and call `Invoke(...)` in the EXPECT above. src/tests/slave_tests.cpp Lines 8933-8936 (patched) <https://reviews.apache.org/r/63843/#comment270130> This is a bit subtle. If a speculative operation fails, the update state will inform the agent that the operation has failed. However, the agent will inform that master that the operation is still pending? What's the downside of telling agent master the same thing (that the operation has failed)? This probably needs more thinking. - Jie Yu On Nov. 28, 2017, 9:32 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63843/ > ----------------------------------------------------------- > > (Updated Nov. 28, 2017, 9:32 p.m.) > > > Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > Whenever a speculated operation fails in a resource provider, we > expect the agent to trigger a 'UpdateSlaveMessage' to the master so it > can rollback its agent state. This patch adds such a test. > > > Diffs > ----- > > src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 > > > Diff: https://reviews.apache.org/r/63843/diff/6/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
