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/05/20 23:58:43 UTC
Review Request: Moved 'tests::mkdtemp' to Environment.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11268/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Description
-------
It's possible we should move mkdtemp into MesosTest instead. The benefit will be that we can cleanup temporary directories in ~MesosTest() which would mean that less temporary directories leak when tests fail. I'll defer this for now.
Diffs
-----
src/tests/environment.hpp 61aa84debbf1ad31bc59be9bc91b6babef4881a3
src/tests/environment.cpp c94c85fc2f710dc2157f6b61edb7bfb0df21a579
src/tests/main.cpp e32ec0cac7b7ab9b31f4e419d0c1426b2c8aeb65
src/tests/script.cpp d7f103eb08335638bca57ccab77646534342b058
src/tests/utils.hpp ca3ecd7f0cab283327bf83e57d4b405b4ada9c74
src/tests/utils.cpp 9d1d5ad1e192b0f44a7a7173839b292f6bedad15
Diff: https://reviews.apache.org/r/11268/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request: Moved 'tests::mkdtemp' to Environment.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11268/#review21030
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On May 20, 2013, 9:58 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11268/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 9:58 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Description
> -------
>
> It's possible we should move mkdtemp into MesosTest instead. The benefit will be that we can cleanup temporary directories in ~MesosTest() which would mean that less temporary directories leak when tests fail. I'll defer this for now.
>
>
> Diffs
> -----
>
> src/tests/environment.hpp 61aa84debbf1ad31bc59be9bc91b6babef4881a3
> src/tests/environment.cpp c94c85fc2f710dc2157f6b61edb7bfb0df21a579
> src/tests/main.cpp e32ec0cac7b7ab9b31f4e419d0c1426b2c8aeb65
> src/tests/script.cpp d7f103eb08335638bca57ccab77646534342b058
> src/tests/utils.hpp ca3ecd7f0cab283327bf83e57d4b405b4ada9c74
> src/tests/utils.cpp 9d1d5ad1e192b0f44a7a7173839b292f6bedad15
>
> Diff: https://reviews.apache.org/r/11268/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Moved 'tests::mkdtemp' to Environment.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On May 21, 2013, 9:57 p.m., Vinod Kone wrote:
> > src/tests/environment.cpp, line 178
> > <https://reviews.apache.org/r/11268/diff/1/?file=294232#file294232line178>
> >
> > Why in the destructor instead of TearDown()?
Because there isn't great semantics about when Environment::mkdtemp is allowed to be called. I was being pedantic and wanted to make sure that any directory created between Environment::TearDown and ~Environment wouldn't leak.
> On May 21, 2013, 9:57 p.m., Vinod Kone wrote:
> > src/tests/environment.cpp, line 200
> > <https://reviews.apache.org/r/11268/diff/1/?file=294232#file294232line200>
> >
> > what about launcher dir?
This has been (and still is) set in MesosTest.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11268/#review20861
-----------------------------------------------------------
On May 20, 2013, 9:58 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11268/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 9:58 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Description
> -------
>
> It's possible we should move mkdtemp into MesosTest instead. The benefit will be that we can cleanup temporary directories in ~MesosTest() which would mean that less temporary directories leak when tests fail. I'll defer this for now.
>
>
> Diffs
> -----
>
> src/tests/environment.hpp 61aa84debbf1ad31bc59be9bc91b6babef4881a3
> src/tests/environment.cpp c94c85fc2f710dc2157f6b61edb7bfb0df21a579
> src/tests/main.cpp e32ec0cac7b7ab9b31f4e419d0c1426b2c8aeb65
> src/tests/script.cpp d7f103eb08335638bca57ccab77646534342b058
> src/tests/utils.hpp ca3ecd7f0cab283327bf83e57d4b405b4ada9c74
> src/tests/utils.cpp 9d1d5ad1e192b0f44a7a7173839b292f6bedad15
>
> Diff: https://reviews.apache.org/r/11268/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Moved 'tests::mkdtemp' to Environment.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11268/#review20861
-----------------------------------------------------------
src/tests/environment.cpp
<https://reviews.apache.org/r/11268/#comment42969>
Why in the destructor instead of TearDown()?
src/tests/environment.cpp
<https://reviews.apache.org/r/11268/#comment42970>
what about launcher dir?
- Vinod Kone
On May 20, 2013, 9:58 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11268/
> -----------------------------------------------------------
>
> (Updated May 20, 2013, 9:58 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Description
> -------
>
> It's possible we should move mkdtemp into MesosTest instead. The benefit will be that we can cleanup temporary directories in ~MesosTest() which would mean that less temporary directories leak when tests fail. I'll defer this for now.
>
>
> Diffs
> -----
>
> src/tests/environment.hpp 61aa84debbf1ad31bc59be9bc91b6babef4881a3
> src/tests/environment.cpp c94c85fc2f710dc2157f6b61edb7bfb0df21a579
> src/tests/main.cpp e32ec0cac7b7ab9b31f4e419d0c1426b2c8aeb65
> src/tests/script.cpp d7f103eb08335638bca57ccab77646534342b058
> src/tests/utils.hpp ca3ecd7f0cab283327bf83e57d4b405b4ada9c74
> src/tests/utils.cpp 9d1d5ad1e192b0f44a7a7173839b292f6bedad15
>
> Diff: https://reviews.apache.org/r/11268/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>