Re: [PATCH v5 1/5] t5323: test cases for git-pack-redundant

From: Jiang Xin <worldhello.net@gmail.com>
Date: Jan 11, 2019, 01:59 UTC
Message-ID: <CANYiYbFxCYSt0yQtt5xsFSshVCDuVU58hWBA6AF2e+QzmdUViQ@mail.gmail.com>
In-Reply-To: <xmqqimyw1am8.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> 于2019年1月11日周五 上午5:11写道：

Show 8 quoted lines

>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> > +create_commits()
> > +{
>
> Style (see Documentation/CodingGuidelines).

OK, parenthese after function name.

Show 16 quoted lines

>
> > +create_pack_1()
> > +{
> > +     P1=$(cd .git/objects/pack; printf "$T\n$A\n$B\n$C\n$D\n$E\n$F\n$R\n" | git pack-objects pack 2>/dev/null) &&
>
> Yikes.  Can't "git pack-objects" get the input directly without
> overlong printf, something along the lines of...
>
>         P1=$(git -C .git/objects/pack pack-objects pack <<-EOF
>                 $A
>                 $B
>                 $C
>                 ...
>                 $R
>                 EOF
>         )

Find that no space before <OID>, because git-pack-objects not allow that, and mached parentheses should in the same line. So Will write like this:

    create_pack_1() {
            P1=$(git -C .git/objects/pack pack-objects pack <<-EOF) &&
    $T
    $A
    $B
    $R
    EOF
            eval P$P1=P1:$P1
    }

Show 15 quoted lines

> > +test_expect_success 'no redundant packs' '
> > +     create_pack_1 && create_pack_2 && create_pack_3 &&
> > +     git pack-redundant --all >out &&
> > +     test_must_be_empty out
> > +'
> > +
> > +test_expect_success 'create pack 4, 5' '
> > +     create_pack_4 && create_pack_5
> > +'
> > +
> > +cat >expected <<EOF
> > +P2:$P2
> > +EOF
>
> Move this to the next "expect success" block?

$P4 and $P5 are defined after calling `create_pack_4` and `create_pack_5`, so create pack functions should be called before write `expected` file, if puts $P4 and/or $P5 in the expected file.

For this case, $P4 and $P5 not in expected file, we can move create_pack_4 and 5 to the following test_expect_success block, but the new algorithm may change the expected file.

Show 8 quoted lines

>
> > +test_expect_success 'one of pack-2/pack-3 is redundant' '
> > +     git pack-redundant --all >out &&
> > +     sed -E -e "s#.*/pack-(.*)\.(idx|pack)#\1#" out | \
>
> How portable is "sed -E" (it is not even in POSIX.1)?  Wouldn't it
> be easier to work with to have two "-e" fed to a single sed
> invocation instead?

will fix using two '-e' commands.