> On Jan. 19, 2015, 2:27 p.m., Alexander Rukletsov wrote:
> > src/tests/master_allocator_tests.cpp, lines 235-236
> > <https://reviews.apache.org/r/29991/diff/1/?file=824066#file824066line235>
> >
> >     What is the reason to leave this expectation? I think it's covered by 
> > the new code in /r/29990.

This is needed because (if you expand the diff) there is **another** 
expectation on recoverResources that overrides the default. This restores the 
default before we shut down.


> On Jan. 19, 2015, 2:27 p.m., Alexander Rukletsov wrote:
> > src/tests/master_allocator_tests.cpp, lines 238-243
> > <https://reviews.apache.org/r/29991/diff/1/?file=824066#file824066line238>
> >
> >     It looks like removing these expectations (actually replacing them with 
> > `EXPECT_CALL(*this, deactivateFramework(_)).WillRepeatedly(DoDefault());` 
> > from /r/29990) changes the original logic. Do I miss the point?

Yes I changed the expectations here, the reasoning is that I want to avoid 
expectations that can be violated without us knowing.

In this case, it's possible that the code calls these things 3 times, but the 
master is always shutting down first and we never trip the failure. These were 
put in place to suppress the potential for warnings (given the old ON_CALL 
mocking approach).


- Ben


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


On Jan. 16, 2015, 11:12 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29991/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2015, 11:12 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2232
>     https://issues.apache.org/jira/browse/MESOS-2232
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Removed unnecessary allocator expectations in the tests.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_allocator_tests.cpp 
> 2430622d09c7ef1e020e2eb8f97444e7efc7c8ea 
>   src/tests/resource_offers_tests.cpp 
> d098e7016ac0da7f1d629af099bb1b8fa66da839 
>   src/tests/slave_recovery_tests.cpp 809822e63b05a21418cd9297c927d656d6fd871d 
> 
> Diff: https://reviews.apache.org/r/29991/diff/
> 
> 
> Testing
> -------
> 
> make check with many iterations
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to