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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 7, 2019 at 11:03 PM SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
>
> On Sat, Oct 05, 2019 at 10:43:51AM +0200, Bert Wesarg wrote:
> > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> > index 83f52614d3..2f2cd6fea6 100755
> > --- a/t/t4014-format-patch.sh
> > +++ b/t/t4014-format-patch.sh
> > @@ -1606,6 +1606,26 @@ test_expect_success 'From line has expected format' '
> >       test_cmp from filtered
> >  '
> >
> > +test_expect_success 'format-patch -o with no leading directories' '
> > +     rm -fr patches &&
> > +     git format-patch -o patches master..side &&
> > +     git rev-list master..side >list &&
> > +     test_line_count = $(ls patches | wc -l) list
>
> This is sort of a nit...
>
> So, these tests check that 'git rev-list ...' lists as many commits as
> the number of files created by 'git format-patch'.  While it doesn't
> affect the tests' correctness, this is subtly different from checking
> that 'git format-patch' created as many files as the number of commits
> listed by 'git rev-list'.
>
> Consider how the tests' output would look like on failure:
> 'test_line_count' shows an error message that includes the content of
> the file to be checked, which in this case would consist of a bunch of
> commit object ids:
>
>   test_line_count: line count for list != 3
>   f7af51d27933a90554b6e9212a7e5d4ad1c74569
>   bd89fce9f5096eb5cad67c342b40818b7e3ce9e4
>
> On one hand, these object ids won't mean much to anyone who might have
> to debug such a test failure in the future, and on the other these
> tests are about 'git format-patch', not about 'git rev-list'.  If the
> check were written like this:
>
>   count=$(git rev-list --count master..side) &&
>   ls patches >list &&
>   test_line_count = $count list
>
> then the error message on failure would look something like this:
>
>   test_line_count: line count for list != 3
>   0001-first.patch
>   0002-second.patch
>
> which, I think, would be more useful.

thanks for this detail. As I copied an existing test with this
pattern, I will add a new pre-patch to this serires which applies your
idea to the existing test first, before I add new tests for this
patch.

Bert

>
>
> > +'
> > +
> > +test_expect_success 'format-patch -o with leading existing directories' '
> > +     git format-patch -o patches/side master..side &&
> > +     git rev-list master..side >list &&
> > +     test_line_count = $(ls patches/side | wc -l) list
> > +'
> > +
> > +test_expect_success 'format-patch -o with leading non-existing directories' '
> > +     rm -fr patches &&
> > +     git format-patch -o patches/side master..side &&
> > +     git rev-list master..side >list &&
> > +     test_line_count = $(ls patches/side | wc -l) list
> > +'
> > +
> >  test_expect_success 'format-patch format.outputDirectory option' '
> >       test_config format.outputDirectory patches &&
> >       rm -fr patches &&
> > --
> > 2.23.0.11.g242cf7f110
> >




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux