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/06/20 22:51:07 UTC

Review Request 35703: Set refuse seconds on the correct filter in reservation test.

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

Review request for mesos, Alexander Rukletsov and Jie Yu.


Repository: mesos


Description
-------

`set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.


Diffs
-----

  src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 

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


Testing
-------

`make check`


Thanks,

Michael Park

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35703/#review89075
-----------------------------------------------------------

Ship it!


Ship It!

- Till Toenshoff


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

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


Patch looks great!

Reviews applied: [35703]

All tests passed.

- Mesos ReviewBot


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

Posted by Zhiwei Chen <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35703/#review89125
-----------------------------------------------------------

Ship it!


Thanks.

- Zhiwei Chen


On June 21, 2015, 4:51 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 21, 2015, 4:51 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote:
> > Looks like a copy-paste bug. How have you found it? Was the test flaky?
> 
> Michael Park wrote:
>     I was using these tests as a reference for writing my new tests for [r35702](https://reviews.apache.org/r/35702/) and noticed it.

I wonder why they were not flaky.


- Alexander


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


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

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

> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote:
> > Looks like a copy-paste bug. How have you found it? Was the test flaky?
> 
> Michael Park wrote:
>     I was using these tests as a reference for writing my new tests for [r35702](https://reviews.apache.org/r/35702/) and noticed it.
> 
> Alexander Rukletsov wrote:
>     I wonder why they were not flaky.

Because `filtersForever` gets default-constructed, it gets initialized with the default value of 5 seconds. We set it to `std::numeric_limits<double>::max()` to be certain + documentation. Practically, since the test finishes within a second, 5 seconds was "forever" enough.


- Michael


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


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

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

> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote:
> > Looks like a copy-paste bug. How have you found it? Was the test flaky?

I was using these tests as a reference for writing my new tests for [r35702](https://reviews.apache.org/r/35702/) and noticed it.


- Michael


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


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote:
> > Looks like a copy-paste bug. How have you found it? Was the test flaky?
> 
> Michael Park wrote:
>     I was using these tests as a reference for writing my new tests for [r35702](https://reviews.apache.org/r/35702/) and noticed it.
> 
> Alexander Rukletsov wrote:
>     I wonder why they were not flaky.
> 
> Michael Park wrote:
>     Because `filtersForever` gets default-constructed, it gets initialized with the default value of 5 seconds. We set it to `std::numeric_limits<double>::max()` to be certain + documentation. Practically, since the test finishes within a second, 5 seconds was "forever" enough.

Makes sense. Thanks for the explanation!


- Alexander


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


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

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

Ship it!


Looks like a copy-paste bug. How have you found it? Was the test flaky?

- Alexander Rukletsov


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> -----------------------------------------------------------
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than `filters`.
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>