You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mc...@gmail.com> on 2015/07/31 23:57:28 UTC

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

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

(Updated July 31, 2015, 9:57 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

See summary


Diffs (updated)
-----

  src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
  src/tests/reservation_master_endpoints_tests.cpp PRE-CREATION 

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


Testing
-------

(1) Added `src/tests/reservation_master_endpoints_tests.cpp`
(2) `make check`


Thanks,

Michael Park

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

Posted by Michael Park <mc...@gmail.com>.

> On Aug. 26, 2015, 6:33 a.m., Jie Yu wrote:
> > src/tests/reservation_endpoints_tests.cpp, line 109
> > <https://reviews.apache.org/r/35984/diff/4/?file=1032440#file1032440line109>
> >
> >     Are we allowed to use cxx11 raw string literals now? This is definitely better!

I found out there are `operator<<` for `JSON::Object` and `JSON::Array`, I think converting `Resources` to `JSON::Array` and invoking `stringify` on it is the right way to do this.


- Michael


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


On Sept. 9, 2015, 10:12 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35984/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 10:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 5fdca0f574e7e08c4b1aebed0fac39140c19adfe 
>   src/tests/reservation_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35984/diff/
> 
> 
> Testing
> -------
> 
> (1) Added `src/tests/reservation_endpoints_tests.cpp`
> (2) `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35984/#review96503
-----------------------------------------------------------

Ship it!


Awesome tests! LGTM!


src/tests/reservation_endpoints_tests.cpp (line 19)
<https://reviews.apache.org/r/35984/#comment151905>

    This is C++ header. Please put it after <string> and <vector>



src/tests/reservation_endpoints_tests.cpp (lines 103 - 132)
<https://reviews.apache.org/r/35984/#comment151908>

    This piece of code is duplicated in many tests here. Could you please pull them into a helper method in the test fixture. For instance
    
    ```
    createBasicAuthHeader();
    createResourcesParameter(...);
    ```



src/tests/reservation_endpoints_tests.cpp (line 109)
<https://reviews.apache.org/r/35984/#comment151907>

    Are we allowed to use cxx11 raw string literals now? This is definitely better!



src/tests/reservation_endpoints_tests.cpp (line 501)
<https://reviews.apache.org/r/35984/#comment151911>

    Please move this comments right above 'Resources taskResources'.
    
    Also, could you please add a comment about why you want to launch a task here.


- Jie Yu


On Aug. 5, 2015, 9:55 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35984/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/tests/reservation_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35984/diff/
> 
> 
> Testing
> -------
> 
> (1) Added `src/tests/reservation_endpoints_tests.cpp`
> (2) `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35984/
-----------------------------------------------------------

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


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Wrapped comments at 80.


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


Repository: mesos


Description
-------

See summary


Diffs (updated)
-----

  src/Makefile.am 5fdca0f574e7e08c4b1aebed0fac39140c19adfe 
  src/tests/reservation_endpoints_tests.cpp PRE-CREATION 

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


Testing
-------

(1) Added `src/tests/reservation_endpoints_tests.cpp`
(2) `make check`


Thanks,

Michael Park

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35984/
-----------------------------------------------------------

(Updated Sept. 9, 2015, 10:12 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Addressed Jie's comments. Introduced helpers and reduced tests.


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


Repository: mesos


Description
-------

See summary


Diffs (updated)
-----

  src/Makefile.am 5fdca0f574e7e08c4b1aebed0fac39140c19adfe 
  src/tests/reservation_endpoints_tests.cpp PRE-CREATION 

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


Testing
-------

(1) Added `src/tests/reservation_endpoints_tests.cpp`
(2) `make check`


Thanks,

Michael Park

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35984/#review97905
-----------------------------------------------------------



src/tests/reservation_endpoints_tests.cpp (lines 78 - 101)
<https://reviews.apache.org/r/35984/#comment153975>

    This piece of code also has some duplicate, is it possible to add merge to the helper function? Such as a startMaster and startSlave help function or just one helper function as startMesos? Thanks.


- Guangya Liu


On 八月 5, 2015, 9:55 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35984/
> -----------------------------------------------------------
> 
> (Updated 八月 5, 2015, 9:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/tests/reservation_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35984/diff/
> 
> 
> Testing
> -------
> 
> (1) Added `src/tests/reservation_endpoints_tests.cpp`
> (2) `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35984/
-----------------------------------------------------------

(Updated Aug. 5, 2015, 9:55 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.


Changes
-------

renamed `reservation_master_endpoints_tests.cpp` to `reservation_endpoints_tests.cpp`


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


Repository: mesos


Description
-------

See summary


Diffs (updated)
-----

  src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
  src/tests/reservation_endpoints_tests.cpp PRE-CREATION 

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


Testing (updated)
-------

(1) Added `src/tests/reservation_endpoints_tests.cpp`
(2) `make check`


Thanks,

Michael Park

Re: Review Request 35984: Added tests for /reserve and /unreserve HTTP endpoints.

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


Patch looks great!

Reviews applied: [36987, 35702, 35983, 35984]

All tests passed.

- Mesos ReviewBot


On July 31, 2015, 9:57 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35984/
> -----------------------------------------------------------
> 
> (Updated July 31, 2015, 9:57 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2600
>     https://issues.apache.org/jira/browse/MESOS-2600
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
>   src/tests/reservation_master_endpoints_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35984/diff/
> 
> 
> Testing
> -------
> 
> (1) Added `src/tests/reservation_master_endpoints_tests.cpp`
> (2) `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>