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.