You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2013/04/15 04:28:10 UTC

Review Request: Renamed and improved testing abstractions for awaiting on futures.

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

Review request for mesos, Vinod Kone and Ben Mahler.


Description
-------

See summary.


Diffs
-----

  third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
  third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
  third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
  third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 

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


Testing
-------

make check


Thanks,

Benjamin Hindman

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10460/#review19197
-----------------------------------------------------------

Ship it!


Ship It!

- Ben Mahler


On April 15, 2013, 2:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10460/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 2:28 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
>   third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
>   third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
>   third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 
> 
> Diff: https://reviews.apache.org/r/10460/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On April 15, 2013, 3:03 a.m., Ben Mahler wrote:
> > third_party/libprocess/include/process/gtest.hpp, line 144
> > <https://reviews.apache.org/r/10460/diff/1/?file=281109#file281109line144>
> >
> >     Can these be overloaded instead of having the explicit _FOR suffixes?

Not that I know of.


- Benjamin


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


On April 15, 2013, 2:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10460/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 2:28 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
>   third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
>   third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
>   third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 
> 
> Diff: https://reviews.apache.org/r/10460/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10460/#review19162
-----------------------------------------------------------



third_party/libprocess/include/process/gtest.hpp
<https://reviews.apache.org/r/10460/#comment39737>

    Can these be overloaded instead of having the explicit _FOR suffixes?


- Ben Mahler


On April 15, 2013, 2:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10460/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 2:28 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
>   third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
>   third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
>   third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 
> 
> Diff: https://reviews.apache.org/r/10460/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10460/
-----------------------------------------------------------

(Updated April 24, 2013, 7:46 p.m.)


Review request for mesos, Vinod Kone and Ben Mahler.


Changes
-------

Rebased.


Description
-------

See summary.


Diffs (updated)
-----

  third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
  third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
  third_party/libprocess/src/tests/process_tests.cpp 83a38e1a3aafda981718506b1bf748329233c9d5 
  third_party/libprocess/src/tests/statistics_tests.cpp bb7a94757eddc522f1ab444ee12c9f237e2042a5 

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


Testing
-------

make check


Thanks,

Benjamin Hindman

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

Posted by Ben Mahler <be...@gmail.com>.

> On April 15, 2013, 2:52 a.m., Vinod Kone wrote:
> >

We could also go with:

ASSERT_WILL_FAIL
ASSERT_WILL_READY


- Ben


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


On April 15, 2013, 2:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10460/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 2:28 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
>   third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
>   third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
>   third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 
> 
> Diff: https://reviews.apache.org/r/10460/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On April 15, 2013, 2:52 a.m., Vinod Kone wrote:
> >
> 
> Ben Mahler wrote:
>     We could also go with:
>     
>     ASSERT_WILL_FAIL
>     ASSERT_WILL_READY

I actually really liked the use of AWAIT. It's very clear ... the test is blocking here until the future is "satisfied". If you want to be explicit, you can do AWAIT_ASSERT_READY or AWAIT_EXPECT_READY (s/READY/FAILED/ and s/READY/DISCARDED/ too). Most of the time, however, you want assert because you want to check the value (via get()) or the failure (via failure()), so there are abbreviated versions of those. In the worst case scenario using AWAIT_READY where a user preferred AWAIT_EXPECT_READY is really not a big deal ... it's a test failure either way and needs to be dealt with (I have yet to see a case where using EXPECT has let more of the test run enabling us to fix more than one test bug at a time).


> On April 15, 2013, 2:52 a.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/gtest.hpp, line 148
> > <https://reviews.apache.org/r/10460/diff/1/?file=281109#file281109line148>
> >
> >     I know I called this AWAIT_READY. But looking at how you renamed things, I would prefer
> >     
> >     ASSERT_READY()
> >     
> >     This tells me you are doing an assert.
> >     
> >     That way you can also have EXPECT_READY(), which you are missing.

EXPECT_READY is not missing, it's just AWAIT_EXPECT_READY.


- Benjamin


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


On April 15, 2013, 2:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10460/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 2:28 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
>   third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
>   third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
>   third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 
> 
> Diff: https://reviews.apache.org/r/10460/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request: Renamed and improved testing abstractions for awaiting on futures.

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



third_party/libprocess/include/process/gtest.hpp
<https://reviews.apache.org/r/10460/#comment39733>

    I know I called this AWAIT_READY. But looking at how you renamed things, I would prefer
    
    ASSERT_READY()
    
    This tells me you are doing an assert.
    
    That way you can also have EXPECT_READY(), which you are missing.



third_party/libprocess/include/process/gtest.hpp
<https://reviews.apache.org/r/10460/#comment39734>

    ditto. s/AWAIT_FAILED/ASSERT_FAILED



third_party/libprocess/include/process/gtest.hpp
<https://reviews.apache.org/r/10460/#comment39735>

    ditto



third_party/libprocess/src/tests/process_tests.cpp
<https://reviews.apache.org/r/10460/#comment39732>

    How about just EXPECT_READY?



third_party/libprocess/src/tests/statistics_tests.cpp
<https://reviews.apache.org/r/10460/#comment39731>

    I think these read better as ASSERT_READY()? Why AWAIT_ASSERT_READY?


- Vinod Kone


On April 15, 2013, 2:28 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10460/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 2:28 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   third_party/libprocess/include/process/gtest.hpp 80d5194489bc8f5d4897a075e655d543480e314e 
>   third_party/libprocess/src/tests/io_tests.cpp 9115a968b81739c33afe98b673797ca472cc8b87 
>   third_party/libprocess/src/tests/process_tests.cpp 562f52c6833183e6e4e81f6ae307cf28e40404b3 
>   third_party/libprocess/src/tests/statistics_tests.cpp 6efcc8004fc637e5c4dc8f0ba0e34c0a06a8c052 
> 
> Diff: https://reviews.apache.org/r/10460/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>