You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Isabel Jimenez <co...@isabeljimenez.com> on 2015/09/29 10:07:16 UTC

Review Request 38844: Added unit tests for Call validation in Agent

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs
-----

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.

> On Sept. 29, 2015, 8:11 p.m., Anand Mazumdar wrote:
> > Can you also update the new tests to use the updated way to test recovery of Agent as done by the other tests ? ( Otherwise, these would be flaky )

Done :) Thanks


- Isabel


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


On Sept. 29, 2015, 8:25 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2015, 8:25 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review101001
-----------------------------------------------------------


Can you also update the new tests to use the updated way to test recovery of Agent as done by the other tests ? ( Otherwise, these would be flaky )

- Anand Mazumdar


On Sept. 29, 2015, 8:05 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2015, 8:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review101073
-----------------------------------------------------------


LGTM ! Just some minor comments to clean up the tests around parameterizing/scoping.


src/tests/executor_http_api_tests.cpp (line 450)
<https://reviews.apache.org/r/38844/#comment158380>

    s/TEST_F/TEST_P
    
    Is there a reason why this test is not parameterized ?



src/tests/executor_http_api_tests.cpp (line 468)
<https://reviews.apache.org/r/38844/#comment158381>

    



src/tests/executor_http_api_tests.cpp (lines 468 - 483)
<https://reviews.apache.org/r/38844/#comment158393>

    s/responseSubscribe/response
    
    Can we use scoping here for each of the 3 different scenarios :
    
    {
      Call call;
      blah blah;
    }
    
    {
      Call call;
      blah blah;
    }
    ...



src/tests/executor_http_api_tests.cpp (line 515)
<https://reviews.apache.org/r/38844/#comment158385>

    s/TEST_F/TEST_P and make this test parametrized. Is there a reason why this is not ?
    
    s/InvalidCallUpdate/StatusUpdateCallFailedValidation to be more descriptive



src/tests/executor_http_api_tests.cpp (line 531)
<https://reviews.apache.org/r/38844/#comment158389>

    // We send a valid Call::Update message with inconsistent executor_id between Call::executor_id and Call::Update::TaskInfo::executor_id. This should result in failed validation.
    
    ^^ How does this sound to you ?



src/tests/executor_http_api_tests.cpp (line 561)
<https://reviews.apache.org/r/38844/#comment158390>

    Nit: // We send a valid Call::Update message with a TASK_STAGING status update. This should fail validation.
    
    (Mainly because after parameterizing the test this won't always be a protobuf message.)



src/tests/executor_http_api_tests.cpp (lines 563 - 573)
<https://reviews.apache.org/r/38844/#comment158391>

    Scope this similar to my earlier comment:
    
    {
      blah blah;
    }


- Anand Mazumdar


On Sept. 29, 2015, 9:27 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review101048
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Sept. 29, 2015, 9:27 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review101366
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Oct. 2, 2015, 6:53 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review104125
-----------------------------------------------------------

Ship it!



src/tests/executor_http_api_tests.cpp (line 543)
<https://reviews.apache.org/r/38844/#comment162376>

    s/responseSubscribe/response/
    
    remove the suffices for response here and everywhere else below.



src/tests/executor_http_api_tests.cpp (line 562)
<https://reviews.apache.org/r/38844/#comment162377>

    ditto.
    
    s/responseUpdate/response/



src/tests/executor_http_api_tests.cpp (line 581)
<https://reviews.apache.org/r/38844/#comment162378>

    ditto. just call it 'response'



src/tests/executor_http_api_tests.cpp (line 611)
<https://reviews.apache.org/r/38844/#comment162375>

    s/valid//
    
    s/executor_id/executor id/



src/tests/executor_http_api_tests.cpp (line 632)
<https://reviews.apache.org/r/38844/#comment162379>

    ditto. just call it 'response'



src/tests/executor_http_api_tests.cpp (line 642)
<https://reviews.apache.org/r/38844/#comment162380>

    s/valid//



src/tests/executor_http_api_tests.cpp (line 693)
<https://reviews.apache.org/r/38844/#comment162381>

    ditto. just call it 'response'


- Vinod Kone


On Oct. 27, 2015, 12:37 a.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2015, 12:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp e429d84 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review104132
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38618, 38577, 38844]

All tests passed.

- Mesos ReviewBot


On Oct. 27, 2015, 1:30 a.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2015, 1:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp e429d84 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Oct. 27, 2015, 1:30 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

changes after review comments


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs (updated)
-----

  src/tests/executor_http_api_tests.cpp e429d84 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Oct. 27, 2015, 12:37 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

rebase


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs (updated)
-----

  src/tests/executor_http_api_tests.cpp e429d84 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/#review101384
-----------------------------------------------------------

Ship it!


LGTM

- Anand Mazumdar


On Oct. 2, 2015, 6:53 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38844/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2015, 6:53 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-2906
>     https://issues.apache.org/jira/browse/MESOS-2906
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Unit tests for Call validation in Agent.
> 
> 
> Diffs
> -----
> 
>   src/tests/executor_http_api_tests.cpp c2c05f4 
> 
> Diff: https://reviews.apache.org/r/38844/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Oct. 2, 2015, 6:53 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs (updated)
-----

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Sept. 29, 2015, 9:27 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

Adding test for invalid status update 'TASK_STAGING'


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs (updated)
-----

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Sept. 29, 2015, 8:25 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs (updated)
-----

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Sept. 29, 2015, 8:05 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
-------

rebase


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs (updated)
-----

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
-------

make check


Thanks,

Isabel Jimenez


Re: Review Request 38844: Added unit tests for Call validation in Agent

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38844/
-----------------------------------------------------------

(Updated Sept. 29, 2015, 8:08 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
-------

Unit tests for Call validation in Agent.


Diffs
-----

  src/tests/executor_http_api_tests.cpp c2c05f4 

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


Testing
-------

make check


Thanks,

Isabel Jimenez