----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57883/#review170071 -----------------------------------------------------------
src/tests/slave_tests.cpp Lines 5138 (patched) <https://reviews.apache.org/r/57883/#comment242859> hmm, thinking about this more. Can we set the expectations on this object first and then hand the `Owned` pointer to the `MockSlave` object? Currently, it looks possible that the test as well as the agent thread might be using the `secretGenerator` leading to a race? src/tests/slave_tests.cpp Lines 5222 (patched) <https://reviews.apache.org/r/57883/#comment242860> How about: ```cpp // The tasks will fail to launch because the executor secret generation fails. ``` src/tests/slave_tests.cpp Lines 5223-5225 (patched) <https://reviews.apache.org/r/57883/#comment242861> You might have to move this earlier as per my earlier comments. src/tests/slave_tests.cpp Lines 5280 (patched) <https://reviews.apache.org/r/57883/#comment242862> New line after since this comment applies to both `ACKNOWLEDGE` code blocks. src/tests/slave_tests.cpp Lines 5336 (patched) <https://reviews.apache.org/r/57883/#comment242866> Ditto as the initial comment in the last test around using `Owned` and setting the expectation before passing ownership? src/tests/slave_tests.cpp Lines 5402-5418 (patched) <https://reviews.apache.org/r/57883/#comment242863> Can you move these after L5428 since these would happen only after those expectations are fullfilled? src/tests/slave_tests.cpp Lines 5421 (patched) <https://reviews.apache.org/r/57883/#comment242864> How about: ```cpp // The tasks will fail to launch because the executor secret is invalid. ``` src/tests/slave_tests.cpp Lines 5486 (patched) <https://reviews.apache.org/r/57883/#comment242865> Newline after this since this comment applies to both `ACKNOWLEGE` code blocks. - Anand Mazumdar On March 24, 2017, 9:51 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57883/ > ----------------------------------------------------------- > > (Updated March 24, 2017, 9:51 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-6999 > https://issues.apache.org/jira/browse/MESOS-6999 > > > Repository: mesos > > > Description > ------- > > This patch adds new tests, > `SlaveTest.RunTaskGroupFailedSecretGeneration` and > `SlaveTest.RunTaskGroupInvalidExecutorSecret`, to > verify the agent's behavior when generation of the > executor secret fails. > > > Diffs > ----- > > src/tests/slave_tests.cpp c31c670b667240c4876d415aa5cf90cb34273e8a > > > Diff: https://reviews.apache.org/r/57883/diff/4/ > > > Testing > ------- > > `make check` > `bin/mesos-tests.sh > --gtest_filter="*SlaveTest.RunTaskGroupFailedSecretGeneration*:SlaveTest.RunTaskGroupInvalidExecutorSecret*" > --gtest_repeat=-1 --gtest_break_on_failure` was used to verify that the new > tests are not flaky. > > > Thanks, > > Greg Mann > >
