Re: [PATCH 1/3] t4014: clean up style

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

 



On Sun, Aug 18, 2019 at 10:38 PM Denton Liu <liu.denton@xxxxxxxxx> wrote:
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> @@ -96,7 +97,7 @@ test_expect_success "format-patch doesn't consider merge commits" '
>         git merge --no-ff slave &&
> -       cnt=$(git format-patch -3 --stdout | grep "^From " | wc -l) &&
> +       cnt=$(git format-patch -3 --stdout >patch && grep "^From " patch | wc -l) &&
>         test $cnt = 3

Just below here, you update tests to use test_line_count(). Why not do
the same here?

> @@ -104,21 +105,22 @@ test_expect_success "format-patch result applies" '
>
>         git checkout -b rebuild-0 master &&
>         git am -3 patch0 &&
> -       cnt=$(git rev-list master.. | wc -l) &&
> -       test $cnt = 2
> +       git rev-list master.. >list &&
> +       test_line_count = 2 list
>  '
> @@ -357,7 +370,7 @@ test_expect_success 'reroll count (-v)' '
>  check_threading () {
>         expect="$1" &&
>         shift &&
> -       (git format-patch --stdout "$@"; echo $? > status.out) |
> +       (git format-patch --stdout "$@"; echo $? >status.out) |
>         # Prints everything between the Message-ID and In-Reply-To,
>         # and replaces all Message-ID-lookalikes by a sequence number
>         perl -ne '
> @@ -372,12 +385,12 @@ check_threading () {
>                         print;
>                 }
>                 print "---\n" if /^From /i;
> -       ' > actual &&
> +       ' >actual &&
>         test 0 = "$(cat status.out)" &&
>         test_cmp "$expect" actual
>  }

Not a new issue, but this is more difficult to reason about than it
could be due to the manual squirreling away of the status code of the
Git command which is upstream of the pipe. More straightforward would
be:

    ...
    shift &&
    git format-patch --stdout "$@" >patch &&
    perl -ne '...' <patch >actual &&
    test_cmp "$expect" actual

No need for capturing or manually checking format-patch's exit code
since the &&-chain handles it for you. And, you no longer have a Git
command upstream of a pipe.

> @@ -852,20 +865,22 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
> -git_version="$(git --version | sed "s/.* //")"
> +git_version="$(git --version >version && sed "s/.* //" version)"

Meh. The exit code is being ignored anyhow since this is not part of a &&-chain.

> @@ -1649,19 +1665,32 @@ test_expect_success 'format-patch -o overrides format.outputDirectory' '
>  test_expect_success 'format-patch --base' '
>         [...]
>         echo >expected &&
>         echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
> -       echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --stable | awk "{print \$1}")" >>expected &&
> -       echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --stable | awk "{print \$1}")" >>expected &&
> -       signature >> expected &&
> +       git show --patch HEAD~2 >patch &&
> +       git patch-id --stable <patch >patch.id.raw &&
> +       awk "{print \$1}" <patch.id.raw >patch.id &&
> +       echo "prerequisite-patch-id: $(cat patch.id)" >>expected &&
> +       git show --patch HEAD~1 >patch &&
> +       git patch-id --stable <patch >patch.id.raw &&
> +       awk "{print \$1}" <patch.id.raw >patch.id &&
> +       echo "prerequisite-patch-id: $(cat patch.id)" >>expected &&

Not a big deal, but you could take advantage of the fact that you're
already invoking 'awk', thus avoid the 'echo':

    awk "{print \"prerequisite-patch-id:\", \$1}" <patch.id.raw >patch.id &&

> +       signature >>expected &&
>         test_cmp expected actual1 &&
>         test_cmp expected actual2 &&
>         echo >fail &&
>         echo "base-commit: $(git rev-parse HEAD~3)" >>fail &&
> -       echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --unstable | awk "{print \$1}")" >>fail &&
> -       echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --unstable | awk "{print \$1}")" >>fail &&
> +       echo "prerequisite-patch-id: $(
> +               git show --patch HEAD~2 >patch &&
> +               git patch-id --unstable <patch >patch.id.raw &&
> +               awk "{print \$1}" <patch.id.raw)" >>fail &&
> +       echo "prerequisite-patch-id: $(git show --patch HEAD~1 >patch &&
> +               git patch-id --unstable <patch >patch.id.raw &&
> +               awk "{print \$1}" <pattch.id.raw)" >>fail &&

Hmph, why is this case transformed in a different way than the case
immediately above.



[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