You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/05/03 12:08:35 UTC

Review Request 58950: WIP: Made persistent volume endpoints tests independent from allocator.

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

Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


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


Repository: mesos


Description
-------

The tests in the case often require an agent ID. To obtain the ID they
were using a mock allocator to grab agent ID, but not other operations
with the allocator were performed.

Instead we now just capture the SlaveRegisteredMessage in order to
learn an agent's ID.


Diffs
-----

  src/tests/persistent_volume_endpoints_tests.cpp 1237d081d781948975f66a8240ae9bdb5e819a3b 


Diff: https://reviews.apache.org/r/58950/diff/1/


Testing
-------

`make check`, also in repetition.

While this patch is part of this series since later patches depend on it, it could be commited independently.


Thanks,

Benjamin Bannier


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

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



Bad patch!

Reviews applied: [58950]

Failed command: python support/apply-reviews.py -n -r 58950

Error:
error: patch failed: src/tests/persistent_volume_endpoints_tests.cpp:1576
error: src/tests/persistent_volume_endpoints_tests.cpp: patch does not apply

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/59/console

- Mesos Reviewbot Windows


On June 6, 2017, 12:38 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58950/
> -----------------------------------------------------------
> 
> (Updated June 6, 2017, 12:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7388
>     https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The tests in the case often require an agent ID. To obtain the ID they
> were using a mock allocator to grab agent ID, but not other operations
> with the allocator were performed.
> 
> Instead we now just capture the SlaveRegisteredMessage in order to
> learn an agent's ID.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 8d0a7970ac03d0dbff6acccce894b922951d4e5d 
> 
> 
> Diff: https://reviews.apache.org/r/58950/diff/3/
> 
> 
> Testing
> -------
> 
> `make check`, also in repetition.
> 
> While this patch is part of this series since later patches depend on it, it could be commited independently.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58950/#review177157
-----------------------------------------------------------


Ship it!




Ship It!

- Alexander Rukletsov


On June 6, 2017, 12:38 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58950/
> -----------------------------------------------------------
> 
> (Updated June 6, 2017, 12:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7388
>     https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The tests in the case often require an agent ID. To obtain the ID they
> were using a mock allocator to grab agent ID, but not other operations
> with the allocator were performed.
> 
> Instead we now just capture the SlaveRegisteredMessage in order to
> learn an agent's ID.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 8d0a7970ac03d0dbff6acccce894b922951d4e5d 
> 
> 
> Diff: https://reviews.apache.org/r/58950/diff/3/
> 
> 
> Testing
> -------
> 
> `make check`, also in repetition.
> 
> While this patch is part of this series since later patches depend on it, it could be commited independently.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58950/
-----------------------------------------------------------

(Updated June 6, 2017, 2:38 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
-------

Addressed comments from alexr.


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


Repository: mesos


Description
-------

The tests in the case often require an agent ID. To obtain the ID they
were using a mock allocator to grab agent ID, but not other operations
with the allocator were performed.

Instead we now just capture the SlaveRegisteredMessage in order to
learn an agent's ID.


Diffs (updated)
-----

  src/tests/persistent_volume_endpoints_tests.cpp 8d0a7970ac03d0dbff6acccce894b922951d4e5d 


Diff: https://reviews.apache.org/r/58950/diff/3/

Changes: https://reviews.apache.org/r/58950/diff/2-3/


Testing
-------

`make check`, also in repetition.

While this patch is part of this series since later patches depend on it, it could be commited independently.


Thanks,

Benjamin Bannier


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

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



Patch looks great!

Reviews applied: [58950]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 30, 2017, 10:22 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58950/
> -----------------------------------------------------------
> 
> (Updated May 30, 2017, 10:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7388
>     https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The tests in the case often require an agent ID. To obtain the ID they
> were using a mock allocator to grab agent ID, but not other operations
> with the allocator were performed.
> 
> Instead we now just capture the SlaveRegisteredMessage in order to
> learn an agent's ID.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668 
> 
> 
> Diff: https://reviews.apache.org/r/58950/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`, also in repetition.
> 
> While this patch is part of this series since later patches depend on it, it could be commited independently.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58950/#review176743
-----------------------------------------------------------




src/tests/persistent_volume_endpoints_tests.cpp
Lines 827 (patched)
<https://reviews.apache.org/r/58950/#comment250206>

    Let's remove this.



src/tests/persistent_volume_endpoints_tests.cpp
Line 1067 (original), 1032 (patched)
<https://reviews.apache.org/r/58950/#comment250207>

    s/befor/before



src/tests/persistent_volume_endpoints_tests.cpp
Lines 1034 (patched)
<https://reviews.apache.org/r/58950/#comment250208>

    No need for this.



src/tests/persistent_volume_endpoints_tests.cpp
Line 1286 (original), 1249 (patched)
<https://reviews.apache.org/r/58950/#comment250209>

    s/befor/before/



src/tests/persistent_volume_endpoints_tests.cpp
Lines 1251 (patched)
<https://reviews.apache.org/r/58950/#comment250210>

    Let's not call it explicitly.



src/tests/persistent_volume_endpoints_tests.cpp
Lines 1848-1849 (original), 1791-1793 (patched)
<https://reviews.apache.org/r/58950/#comment250211>

    I believe `slaveId1` is more common in our code base. Feel free to change it, preferably in a separate patch : )


- Alexander Rukletsov


On May 30, 2017, 10:22 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58950/
> -----------------------------------------------------------
> 
> (Updated May 30, 2017, 10:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7388
>     https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The tests in the case often require an agent ID. To obtain the ID they
> were using a mock allocator to grab agent ID, but not other operations
> with the allocator were performed.
> 
> Instead we now just capture the SlaveRegisteredMessage in order to
> learn an agent's ID.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668 
> 
> 
> Diff: https://reviews.apache.org/r/58950/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`, also in repetition.
> 
> While this patch is part of this series since later patches depend on it, it could be commited independently.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

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



Patch looks great!

Reviews applied: [58950]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 30, 2017, 10:22 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58950/
> -----------------------------------------------------------
> 
> (Updated May 30, 2017, 10:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7388
>     https://issues.apache.org/jira/browse/MESOS-7388
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The tests in the case often require an agent ID. To obtain the ID they
> were using a mock allocator to grab agent ID, but not other operations
> with the allocator were performed.
> 
> Instead we now just capture the SlaveRegisteredMessage in order to
> learn an agent's ID.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668 
> 
> 
> Diff: https://reviews.apache.org/r/58950/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`, also in repetition.
> 
> While this patch is part of this series since later patches depend on it, it could be commited independently.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Re: Review Request 58950: Made persistent volume endpoints tests independent from allocator.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58950/
-----------------------------------------------------------

(Updated May 30, 2017, 12:22 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
-------

Rebased.


Summary (updated)
-----------------

Made persistent volume endpoints tests independent from allocator.


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


Repository: mesos


Description
-------

The tests in the case often require an agent ID. To obtain the ID they
were using a mock allocator to grab agent ID, but not other operations
with the allocator were performed.

Instead we now just capture the SlaveRegisteredMessage in order to
learn an agent's ID.


Diffs (updated)
-----

  src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668 


Diff: https://reviews.apache.org/r/58950/diff/2/

Changes: https://reviews.apache.org/r/58950/diff/1-2/


Testing
-------

`make check`, also in repetition.

While this patch is part of this series since later patches depend on it, it could be commited independently.


Thanks,

Benjamin Bannier