You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Thomas Marshall <tw...@gmail.com> on 2013/04/07 20:38:51 UTC
Review Request: Fixed Allocator Test to use DROP_MESSAGE, FutureArg,
moved expectations closer to actions, etc
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10329/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Description
-------
See summary.
Additionally, I replaced the MockAllocator in DRFAllocatorTest with a regular Allocator because the expectations were uninteresting, expectations on calls to the allocator are covered in other tests, and it just made the test a lot messier.
Diffs
-----
src/tests/allocator_tests.cpp 04a8581
Diff: https://reviews.apache.org/r/10329/diff/
Testing
-------
bin/mesos-tests.sh --gtest_filter=*AllocatorTest* --gtest_repeat=3000 --gtest_break_on_failure
Thanks,
Thomas Marshall
Re: Review Request: Fixed Allocator Test to use DROP_MESSAGE, FutureArg,
moved expectations closer to actions, etc
Posted by Thomas Marshall <tw...@gmail.com>.
> On April 9, 2013, 8:52 p.m., Ben Mahler wrote:
> > src/tests/allocator_tests.cpp, line 1353
> > <https://reviews.apache.org/r/10329/diff/1/?file=278455#file278455line1353>
> >
> > Do you want to settle() after this advance?
I didn't realize before that Clock::advance doesn't actually do anything unless the clock is paused, so since the tests have been passing despite these advances being useless they apparently aren't necessary and I took them out.
> On April 9, 2013, 8:52 p.m., Ben Mahler wrote:
> > src/tests/allocator_tests.cpp, line 1459
> > <https://reviews.apache.org/r/10329/diff/1/?file=278455#file278455line1459>
> >
> > Why this commented out line?
Sorry, that should've been taken out.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10329/#review18885
-----------------------------------------------------------
On April 10, 2013, 6:36 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10329/
> -----------------------------------------------------------
>
> (Updated April 10, 2013, 6:36 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
> Additionally, I replaced the MockAllocator in DRFAllocatorTest with a regular Allocator because the expectations were uninteresting, expectations on calls to the allocator are covered in other tests, and it just made the test a lot messier.
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp 04a8581
>
> Diff: https://reviews.apache.org/r/10329/diff/
>
>
> Testing
> -------
>
> bin/mesos-tests.sh --gtest_filter=*AllocatorTest* --gtest_repeat=3000 --gtest_break_on_failure
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request: Fixed Allocator Test to use DROP_MESSAGE, FutureArg,
moved expectations closer to actions, etc
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10329/#review18885
-----------------------------------------------------------
Ship it!
Looks good at a quick glance.
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/10329/#comment39342>
Do you want to settle() after this advance?
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/10329/#comment39343>
ditto
s/1.0/1?
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/10329/#comment39345>
Why this commented out line?
src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/10329/#comment39346>
ditto
- Ben Mahler
On April 7, 2013, 6:38 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10329/
> -----------------------------------------------------------
>
> (Updated April 7, 2013, 6:38 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
> Additionally, I replaced the MockAllocator in DRFAllocatorTest with a regular Allocator because the expectations were uninteresting, expectations on calls to the allocator are covered in other tests, and it just made the test a lot messier.
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp 04a8581
>
> Diff: https://reviews.apache.org/r/10329/diff/
>
>
> Testing
> -------
>
> bin/mesos-tests.sh --gtest_filter=*AllocatorTest* --gtest_repeat=3000 --gtest_break_on_failure
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request: Fixed Allocator Test to use DROP_MESSAGE, FutureArg,
moved expectations closer to actions, etc
Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10329/
-----------------------------------------------------------
(Updated April 10, 2013, 6:36 p.m.)
Review request for mesos and Benjamin Hindman.
Changes
-------
Updated to Ben's review.
I also changed all of the resourcesChanged expectations to be WillRepeatedly(DoDefault()). This is to make the tests more robust since the number of times resourcesChanged might get called isn't a hard guarantee depending on specifics about the slave and the timing of the test, and its not something that these tests care about anyways.
Description
-------
See summary.
Additionally, I replaced the MockAllocator in DRFAllocatorTest with a regular Allocator because the expectations were uninteresting, expectations on calls to the allocator are covered in other tests, and it just made the test a lot messier.
Diffs (updated)
-----
src/tests/allocator_tests.cpp 04a8581
Diff: https://reviews.apache.org/r/10329/diff/
Testing
-------
bin/mesos-tests.sh --gtest_filter=*AllocatorTest* --gtest_repeat=3000 --gtest_break_on_failure
Thanks,
Thomas Marshall
Re: Review Request: Fixed Allocator Test to use DROP_MESSAGE, FutureArg,
moved expectations closer to actions, etc
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On April 9, 2013, 7:43 p.m., Ben Mahler wrote:
> > Great to see all the sleeps gone!!
> >
> > Does the new Cluster testing abstraction fit cleanly here? See: src/tests/cluster.hpp.
We'd talked about doing that in another review.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10329/#review18873
-----------------------------------------------------------
On April 7, 2013, 6:38 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10329/
> -----------------------------------------------------------
>
> (Updated April 7, 2013, 6:38 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
> Additionally, I replaced the MockAllocator in DRFAllocatorTest with a regular Allocator because the expectations were uninteresting, expectations on calls to the allocator are covered in other tests, and it just made the test a lot messier.
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp 04a8581
>
> Diff: https://reviews.apache.org/r/10329/diff/
>
>
> Testing
> -------
>
> bin/mesos-tests.sh --gtest_filter=*AllocatorTest* --gtest_repeat=3000 --gtest_break_on_failure
>
>
> Thanks,
>
> Thomas Marshall
>
>
Re: Review Request: Fixed Allocator Test to use DROP_MESSAGE, FutureArg,
moved expectations closer to actions, etc
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10329/#review18873
-----------------------------------------------------------
Great to see all the sleeps gone!!
Does the new Cluster testing abstraction fit cleanly here? See: src/tests/cluster.hpp.
- Ben Mahler
On April 7, 2013, 6:38 p.m., Thomas Marshall wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10329/
> -----------------------------------------------------------
>
> (Updated April 7, 2013, 6:38 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Description
> -------
>
> See summary.
>
> Additionally, I replaced the MockAllocator in DRFAllocatorTest with a regular Allocator because the expectations were uninteresting, expectations on calls to the allocator are covered in other tests, and it just made the test a lot messier.
>
>
> Diffs
> -----
>
> src/tests/allocator_tests.cpp 04a8581
>
> Diff: https://reviews.apache.org/r/10329/diff/
>
>
> Testing
> -------
>
> bin/mesos-tests.sh --gtest_filter=*AllocatorTest* --gtest_repeat=3000 --gtest_break_on_failure
>
>
> Thanks,
>
> Thomas Marshall
>
>