You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/03/22 08:12:18 UTC
Review Request: Fixed TemporaryDirectoryTest to clean up the sandbox,
and use ASSERTs instead of CHECKs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10077/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Description
-------
The TemporaryDirectoryTest fixture leaks the sandbox directories.
Diffs
-----
src/tests/utils.hpp 50203579f1ea14ee751c12a373047e9295e4ebb3
src/tests/utils.cpp 910396d872a8732ed53da353c4225dd81ff10b32
Diff: https://reviews.apache.org/r/10077/diff/
Testing
-------
make check
verified this was cleaning up
Thanks,
Ben Mahler
Re: Review Request: Fixed TemporaryDirectoryTest to clean up the sandbox,
and use ASSERTs instead of CHECKs.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10077/#review18287
-----------------------------------------------------------
Ship it!
src/tests/utils.hpp
<https://reviews.apache.org/r/10077/#comment38475>
Why not just save the Try<std::string> directory you get back from mkdtemp?
- Benjamin Hindman
On March 22, 2013, 7:12 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10077/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 7:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> The TemporaryDirectoryTest fixture leaks the sandbox directories.
>
>
> Diffs
> -----
>
> src/tests/utils.hpp 50203579f1ea14ee751c12a373047e9295e4ebb3
> src/tests/utils.cpp 910396d872a8732ed53da353c4225dd81ff10b32
>
> Diff: https://reviews.apache.org/r/10077/diff/
>
>
> Testing
> -------
>
> make check
> verified this was cleaning up
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Fixed TemporaryDirectoryTest to clean up the sandbox,
and use ASSERTs instead of CHECKs.
Posted by Ben Mahler <be...@gmail.com>.
> On March 22, 2013, 9:46 p.m., Vinod Kone wrote:
> > src/tests/utils.hpp, line 113
> > <https://reviews.apache.org/r/10077/diff/1/?file=273889#file273889line113>
> >
> > Why did you need to add this?
> >
> > Can't tests just do os::cwd() ?
Dropping as discussed (this is private in visibility, only used to be able to delete the sandbox in TearDown()).
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10077/#review18288
-----------------------------------------------------------
On March 22, 2013, 7:12 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10077/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 7:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> The TemporaryDirectoryTest fixture leaks the sandbox directories.
>
>
> Diffs
> -----
>
> src/tests/utils.hpp 50203579f1ea14ee751c12a373047e9295e4ebb3
> src/tests/utils.cpp 910396d872a8732ed53da353c4225dd81ff10b32
>
> Diff: https://reviews.apache.org/r/10077/diff/
>
>
> Testing
> -------
>
> make check
> verified this was cleaning up
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Fixed TemporaryDirectoryTest to clean up the sandbox,
and use ASSERTs instead of CHECKs.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10077/#review18288
-----------------------------------------------------------
src/tests/utils.hpp
<https://reviews.apache.org/r/10077/#comment38477>
Why did you need to add this?
Can't tests just do os::cwd() ?
src/tests/utils.cpp
<https://reviews.apache.org/r/10077/#comment38476>
not yours, but can you s/cerr/cout
- Vinod Kone
On March 22, 2013, 7:12 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10077/
> -----------------------------------------------------------
>
> (Updated March 22, 2013, 7:12 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Description
> -------
>
> The TemporaryDirectoryTest fixture leaks the sandbox directories.
>
>
> Diffs
> -----
>
> src/tests/utils.hpp 50203579f1ea14ee751c12a373047e9295e4ebb3
> src/tests/utils.cpp 910396d872a8732ed53da353c4225dd81ff10b32
>
> Diff: https://reviews.apache.org/r/10077/diff/
>
>
> Testing
> -------
>
> make check
> verified this was cleaning up
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request: Fixed TemporaryDirectoryTest to clean up the sandbox,
and use ASSERTs instead of CHECKs.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10077/
-----------------------------------------------------------
(Updated March 22, 2013, 10:16 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
vinod and benh reviews
Description
-------
The TemporaryDirectoryTest fixture leaks the sandbox directories.
Diffs (updated)
-----
src/tests/utils.hpp 50203579f1ea14ee751c12a373047e9295e4ebb3
src/tests/utils.cpp 910396d872a8732ed53da353c4225dd81ff10b32
Diff: https://reviews.apache.org/r/10077/diff/
Testing
-------
make check
verified this was cleaning up
Thanks,
Ben Mahler