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/03/28 23:06:57 UTC
Review Request: Refactored garbage collector tests to use EXPECT_DISPATCH
instead of sleep.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10179/
-----------------------------------------------------------
Review request for mesos, Vinod Kone and Ben Mahler.
Description
-------
It probably makes sense to create tickets to replace existing sleep calls in other tests to use EXPECT_DISPATCH.
Diffs
-----
src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
src/tests/environment.cpp fd618787916a3d88eecf72c51a755b05c79eef59
src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
Diff: https://reviews.apache.org/r/10179/diff/
Testing
-------
# Check it:
./bin/mesos-tests.sh --gtest_filter=Garbage* --gtest_repeat=1000 --gtest_shuffle
Thanks,
Benjamin Hindman
Re: Review Request: Refactored garbage collector tests to use
EXPECT_DISPATCH instead of sleep.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 29, 2013, 7:38 p.m., Ben Mahler wrote:
> > src/slave/gc.hpp, line 104
> > <https://reviews.apache.org/r/10179/diff/1/?file=276153#file276153line104>
> >
> > Why are these together? I thought we placed all variables below functions?
I just copied this from gc.cpp. I think your upcoming changes muck with this, so I'll just let it be.
> On March 29, 2013, 7:38 p.m., Ben Mahler wrote:
> > src/tests/gc_tests.cpp, line 245
> > <https://reviews.apache.org/r/10179/diff/1/?file=276156#file276156line245>
> >
> > Should there be a TODO on these to be using a non-clock settle?
No, the clock is paused. Unless I'm missing something?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10179/#review18528
-----------------------------------------------------------
On March 28, 2013, 10:06 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10179/
> -----------------------------------------------------------
>
> (Updated March 28, 2013, 10:06 p.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> It probably makes sense to create tickets to replace existing sleep calls in other tests to use EXPECT_DISPATCH.
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/environment.cpp fd618787916a3d88eecf72c51a755b05c79eef59
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10179/diff/
>
>
> Testing
> -------
>
> # Check it:
> ./bin/mesos-tests.sh --gtest_filter=Garbage* --gtest_repeat=1000 --gtest_shuffle
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored garbage collector tests to use
EXPECT_DISPATCH instead of sleep.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10179/#review18528
-----------------------------------------------------------
Ship it!
src/slave/gc.hpp
<https://reviews.apache.org/r/10179/#comment38836>
Seems like wrapping at the ( would have sufficed.
src/slave/gc.hpp
<https://reviews.apache.org/r/10179/#comment38837>
Why are these together? I thought we placed all variables below functions?
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10179/#comment38839>
Should there be a TODO on these to be using a non-clock settle?
- Ben Mahler
On March 28, 2013, 10:06 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10179/
> -----------------------------------------------------------
>
> (Updated March 28, 2013, 10:06 p.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> It probably makes sense to create tickets to replace existing sleep calls in other tests to use EXPECT_DISPATCH.
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/environment.cpp fd618787916a3d88eecf72c51a755b05c79eef59
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10179/diff/
>
>
> Testing
> -------
>
> # Check it:
> ./bin/mesos-tests.sh --gtest_filter=Garbage* --gtest_repeat=1000 --gtest_shuffle
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored garbage collector tests to use
EXPECT_DISPATCH instead of sleep.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 29, 2013, 1:07 a.m., Vinod Kone wrote:
> > src/tests/environment.cpp, line 139
> > <https://reviews.apache.org/r/10179/diff/1/?file=276155#file276155line139>
> >
> > why did you kill clearing mesos env variables?
I'm not.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10179/#review18489
-----------------------------------------------------------
On March 28, 2013, 10:06 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10179/
> -----------------------------------------------------------
>
> (Updated March 28, 2013, 10:06 p.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> It probably makes sense to create tickets to replace existing sleep calls in other tests to use EXPECT_DISPATCH.
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/environment.cpp fd618787916a3d88eecf72c51a755b05c79eef59
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10179/diff/
>
>
> Testing
> -------
>
> # Check it:
> ./bin/mesos-tests.sh --gtest_filter=Garbage* --gtest_repeat=1000 --gtest_shuffle
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored garbage collector tests to use
EXPECT_DISPATCH instead of sleep.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10179/#review18489
-----------------------------------------------------------
Ship it!
src/slave/gc.hpp
<https://reviews.apache.org/r/10179/#comment38783>
Moving this into .hpp is kinda unfortunate, because it is breaking the pimpl idiom. oh well.
src/tests/environment.cpp
<https://reviews.apache.org/r/10179/#comment38780>
why did you kill clearing mesos env variables?
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10179/#comment38781>
s/Repeatedly/Once/
Lets be as strict as we can.
src/tests/gc_tests.cpp
<https://reviews.apache.org/r/10179/#comment38782>
aha, so we can match on any pid. you can ignore, my comment from the previous review then.
- Vinod Kone
On March 28, 2013, 10:06 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10179/
> -----------------------------------------------------------
>
> (Updated March 28, 2013, 10:06 p.m.)
>
>
> Review request for mesos, Vinod Kone and Ben Mahler.
>
>
> Description
> -------
>
> It probably makes sense to create tickets to replace existing sleep calls in other tests to use EXPECT_DISPATCH.
>
>
> Diffs
> -----
>
> src/slave/gc.hpp c5cceb13b907c222fc177f1bdeeb838ba8efcc4e
> src/slave/gc.cpp 92ea79c126f0852a9aeaacd51e7261962f56b8d1
> src/tests/environment.cpp fd618787916a3d88eecf72c51a755b05c79eef59
> src/tests/gc_tests.cpp 67bf957c2ed61daefcd3baf2ee7aa4abaa524eab
>
> Diff: https://reviews.apache.org/r/10179/diff/
>
>
> Testing
> -------
>
> # Check it:
> ./bin/mesos-tests.sh --gtest_filter=Garbage* --gtest_repeat=1000 --gtest_shuffle
>
>
> Thanks,
>
> Benjamin Hindman
>
>