Re: Review Request 38126: Add UT for QuiesceOffers

[Guangya Liu]

> On 九月 15, 2015, 7:43 p.m., Vinod Kone wrote:
> > you should've merged this test review with the dependent review. i really 
> > like tests to be present in the same patch as the code change, to give me 
> > confidence that the code changes are correct.
> > 
> > also, no tests for the scheduler driver?

This is now merged to https://reviews.apache.org/r/38124/diff/2/ 

I'm now working on another patch to add some tests for schduler driver. I see 
that the reviveOffers also do not have a test, will try to see if the test can 
cover both cases: revive and quiesce offer.


> On 九月 15, 2015, 7:43 p.m., Vinod Kone wrote:
> > src/tests/scheduler_tests.cpp, lines 1004-1005
> > 
> >
> > // On revival scheduler should get another offer with the same amount 
> > of resources.
> > 
> > also, i'm a bit confused on how/why the scheduler gets another offer 
> > (for the same resources) considering the framework is holding on to the 
> > original offer?

I updated the logic a bit, the sequence is as this: subscribe->get 
offer->decline offer with a filter of 60min->enable quiesce offer-> wait 
100min, no offer->revive the offer->get offer, please refer to 
https://reviews.apache.org/r/38124/diff/2/ for detail. Thanks.


- Guangya


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


On 九月 16, 2015, 5:44 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> ---
> 
> (Updated 九月 16, 2015, 5:44 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38126: Add UT for QuiesceOffers

[Guangya Liu]

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

(Updated 九月 16, 2015, 5:44 a.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
Kone.


Bugs: MESOS-3037
https://issues.apache.org/jira/browse/MESOS-3037


Repository: mesos


Description
---

Add UT for QuiesceOffers


Diffs
-

  src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 

Diff: https://reviews.apache.org/r/38126/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38126: Add UT for QuiesceOffers

[Vinod Kone]

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


you should've merged this test review with the dependent review. i really like 
tests to be present in the same patch as the code change, to give me confidence 
that the code changes are correct.

also, no tests for the scheduler driver?


src/tests/scheduler_tests.cpp (lines 1004 - 1005)


// On revival scheduler should get another offer with the same amount of 
resources.

also, i'm a bit confused on how/why the scheduler gets another offer (for 
the same resources) considering the framework is holding on to the original 
offer?


- Vinod Kone


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> ---
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38126: Add UT for QuiesceOffers

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37532, 37873, 38119, 38120, 38121, 38124, 38126]

All tests passed.

- Mesos ReviewBot


On Sept. 4, 2015, 12:06 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38126/
> ---
> 
> (Updated Sept. 4, 2015, 12:06 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Jie Yu, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add UT for QuiesceOffers
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 
> 
> Diff: https://reviews.apache.org/r/38126/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>