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



src/tests/mesos.hpp
<https://reviews.apache.org/r/35157/#comment139349>

    The StartSlave overload is growing a bit out of hand. Let's create a JIRA 
so we get those consolidated somehow. Can you add the ticket as a comment? :)



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139350>

    Can't you compare the message objects instead? If there isn't a == operator 
overload - let's add one :)



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139381>

    Can you inline this below? (Is it used other places?) If not, should we 
const it? Here and below



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139382>

    Have you looked into, whether you can use createTask() instead?
    
    Here and below



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139380>

    Why not await ready? Here and below :)



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139370>

    const?



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139372>

    We tend to use the shorter form, if there is no ambguity (controller 
instead of 'qoSController' :) I'll leave that up to you, if you want to change 
it



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139369>

    Let's introduce a 'using std::list;' above :)



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139375>

    Why copy the offer?



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139378>

    Strip 'std::' :)



src/tests/oversubscription_tests.cpp
<https://reviews.apache.org/r/35157/#comment139377>

    Let's get the operator overload wired up and do ASSERT_EQ() :)


- Niklas Nielsen


On June 8, 2015, 12:41 p.m., Bartek Plotka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35157/
> -----------------------------------------------------------
> 
> (Updated June 8, 2015, 12:41 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage 
> from ResourceMonitor.
> 
> This is the unit test for https://reviews.apache.org/r/34980/ and 
> https://reviews.apache.org/r/35164/
> 
> 
> Diffs
> -----
> 
>   src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 
>   src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a 
>   src/tests/oversubscription_tests.cpp 
> afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 
> 
> Diff: https://reviews.apache.org/r/35157/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Bartek Plotka
> 
>

Reply via email to