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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Oct 11, 2019, 16:17 UTC
Message-ID: <20191011161707.GN29845@szeder.dev>
In-Reply-To: <CAKPyHN22-SnZ2_o1MO6EXuWk7S2AAWGQSjxu0OrhDq4pw=ap=g@mail.gmail.com>

On Fri, Oct 11, 2019 at 05:47:44PM +0200, Bert Wesarg wrote:

Show 28 quoted lines

> > > > +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.