You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2017/04/04 19:37:27 UTC

Re: Review Request 57925: Added further tests for executor secret generation.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57925/#review171028
-----------------------------------------------------------




src/tests/slave_tests.cpp
Lines 5904-5905 (patched)
<https://reviews.apache.org/r/57925/#comment243860>

    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?


- Vinod Kone


On March 25, 2017, 6:27 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57925/
> -----------------------------------------------------------
> 
> (Updated March 25, 2017, 6:27 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 c31c670b667240c4876d415aa5cf90cb34273e8a 
> 
> 
> Diff: https://reviews.apache.org/r/57925/diff/7/
> 
> 
> 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
> 
>


Re: Review Request 57925: Added further tests for executor secret generation.

Posted by Greg Mann <gr...@mesosphere.io>.

> 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
> 
>