Re: [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests
- From
René Scharfe <l.s.r@web.de>
- Date
- Dec 2, 2022, 15:59 UTC
- Message-ID
- <8cfb5bc5-6e17-4cac-cc2f-2f900f14fa60@web.de>
- In-Reply-To
- <patch-v3-2.8-394d5e46494-20221202T114733Z-avarab@gmail.com>
Am 02.12.22 um 12:52 schrieb Ævar Arnfjörð Bjarmason:
Show 106 quoted lines
> Change the functions which are called from within
> "test_expect_success" to add the "|| return 1" idiom to their
> for-loops, so we won't lose the exit code of "cp", "git" etc.
>
> Then for those setup functions that aren't called from a
> "test_expect_success" we need to put the setup code in a
> "test_expect_success" as well. It would not be enough to properly
> &&-chain these, as the calling code is the top-level script itself. As
> we don't run the tests with "set -e" we won't report failing commands
> at the top-level.
>
> The "checkout" part of this would miss memory leaks under
> SANITIZE=leak, this code doesn't leak (the relevant "git checkout"
> leak has been fixed), but in a past version of git we'd continue past
> this failure under SANITIZE=leak when these invocations had errored
> out, even under "--immediate".
>
> Helped-by: René Scharfe <l.s.r@web.de>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t0027-auto-crlf.sh | 60 +++++++++++++++++++++++++-------------------
> 1 file changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index a94ac1eae37..23f2e613401 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -70,7 +70,8 @@ create_NNO_MIX_files () {
> cp CRLF ${pfx}_CRLF.txt &&
> cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
> cp LF_mix_CR ${pfx}_LF_mix_CR.txt &&
> - cp CRLF_nul ${pfx}_CRLF_nul.txt
> + cp CRLF_nul ${pfx}_CRLF_nul.txt ||
> + return 1
> done
> done
> done
> @@ -101,7 +102,8 @@ commit_check_warn () {
> do
> fname=${pfx}_$f.txt &&
> cp $f $fname &&
> - git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
> + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err" ||
> + return 1
> done &&
> git commit -m "core.autocrlf $crlf" &&
> check_warning "$lfname" ${pfx}_LF.err &&
> @@ -121,15 +123,19 @@ commit_chk_wrnNNO () {
> lfmixcr=$1 ; shift
> crlfnul=$1 ; shift
> pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
> - #Commit files on top of existing file
> - create_gitattributes "$attr" $aeol &&
> - for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
> - do
> - fname=${pfx}_$f.txt &&
> - cp $f $fname &&
> - printf Z >>"$fname" &&
> - git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
> - done
> +
> + test_expect_success 'setup commit NNO files' '
> + #Commit files on top of existing file
> + create_gitattributes "$attr" $aeol &&
> + for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
> + do
> + fname=${pfx}_$f.txt &&
> + cp $f $fname &&
> + printf Z >>"$fname" &&
> + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err" ||
> + return 1
> + done
> + '
>
> test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
> check_warning "$lfwarn" ${pfx}_LF.err
> @@ -163,15 +169,19 @@ commit_MIX_chkwrn () {
> lfmixcr=$1 ; shift
> crlfnul=$1 ; shift
> pfx=MIX_attr_${attr}_aeol_${aeol}_${crlf}
> - #Commit file with CLRF_mix_LF on top of existing file
> - create_gitattributes "$attr" $aeol &&
> - for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
> - do
> - fname=${pfx}_$f.txt &&
> - cp CRLF_mix_LF $fname &&
> - printf Z >>"$fname" &&
> - git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
> - done
> +
> + test_expect_success 'setup commit file with mixed EOL' '
> + #Commit file with CLRF_mix_LF on top of existing file
> + create_gitattributes "$attr" $aeol &&
> + for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
> + do
> + fname=${pfx}_$f.txt &&
> + cp CRLF_mix_LF $fname &&
> + printf Z >>"$fname" &&
> + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err" ||
> + return 1
> + done
> + '
>
> test_expect_success "commit file with mixed EOL onto LF crlf=$crlf attr=$attr" '
> check_warning "$lfwarn" ${pfx}_LF.err
> @@ -294,12 +304,10 @@ checkout_files () {The context lines right here are:
create_gitattributes "$attr" $ident $aeol && git config core.autocrlf $crlf &&
Those better be covered by test_expect_success as well, right? Wrap them and the whole loop in a single test_expect_success, like you did above?
Show 16 quoted lines
> pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
> for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
> do
> - rm crlf_false_attr__$f.txt &&
> - if test -z "$ceol"; then
> - git checkout -- crlf_false_attr__$f.txt
> - else
> - git -c core.eol=$ceol checkout -- crlf_false_attr__$f.txt
> - fi
> + test_expect_success "setup $f checkout ${ceol:+ with -c core.eol=$ceol}" '
> + rm -f crlf_false_attr__$f.txt &&
> + git ${ceol:+-c core.eol=$ceol} checkout -- crlf_false_attr__$f.txt
> + '
> done
>
> test_expect_success "ls-files --eol attr=$attr $ident aeol=$aeol core.autocrlf=$crlf core.eol=$ceol" '