> On April 4, 2017, 7:37 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp > > Lines 5904-5905 (patched) > > <https://reviews.apache.org/r/57925/diff/7/?file=1675021#file1675021line5904> > > > > I'm assuming the tasks also fail even if the secret generation succeeds > > because of the shutdown sent by the framework? If yes, should we test that > > as well or instead?
Good point, the same code path is executed regardless of the state of the `Future<Secret>` returned by the secret generator in this case. I think it makes more sense to only test the successful secret generation case, since that is most common/relevant, and no need to test the failure case as well. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57925/#review171028 ----------------------------------------------------------- On April 5, 2017, 10:34 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57925/ > ----------------------------------------------------------- > > (Updated April 5, 2017, 10:34 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 two further tests for executor secret > generation, `SlaveTest.RunTaskGroupReferenceTypeSecret` > and `SlaveTest.RunTaskGroupFailedSecretAfterShutdown`. > > > Diffs > ----- > > src/tests/slave_tests.cpp cd769687e0f161512c0114ef4651508c31f51639 > > > Diff: https://reviews.apache.org/r/57925/diff/8/ > > > Testing > ------- > > `make check` > > `bin/mesos-tests.sh --gtest_filter="SlaveTest.*Secret*" --gtest_repeat=-1 > --gtest_break_on_failure` was used to check the new tests. > > > Thanks, > > Greg Mann > >
