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 2012/06/22 23:51:01 UTC

Review Request: Allocator tests

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

Review request for mesos and Benjamin Hindman.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


Diffs
-----

  src/Makefile.am 8760f59 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/base_zookeeper_test.hpp 4613376 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.

> On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 715
> > <https://reviews.apache.org/r/5532/diff/1/?file=115993#file115993line715>
> >
> >     DoDefault?

Unfortunately, DoDefault can't be used in a DoAll, but I added a comment to make this clearer


> On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote:
> > src/tests/base_zookeeper_test.hpp, line 89
> > <https://reviews.apache.org/r/5532/diff/1/?file=115994#file115994line89>
> >
> >     Was SetUpTestCase not protected?

This was necessary to be able to extend BaseZooKeeperTest for MasterFailoverAllocatorTest. I agree that taking something that was protected and making it public isn't ideal, but I don't know how else to solve this problem in C++.


> On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 160
> > <https://reviews.apache.org/r/5532/diff/1/?file=115993#file115993line160>
> >
> >     If the 'this->' isn't actually necessary we should remove it (here and everywhere else).

I agree that it doesn't seem like this should be necessary, but it doesn't compile without it (I guess because of the way the TYPED_TEST macro expands).


> On June 25, 2012, 6:41 p.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 288
> > <https://reviews.apache.org/r/5532/diff/1/?file=115993#file115993line288>
> >
> >     Why do you need to invoke this directly? A comment here (and anywhere else you're doing something like this) would be very useful. Also, this isn't "thread" safe, you'll need to dispatch here if you actually need to do this.

I was invoking them directly in order to ensure that they're deterministically called "out of order", i.e. a framework is removed and then some of its resources are recovered, but I can do the same thing with triggers and dispatches.


- Thomas


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


On June 26, 2012, 7:01 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5532/
> -----------------------------------------------------------
> 
> (Updated June 26, 2012, 7:01 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> Added tests for allocators.
> 
> This patch depends on two other currently pending review requests:
> https://reviews.apache.org/r/5448/
> https://reviews.apache.org/r/5451/
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 8760f59 
>   src/tests/allocator_tests.cpp PRE-CREATION 
>   src/tests/base_zookeeper_test.hpp 4613376 
>   src/tests/resource_offers_tests.cpp c1f1760 
>   src/tests/utils.hpp e81ec82 
>   src/tests/utils.cpp e7cda40 
> 
> Diff: https://reviews.apache.org/r/5532/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Allocator tests

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/#review8549
-----------------------------------------------------------



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18210>

    Newline.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18205>

    s/MyTypes/AllocatorTypes/



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18206>

    This will be great for future allocators! But maybe add a comment explaining that this is instantiating tests for each of the types in 'AllocatorTypes' so future people know exactly what to do.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18209>

    Newline.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18213>

    If the 'this->' isn't actually necessary we should remove it (here and everywhere else).



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18223>

    On previous line please (here and everywhere else).



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18224>

    Why do you need to invoke this directly? A comment here (and anywhere else you're doing something like this) would be very useful. Also, this isn't "thread" safe, you'll need to dispatch here if you actually need to do this.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18226>

    Ah, do these need to be done via a DoDefault action in the EXPECT_CALL's above?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18227>

    I'd like us to consider an alternative to TestScheduler. If you think the code in TestScheduler::resourceOffers is too much, let's create a helper function (maybe in utils) that given some offers and list of tasks does the necessary driver->launchTasks. That will be useful other places too! In addition, we will be able to kill the TestScheduler completely.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18211>

    An abbreviated version of the comment above would be great!



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18208>

    Newline.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18222>

    Fix whitespace problems please.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18212>

    Newline.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18216>

    How about we move this test to be the first one in the file? ;)



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18217>

    Isn't this just DoDefault?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18218>

    DoDefault?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18219>

    DoDefault?



src/tests/base_zookeeper_test.hpp
<https://reviews.apache.org/r/5532/#comment18207>

    Was SetUpTestCase not protected?


- Benjamin Hindman


On June 22, 2012, 9:51 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5532/
> -----------------------------------------------------------
> 
> (Updated June 22, 2012, 9:51 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> Added tests for allocators.
> 
> This patch depends on two other currently pending review requests:
> https://reviews.apache.org/r/5448/
> https://reviews.apache.org/r/5451/
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 8760f59 
>   src/tests/allocator_tests.cpp PRE-CREATION 
>   src/tests/base_zookeeper_test.hpp 4613376 
>   src/tests/resource_offers_tests.cpp c1f1760 
>   src/tests/utils.hpp e81ec82 
> 
> Diff: https://reviews.apache.org/r/5532/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Allocator tests

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/#review8993
-----------------------------------------------------------



src/tests/utils.hpp
<https://reviews.apache.org/r/5532/#comment19117>

    This is so cool!



src/tests/utils.hpp
<https://reviews.apache.org/r/5532/#comment19119>

    s/DeclineOffer/DeclineOffers/


- Benjamin Hindman


On July 9, 2012, 9:25 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5532/
> -----------------------------------------------------------
> 
> (Updated July 9, 2012, 9:25 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> Added tests for allocators.
> 
> This patch depends on two other currently pending review requests:
> https://reviews.apache.org/r/5448/
> https://reviews.apache.org/r/5451/
> 
> 
> This addresses bug MESOS-224.
>     https://issues.apache.org/jira/browse/MESOS-224
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 8760f59 
>   src/common/type_utils.hpp acf3e65 
>   src/tests/allocator_tests.cpp PRE-CREATION 
>   src/tests/resource_offers_tests.cpp c1f1760 
>   src/tests/utils.hpp e81ec82 
> 
> Diff: https://reviews.apache.org/r/5532/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated July 9, 2012, 11:55 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Updated for the latest code review.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


This addresses bug MESOS-224.
    https://issues.apache.org/jira/browse/MESOS-224


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/common/type_utils.hpp acf3e65 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated July 9, 2012, 9:25 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Reduced code duplication.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


This addresses bug MESOS-224.
    https://issues.apache.org/jira/browse/MESOS-224


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/common/type_utils.hpp acf3e65 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated July 2, 2012, 10:32 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Updated to reflect the recommendations from the last code review.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


This addresses bug MESOS-224.
    https://issues.apache.org/jira/browse/MESOS-224


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/common/type_utils.hpp acf3e65 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.

> On July 2, 2012, 4:54 a.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 685
> > <https://reviews.apache.org/r/5532/diff/4/?file=117150#file117150line685>
> >
> >     Sometimes you do .WillRepeatedly(Return()) and sometimes you do .Times(AnyNumber()). Are there semantic differences that keep you from being consistent and just picking one?

The difference is that WillRepeatedly(Return()) won't do the default whereas Times(AnyNumber()) will. I'll change all of the Times(AnyNumber()) to WillRepeatedly(DoDefault()) to make this clearer.


> On July 2, 2012, 4:54 a.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 608
> > <https://reviews.apache.org/r/5532/diff/4/?file=117150#file117150line608>
> >
> >     So, I think the right thing to do here is make this protected not public. In addition, I think you want to add a static void SetUpTestCase() here that just calls BaseZooKeeperTest::SetUpTestCase(), and make BaseZooKeeperTest::SetUpTestCase be protected again.

Apparently SetUpTestCase must be public for TYPED_TESTs (like not putting DoDefaults in DoAlls, it's for "technical reasons"), but at the very least I can make BaseZooKeeperTest::SetUpTestCase protected by declaring a public MasterFailoverAllocatorTest::SetUpTestCase which calls it.


> On July 2, 2012, 4:54 a.m., Benjamin Hindman wrote:
> > src/tests/allocator_tests.cpp, line 393
> > <https://reviews.apache.org/r/5532/diff/4/?file=117150#file117150line393>
> >
> >     Is this Return() necessary?

As far as I can tell, the compiler doesn't recognize DoAll unless it contains at least one predefined gmock action, as opposed to custom actions we defined. I can't find any documentation on this, but I've asked the Google Mock Group about it, so I'll leave it in for now and fix it later if they get back to me with a solution.


- Thomas


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


On June 28, 2012, 10:19 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5532/
> -----------------------------------------------------------
> 
> (Updated June 28, 2012, 10:19 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> Added tests for allocators.
> 
> This patch depends on two other currently pending review requests:
> https://reviews.apache.org/r/5448/
> https://reviews.apache.org/r/5451/
> 
> 
> This addresses bug MESOS-224.
>     https://issues.apache.org/jira/browse/MESOS-224
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 8760f59 
>   src/common/type_utils.hpp acf3e65 
>   src/tests/allocator_tests.cpp PRE-CREATION 
>   src/tests/base_zookeeper_test.hpp 4613376 
>   src/tests/resource_offers_tests.cpp c1f1760 
>   src/tests/utils.hpp e81ec82 
>   src/tests/utils.cpp e7cda40 
> 
> Diff: https://reviews.apache.org/r/5532/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Allocator tests

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/#review8779
-----------------------------------------------------------


This is cleaning up really nicely!


src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18570>

    Is this Return() necessary?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18571>

    Is this Return() necessary?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18564>

    Hmm, I didn't fully get this comment. I think you want to say that the first time you _don't_ invoke resourcesRecovered, but you _wait_ until frameworkRemoved is called to test that doing the dispatch after frameworkRemoved is okay (more or less parroting what the comment above the test says).



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18566>

    Again, remind the reader of the test that we explicitly do this dispatch on behalf of the master to make sure that the allocator can handle getting resources recovered after the framework has been removed.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18565>

    Wrap this line please.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18568>

    '{' on newline.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18577>

    So, I think the right thing to do here is make this protected not public. In addition, I think you want to add a static void SetUpTestCase() here that just calls BaseZooKeeperTest::SetUpTestCase(), and make BaseZooKeeperTest::SetUpTestCase be protected again.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18567>

    '{' on new line.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18569>

    Please use braces.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18572>

    Please use braces.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18573>

    Is .Times(2) necessary if you have two WillOnce?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18574>

    Sometimes you do .WillRepeatedly(Return()) and sometimes you do .Times(AnyNumber()). Are there semantic differences that keep you from being consistent and just picking one?



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18575>

    Braces please. But instead of this if check (and the ones above), please turn these all into CHECK(!someTry.isError()) << "error message" << someTry.error();



src/tests/utils.hpp
<https://reviews.apache.org/r/5532/#comment18578>

    I love this factored out and useable by others, but I don't love implicitly passing arguments through environment variables. How hard would it be to explicitly pass the arguments to this function? As in, make the signature something like:
    
    launchTask(SchedulerDriver* driver, const std::vector<Offer>& offers, double cpus, double mem)
    
    And maybe even take the total number of tasks to launch?



src/tests/utils.hpp
<https://reviews.apache.org/r/5532/#comment18576>

    driver->declineOffer(offer.id());


- Benjamin Hindman


On June 28, 2012, 10:19 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5532/
> -----------------------------------------------------------
> 
> (Updated June 28, 2012, 10:19 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> Added tests for allocators.
> 
> This patch depends on two other currently pending review requests:
> https://reviews.apache.org/r/5448/
> https://reviews.apache.org/r/5451/
> 
> 
> This addresses bug MESOS-224.
>     https://issues.apache.org/jira/browse/MESOS-224
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 8760f59 
>   src/common/type_utils.hpp acf3e65 
>   src/tests/allocator_tests.cpp PRE-CREATION 
>   src/tests/base_zookeeper_test.hpp 4613376 
>   src/tests/resource_offers_tests.cpp c1f1760 
>   src/tests/utils.hpp e81ec82 
>   src/tests/utils.cpp e7cda40 
> 
> Diff: https://reviews.apache.org/r/5532/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated June 28, 2012, 10:19 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Added comments, rearranged EXPECT_CALLs, and broke up WillOnce chains to make the intent of the tests clearer.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


This addresses bug MESOS-224.
    https://issues.apache.org/jira/browse/MESOS-224


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/common/type_utils.hpp acf3e65 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/base_zookeeper_test.hpp 4613376 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 
  src/tests/utils.cpp e7cda40 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/#review8667
-----------------------------------------------------------



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18330>

    '{' on newline please.



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/5532/#comment18329>

    '{' on newline please.



src/tests/utils.hpp
<https://reviews.apache.org/r/5532/#comment18327>

    s/_)/_))/



src/tests/utils.hpp
<https://reviews.apache.org/r/5532/#comment18328>

    const & instead of just const.


- Benjamin Hindman


On June 27, 2012, 7:55 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5532/
> -----------------------------------------------------------
> 
> (Updated June 27, 2012, 7:55 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Description
> -------
> 
> Added tests for allocators.
> 
> This patch depends on two other currently pending review requests:
> https://reviews.apache.org/r/5448/
> https://reviews.apache.org/r/5451/
> 
> 
> This addresses bug MESOS-224.
>     https://issues.apache.org/jira/browse/MESOS-224
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 8760f59 
>   src/tests/allocator_tests.cpp PRE-CREATION 
>   src/tests/base_zookeeper_test.hpp 4613376 
>   src/tests/resource_offers_tests.cpp c1f1760 
>   src/tests/utils.hpp e81ec82 
>   src/tests/utils.cpp e7cda40 
> 
> Diff: https://reviews.apache.org/r/5532/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated June 27, 2012, 7:55 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Removed test which tested an impossible situation (resourcesUnused called for already removed framework).


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


This addresses bug MESOS-224.
    https://issues.apache.org/jira/browse/MESOS-224


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/base_zookeeper_test.hpp 4613376 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 
  src/tests/utils.cpp e7cda40 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated June 26, 2012, 7:10 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

I created a Jira issue to track progress on the allocator work.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


This addresses bug MESOS-224.
    https://issues.apache.org/jira/browse/MESOS-224


Diffs
-----

  src/Makefile.am 8760f59 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/base_zookeeper_test.hpp 4613376 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 
  src/tests/utils.cpp e7cda40 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall


Re: Review Request: Allocator tests

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5532/
-----------------------------------------------------------

(Updated June 26, 2012, 7:01 p.m.)


Review request for mesos and Benjamin Hindman.


Description
-------

Added tests for allocators.

This patch depends on two other currently pending review requests:
https://reviews.apache.org/r/5448/
https://reviews.apache.org/r/5451/


Diffs (updated)
-----

  src/Makefile.am 8760f59 
  src/tests/allocator_tests.cpp PRE-CREATION 
  src/tests/base_zookeeper_test.hpp 4613376 
  src/tests/resource_offers_tests.cpp c1f1760 
  src/tests/utils.hpp e81ec82 
  src/tests/utils.cpp e7cda40 

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


Testing
-------

make check on Lion


Thanks,

Thomas Marshall