Subject: Re: [PATCH v4 1/2] format-patch: create leading components of output directory

From: SZEDER Gábor <[email-masked]>
Date: Fri, 11 Oct 2019 18:17:07 +0200

On Fri, Oct 11, 2019 at 05:47:44PM +0200, Bert Wesarg wrote:
> > > > +test_expect_success 'format-patch -o with no leading directories' '
> > > > +     rm -fr patches &&
> > > > +     git format-patch -o patches master..side &&
> > > > +     count=$(git rev-list --count master..side) &&
> > > > +     ls patches >list &&
> > > > +     test_line_count = $count list
> > > > +'
> > > > +
> > > > +test_expect_success 'format-patch -o with leading existing directories' '
> > > > +     git format-patch -o patches/side master..side &&
> > >
> > > The previous test case creates the 'patches' directory and leaves it
> > > behind, and this test implicitly relies on that directory to check
> > > that 'format-patch -o' can deal with already existing directories.  So
> > > if the previous test case were to fail early or were not run at all
> > > (e.g. './t4014-format-patch.sh -r 1,137'), then that directory
> > > wouldn't exist, and, consequently, this test case would not check what
> > > it was supposed to.
> > >
> > > I think it would be better to be more explicit and self-contained
> > > about it, and create a leading directory in this test case:
> > >
> > >    mkdir existing-dir &&
> > >    git format-patch -o existing-dir/side master..size &&
> > >    ls existing-dir/side >list &&
> 
> one question: How about removing this directory first, just to be
> sure, that mkdir does create a directory?

I'm not sure I understand...

Do you mean that a previous test might have already created and left a
directory with the same name behind, and then this 'mkdir' would error
out and thus fail the test?  If yes, then you're right with your
nitpicking on my nitpicking ;)  Though instead of 'rm -rf'ing that
directory I would suggest 'mkdir -p' to simply ignore it if it were to
exist.