Re: [Outreachy][PATCH v5] t6050: avoid pipes with upstream Git commands

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

 



On Mon, 14 Oct 2024 at 15:00, Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
>
> Hi Chizoba
>
> On 12/10/2024 07:21, chizobajames21@xxxxxxxxx wrote:
> > From: Chizoba ODINAKA <chizobajames21@xxxxxxxxx>
> >
> > In pipes, the exit code of a chain of commands is determined by
> > the final command. In order not to miss the exit code of a failed
> > Git command, avoid pipes instead write output of Git commands
> > into a file.
> > For better debugging experience, instances of "grep" were changed
> > to "test_grep". "test_grep" provides more context in case of a
> > failed "grep".
> >
> > Signed-off-by: Chizoba ODINAKA <chizobajames21@xxxxxxxxx>
> > Changes in v5:
> > - replace 'sort -o' which might not be supported by some platforms
> >    to the more conventional output redirection, > .
>
> It's really helpful to list the changes like this, but please do so
> below the "---" line so it does not become part of the commit message
Fixed.

> when the patch is applied. The range-diff looks good, with a fixed up
> commit message I think we can declare victory for this microproject.
>
> Best Wishes
>
> Phillip
>
Thanks Philip

> > ---
> > Range-diff against v4:
> > 1:  28cab4c752 ! 1:  b339b1f550 t6050: avoid pipes with upstream Git commands
> >      @@ Commit message
> >           failed "grep".
> >
> >           Signed-off-by: Chizoba ODINAKA <chizobajames21@xxxxxxxxx>
> >      -    Changes in v4:
> >      -    - Correct the subject
> >      -    - fixed some ommitted output option in sorts that where writing to
> >      -      stdout
> >      +    Changes in v5:
> >      +    - replace 'sort -o' which might not be supported by some platforms
> >      +      to the more conventional output redirection, > .
> >
> >        ## t/t6050-replace.sh ##
> >       @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
> >      @@ t/t6050-replace.sh: test_expect_success 'test --format medium' '
> >                       echo "$MYTAG -> $HASH1"
> >               } | sort >expected &&
> >       -       git replace -l --format medium | sort >actual &&
> >      -+       git replace -l --format medium >actual &&
> >      -+       sort actual -o actual &&
> >      ++       git replace -l --format medium >output &&
> >      ++       sort output >actual &&
> >               test_cmp expected actual
> >        '
> >
> >      @@ t/t6050-replace.sh: test_expect_success 'test --format long' '
> >                       echo "$MYTAG (tag) -> $HASH1 (commit)"
> >               } | sort >expected &&
> >       -       git replace --format=long | sort >actual &&
> >      -+       git replace --format=long >actual &&
> >      -+       sort actual -o actual &&
> >      ++       git replace --format=long >output &&
> >      ++       sort output >actual &&
> >               test_cmp expected actual
> >        '
> >
> >
> >   t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
> >   1 file changed, 86 insertions(+), 47 deletions(-)
> >
> > diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> > index d7702fc756..37cb6547b6 100755
> > --- a/t/t6050-replace.sh
> > +++ b/t/t6050-replace.sh
> > @@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
> >   '
> >
> >   test_expect_success 'replace the author' '
> > -     git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $R | grep "author O Thor" &&
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $R >actual &&
> > +     test_grep "author O Thor" actual &&
> >       git update-ref refs/replace/$HASH2 $R &&
> > -     git show HEAD~5 | grep "O Thor" &&
> > -     git show $HASH2 | grep "O Thor"
> > +     git show HEAD~5 >actual &&
> > +     test_grep "O Thor" actual &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual
> >   '
> >
> >   test_expect_success 'test --no-replace-objects option' '
> > -     git cat-file commit $HASH2 | grep "author O Thor" &&
> > -     git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     git show $HASH2 | grep "O Thor" &&
> > -     git --no-replace-objects show $HASH2 | grep "A U Thor"
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author O Thor" actual &&
> > +     git --no-replace-objects cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual &&
> > +     git --no-replace-objects show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual
> >   '
> >
> >   test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
> > -     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
> > +     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor"  actual &&
> > +     GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual
> >   '
> >
> >   test_expect_success 'test core.usereplacerefs config option' '
> >       test_config core.usereplacerefs false &&
> > -     git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     git show $HASH2 | grep "A U Thor"
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual
> >   '
> >
> >   cat >tag.sig <<EOF
> > @@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
> >       git clone --no-hardlinks . clone_dir &&
> >       (
> >               cd clone_dir &&
> > -             git show HEAD~5 | grep "A U Thor" &&
> > -             git show $HASH2 | grep "A U Thor" &&
> > +             git show HEAD~5 >actual &&
> > +             test_grep "A U Thor" actual &&
> > +             git show $HASH2 >actual &&
> > +             test_grep "A U Thor" actual &&
> >               git cat-file commit $R &&
> >               git repack -a -d &&
> >               test_must_fail git cat-file commit $R &&
> >               git fetch ../ "refs/replace/*:refs/replace/*" &&
> > -             git show HEAD~5 | grep "O Thor" &&
> > -             git show $HASH2 | grep "O Thor" &&
> > +             git show HEAD~5 >actual &&
> > +             test_grep "O Thor" actual &&
> > +             git show $HASH2 >actual &&
> > +             test_grep "O Thor" actual &&
> >               git cat-file commit $R
> >       )
> >   '
> > @@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
> >       test_must_fail git replace --delete &&
> >       test_must_fail git replace -l -d $HASH2 &&
> >       git replace -d $HASH2 &&
> > -     git show $HASH2 | grep "A U Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual &&
> >       test -z "$(git replace -l)"
> >   '
> >
> >   test_expect_success '"git replace" replacing' '
> >       git replace $HASH2 $R &&
> > -     git show $HASH2 | grep "O Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual &&
> >       test_must_fail git replace $HASH2 $R &&
> >       git replace -f $HASH2 $R &&
> >       test_must_fail git replace -f &&
> > @@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
> >       SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
> >       git replace -d $SHORTHASH2 &&
> >       git replace $SHORTHASH2 $R &&
> > -     git show $HASH2 | grep "O Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual &&
> >       test_must_fail git replace $HASH2 $R &&
> >       git replace -f $HASH2 $R &&
> >       test_must_fail git replace --force &&
> > @@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
> >   #
> >   test_expect_success 'create parallel branch without the bug' '
> >       git replace -d $HASH2 &&
> > -     git show $HASH2 | grep "A U Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual &&
> >       git checkout $HASH1 &&
> >       git cherry-pick $HASH2 &&
> > -     git show $HASH5 | git apply &&
> > +     git show $HASH5 >actual &&
> > +     git apply actual &&
> >       git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
> >       PARA2=$(git rev-parse --verify HEAD) &&
> >       git cherry-pick $HASH3 &&
> > @@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
> >       git checkout main &&
> >       cur=$(git rev-parse --verify HEAD) &&
> >       test "$cur" = "$HASH7" &&
> > -     git log --pretty=oneline | grep $PARA2 &&
> > +     git log --pretty=oneline >actual &&
> > +     test_grep $PARA2 actual &&
> >       git remote add cloned ./clone_dir
> >   '
> >
> > @@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
> >       (
> >               cd clone_dir &&
> >               git checkout parallel &&
> > -             git log --pretty=oneline | grep $PARA2
> > +             git log --pretty=oneline >actual &&
> > +             test_grep $PARA2 actual
> >       )
> >   '
> >
> >   test_expect_success 'push branch with replacement' '
> > -     git cat-file commit $PARA3 | grep "author A U Thor" &&
> > -     S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $S | grep "author O Thor" &&
> > +     git cat-file commit $PARA3 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $S >actual &&
> > +     test_grep "author O Thor" actual &&
> >       git replace $PARA3 $S &&
> > -     git show $HASH6~2 | grep "O Thor" &&
> > -     git show $PARA3 | grep "O Thor" &&
> > +     git show $HASH6~2 >actual &&
> > +     test_grep "O Thor" actual &&
> > +     git show $PARA3 >actual &&
> > +     test_grep "O Thor" actual &&
> >       git push cloned $HASH6^:refs/heads/parallel2 &&
> >       (
> >               cd clone_dir &&
> >               git checkout parallel2 &&
> > -             git log --pretty=oneline | grep $PARA3 &&
> > -             git show $PARA3 | grep "A U Thor"
> > +             git log --pretty=oneline >actual &&
> > +             test_grep $PARA3 actual &&
> > +             git show $PARA3 >actual &&
> > +             test_grep "A U Thor" actual
> >       )
> >   '
> >
> > @@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
> >               cd clone_dir &&
> >               git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
> >               git log --pretty=oneline parallel3 >output.txt &&
> > -             ! grep $PARA3 output.txt &&
> > +             test_grep ! $PARA3 output.txt &&
> >               git show $PARA3 >para3.txt &&
> > -             grep "A U Thor" para3.txt &&
> > +             test_grep "A U Thor" para3.txt &&
> >               git fetch origin "refs/replace/*:refs/replace/*" &&
> >               git log --pretty=oneline parallel3 >output.txt &&
> > -             grep $PARA3 output.txt &&
> > +             test_grep $PARA3 output.txt &&
> >               git show $PARA3 >para3.txt &&
> > -             grep "O Thor" para3.txt
> > +             test_grep "O Thor" para3.txt
> >       )
> >   '
> >
> > @@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
> >   '
> >
> >   test_expect_success 'index-pack and replacements' '
> > -     git --no-replace-objects rev-list --objects HEAD |
> > -     git --no-replace-objects pack-objects test- &&
> > +     git --no-replace-objects rev-list --objects HEAD >actual &&
> > +     git --no-replace-objects pack-objects test- <actual &&
> >       git index-pack test-*.pack
> >   '
> >
> > @@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
> >   '
> >
> >   test_expect_success 'git cat-file --batch works on replace objects' '
> > -     git replace | grep $PARA3 &&
> > +     git replace >actual &&
> > +     test_grep $PARA3 actual &&
> >       echo $PARA3 | git cat-file --batch
> >   '
> >
> > @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
> >               echo "$PARA3 -> $S" &&
> >               echo "$MYTAG -> $HASH1"
> >       } | sort >expected &&
> > -     git replace -l --format medium | sort >actual &&
> > +     git replace -l --format medium >output &&
> > +     sort output >actual &&
> >       test_cmp expected actual
> >   '
> >
> > @@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
> >               echo "$PARA3 (commit) -> $S (commit)" &&
> >               echo "$MYTAG (tag) -> $HASH1 (commit)"
> >       } | sort >expected &&
> > -     git replace --format=long | sort >actual &&
> > +     git replace --format=long >output &&
> > +     sort output >actual &&
> >       test_cmp expected actual
> >   '
> >
> > @@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
> >   test_expect_success '--edit with and without already replaced object' '
> >       test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> >       GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
> > -     git replace -l | grep "$PARA3" &&
> > -     git cat-file commit "$PARA3" | grep "A fake Thor" &&
> > +     git replace -l >actual &&
> > +     test_grep "$PARA3" actual &&
> > +     git cat-file commit "$PARA3" >actual &&
> > +     test_grep "A fake Thor" actual &&
> >       git replace -d "$PARA3" &&
> >       GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> > -     git replace -l | grep "$PARA3" &&
> > -     git cat-file commit "$PARA3" | grep "A fake Thor"
> > +     git replace -l >actual &&
> > +     test_grep "$PARA3" actual &&
> > +     git cat-file commit "$PARA3" >actual &&
> > +     test_grep "A fake Thor" actual
> >   '
> >
> >   test_expect_success '--edit and change nothing or command failed' '
> > @@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
> >       test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
> >       test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
> >       GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> > -     git replace -l | grep "$PARA3" &&
> > -     git cat-file commit "$PARA3" | grep "A fake Thor"
> > +     git replace -l >actual &&
> > +     test_grep "$PARA3" actual &&
> > +     git cat-file commit "$PARA3" >actual &&
> > +     test_grep "A fake Thor" actual
> >   '
> >
> >   test_expect_success 'replace ref cleanup' '
> > @@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
> >       git checkout main &&
> >       git merge -s ours test_tag &&
> >       HASH10=$(git rev-parse --verify HEAD) &&
> > -     git cat-file commit $HASH10 | grep "^mergetag object"
> > +     git cat-file commit $HASH10 >actual &&
> > +     test_grep "^mergetag object" actual
> >   '
> >
> >   test_expect_success GPG '--graft on a commit with a mergetag' '
>




[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