> On March 20, 2016, 6:36 p.m., Vinod Kone wrote:
> > src/tests/fetcher_tests.cpp, lines 645-647
> > <https://reviews.apache.org/r/45046/diff/3/?file=1307412#file1307412line645>
> >
> > I know you followed the existing pattern in this file but we shouldn't
> > do mktemp() like this in the tests because if the test fails for whatever
> > reason the temp file and the directory are leaked.
> >
> > The FetcherTest fixture inherits from TemporaryDirectoryTest so that
> > the test runs in a temporary sandbox.
> >
> > So you should do:
> >
> > ```
> > Try<string> dir = os::mkdtemp("./XXXX"));
> > ASSERT_SOME(dir);
> >
> > Try<string> path = os::mktemp(path::join(dir.get(), "XXXX"));
> > ASSERT_SOME(path);
> > ```
> >
> > Feel free to fix the other tests in this file in a subsequent review or
> > just add a TODO for now.
>
> Michael Browning wrote:
> Happy to fix the other tests in this file. I think it'd be simpler to do
> it in a separate review -- should I also create an issue for that?