* [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-05 0:24 ` Junio C Hamano
2022-12-02 11:52 ` [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
` (7 subsequent siblings)
8 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Change the verify_mergeheads() helper the check the exit code of "git
rev-parse".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t7600-merge.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 7c3f6ed9943..060e145957f 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -105,7 +105,7 @@ verify_mergeheads () {
test_write_lines "$@" >mergehead.expected &&
while read sha1 rest
do
- git rev-parse $sha1
+ git rev-parse $sha1 || return 1
done <.git/MERGE_HEAD >mergehead.actual &&
test_cmp mergehead.expected mergehead.actual
}
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper
2022-12-02 11:52 ` [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper Ævar Arnfjörð Bjarmason
@ 2022-12-05 0:24 ` Junio C Hamano
0 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-05 0:24 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Subject: Re: [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper
Just one is inclded here, so I'll retitle (and I am tempted to
disassemble the "series" into separate individual topics, as that
will help to use good bits early without getting distracted by the
rest) this to something like "s/^[^:]*:/t7600:/"
> Change the verify_mergeheads() helper the check the exit code of "git
> rev-parse".
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t7600-merge.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks.
>
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 7c3f6ed9943..060e145957f 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -105,7 +105,7 @@ verify_mergeheads () {
> test_write_lines "$@" >mergehead.expected &&
> while read sha1 rest
> do
> - git rev-parse $sha1
> + git rev-parse $sha1 || return 1
> done <.git/MERGE_HEAD >mergehead.actual &&
> test_cmp mergehead.expected mergehead.actual
> }
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
2022-12-02 11:52 ` [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-02 15:59 ` René Scharfe
2022-12-02 11:52 ` [PATCH v3 3/8] diff tests: fix ignored exit codes in t4023 Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
8 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
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 () {
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" '
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests
2022-12-02 11:52 ` [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
@ 2022-12-02 15:59 ` René Scharfe
0 siblings, 0 replies; 83+ messages in thread
From: René Scharfe @ 2022-12-02 15:59 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Eric Sunshine, Torsten Bögershausen
Am 02.12.22 um 12:52 schrieb Ævar Arnfjörð Bjarmason:
> 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?
> 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" '
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3 3/8] diff tests: fix ignored exit codes in t4023
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
2022-12-02 11:52 ` [PATCH v3 1/8] merge tests: don't ignore "rev-parse" exit code in helper Ævar Arnfjörð Bjarmason
2022-12-02 11:52 ` [PATCH v3 2/8] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-05 0:26 ` Junio C Hamano
2022-12-02 11:52 ` [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
8 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Change a "git diff-tree" command to be &&-chained so that we won't
ignore its exit code, see the ea05fd5fbf7 (Merge branch
'ab/keep-git-exit-codes-in-tests', 2022-03-16) topic for prior art.
This fixes code added in b45563a229f (rename: Break filepairs with
different types., 2007-11-30). Due to hiding the exit code we hid a
memory leak under SANITIZE=leak.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t4023-diff-rename-typechange.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh
index 7cb99092938..25c31b0cb1b 100755
--- a/t/t4023-diff-rename-typechange.sh
+++ b/t/t4023-diff-rename-typechange.sh
@@ -52,8 +52,8 @@ test_expect_success setup '
'
test_expect_success 'cross renames to be detected for regular files' '
-
- git diff-tree five six -r --name-status -B -M | sort >actual &&
+ git diff-tree five six -r --name-status -B -M >out &&
+ sort <out >actual &&
{
echo "R100 foo bar" &&
echo "R100 bar foo"
@@ -63,8 +63,8 @@ test_expect_success 'cross renames to be detected for regular files' '
'
test_expect_success 'cross renames to be detected for typechange' '
-
- git diff-tree one two -r --name-status -B -M | sort >actual &&
+ git diff-tree one two -r --name-status -B -M >out &&
+ sort <out >actual &&
{
echo "R100 foo bar" &&
echo "R100 bar foo"
@@ -74,8 +74,8 @@ test_expect_success 'cross renames to be detected for typechange' '
'
test_expect_success 'moves and renames' '
-
- git diff-tree three four -r --name-status -B -M | sort >actual &&
+ git diff-tree three four -r --name-status -B -M >out &&
+ sort <out >actual &&
{
# see -B -M (#6) in t4008
echo "C100 foo bar" &&
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 3/8] diff tests: fix ignored exit codes in t4023
2022-12-02 11:52 ` [PATCH v3 3/8] diff tests: fix ignored exit codes in t4023 Ævar Arnfjörð Bjarmason
@ 2022-12-05 0:26 ` Junio C Hamano
0 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-05 0:26 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Change a "git diff-tree" command to be &&-chained so that we won't
> ignore its exit code, see the ea05fd5fbf7 (Merge branch
> 'ab/keep-git-exit-codes-in-tests', 2022-03-16) topic for prior art.
>
> This fixes code added in b45563a229f (rename: Break filepairs with
> different types., 2007-11-30). Due to hiding the exit code we hid a
> memory leak under SANITIZE=leak.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t4023-diff-rename-typechange.sh | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
I already have this as 1ebeb849 (diff tests: fix ignored exit codes
in t4023, 2022-12-02) on ab/t4023-avoid-losing-exit-status-of-diff
topic.
Thanks.
>
> diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh
> index 7cb99092938..25c31b0cb1b 100755
> --- a/t/t4023-diff-rename-typechange.sh
> +++ b/t/t4023-diff-rename-typechange.sh
> @@ -52,8 +52,8 @@ test_expect_success setup '
> '
>
> test_expect_success 'cross renames to be detected for regular files' '
> -
> - git diff-tree five six -r --name-status -B -M | sort >actual &&
> + git diff-tree five six -r --name-status -B -M >out &&
> + sort <out >actual &&
> {
> echo "R100 foo bar" &&
> echo "R100 bar foo"
> @@ -63,8 +63,8 @@ test_expect_success 'cross renames to be detected for regular files' '
> '
>
> test_expect_success 'cross renames to be detected for typechange' '
> -
> - git diff-tree one two -r --name-status -B -M | sort >actual &&
> + git diff-tree one two -r --name-status -B -M >out &&
> + sort <out >actual &&
> {
> echo "R100 foo bar" &&
> echo "R100 bar foo"
> @@ -74,8 +74,8 @@ test_expect_success 'cross renames to be detected for typechange' '
> '
>
> test_expect_success 'moves and renames' '
> -
> - git diff-tree three four -r --name-status -B -M | sort >actual &&
> + git diff-tree three four -r --name-status -B -M >out &&
> + sort <out >actual &&
> {
> # see -B -M (#6) in t4008
> echo "C100 foo bar" &&
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2022-12-02 11:52 ` [PATCH v3 3/8] diff tests: fix ignored exit codes in t4023 Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-02 15:59 ` René Scharfe
2022-12-04 0:45 ` Eric Sunshine
2022-12-02 11:52 ` [PATCH v3 5/8] tests: use "test_cmp" instead of "test" in sub-shells Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
8 siblings, 2 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Fix code added in b319ef70a94 (Add a small patch-mode testing library,
2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
This avoids losing both the exit code of a "git" and the "cat"
processes.
This fixes cases where we'd have e.g. missed memory leaks under
SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
discovered it while looking at leaks in related code.
The "cat _head >expect" here is redundant, we could simply give
"_head" to "test_cmp", but let's be consistent in using the "expect"
and "actual" names for clarity.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-patch-mode.sh | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
index cfd76bf987b..89ca1f78055 100644
--- a/t/lib-patch-mode.sh
+++ b/t/lib-patch-mode.sh
@@ -29,8 +29,12 @@ set_and_save_state () {
# verify_state <path> <expected-worktree-content> <expected-index-content>
verify_state () {
- test "$(cat "$1")" = "$2" &&
- test "$(git show :"$1")" = "$3"
+ echo "$2" >expect &&
+ test_cmp expect "$1" &&
+
+ echo "$3" >expect &&
+ git show :"$1" >actual &&
+ test_cmp expect actual
}
# verify_saved_state <path>
@@ -46,5 +50,6 @@ save_head () {
}
verify_saved_head () {
- test "$(cat _head)" = "$(git rev-parse HEAD)"
+ git rev-parse HEAD >actual &&
+ test_cmp _head actual
}
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-02 11:52 ` [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-02 15:59 ` René Scharfe
2022-12-04 0:45 ` Eric Sunshine
1 sibling, 0 replies; 83+ messages in thread
From: René Scharfe @ 2022-12-02 15:59 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Eric Sunshine, Torsten Bögershausen
Am 02.12.22 um 12:52 schrieb Ævar Arnfjörð Bjarmason:
> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
> 2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
> This avoids losing both the exit code of a "git" and the "cat"
> processes.
>
> This fixes cases where we'd have e.g. missed memory leaks under
> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
> discovered it while looking at leaks in related code.
>
> The "cat _head >expect" here is redundant, we could simply give
> "_head" to "test_cmp", but let's be consistent in using the "expect"
> and "actual" names for clarity.
The code at the bottom uses _head directly, which is fine IMHO, but then
this sentence should go.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/lib-patch-mode.sh | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
> index cfd76bf987b..89ca1f78055 100644
> --- a/t/lib-patch-mode.sh
> +++ b/t/lib-patch-mode.sh
> @@ -29,8 +29,12 @@ set_and_save_state () {
>
> # verify_state <path> <expected-worktree-content> <expected-index-content>
> verify_state () {
> - test "$(cat "$1")" = "$2" &&
> - test "$(git show :"$1")" = "$3"
> + echo "$2" >expect &&
> + test_cmp expect "$1" &&
> +
> + echo "$3" >expect &&
> + git show :"$1" >actual &&
> + test_cmp expect actual
> }
>
> # verify_saved_state <path>
> @@ -46,5 +50,6 @@ save_head () {
> }
>
> verify_saved_head () {
> - test "$(cat _head)" = "$(git rev-parse HEAD)"
> + git rev-parse HEAD >actual &&
> + test_cmp _head actual
> }
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-02 11:52 ` [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
2022-12-02 15:59 ` René Scharfe
@ 2022-12-04 0:45 ` Eric Sunshine
1 sibling, 0 replies; 83+ messages in thread
From: Eric Sunshine @ 2022-12-04 0:45 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, René Scharfe, Torsten Bögershausen
On Fri, Dec 2, 2022 at 6:53 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
> 2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
> This avoids losing both the exit code of a "git" and the "cat"
> processes.
This still talks about test_cmp_cmd() which is no longer present in v3.
> This fixes cases where we'd have e.g. missed memory leaks under
> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
> discovered it while looking at leaks in related code.
>
> The "cat _head >expect" here is redundant, we could simply give
> "_head" to "test_cmp", but let's be consistent in using the "expect"
> and "actual" names for clarity.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3 5/8] tests: use "test_cmp" instead of "test" in sub-shells
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2022-12-02 11:52 ` [PATCH v3 4/8] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-05 0:39 ` Junio C Hamano
2022-12-02 11:52 ` [PATCH v3 6/8] tests: don't lose 'test <str> = $(cmd ...)"' exit code Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
8 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Convert a few cases where we were using "test" inside a sub-shell, and
were losing the exit code of "git".
In the case of "t3200-branch.sh" some adjacent code outside of a
sub-shell that was losing the exit code is also being converted, as
it's within the same hunk.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-httpd.sh | 5 +++--
t/lib-submodule-update.sh | 22 +++++++++-------------
t/t0060-path-utils.sh | 4 +++-
t/t3200-branch.sh | 13 +++++++------
t/t5605-clone-local.sh | 15 ++++++++++-----
t/t7402-submodule-rebase.sh | 14 +++++++++++---
6 files changed, 43 insertions(+), 30 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 608949ea80b..31e7fa3010c 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -217,8 +217,9 @@ test_http_push_nonff () {
git commit -a -m path2 --amend &&
test_must_fail git push -v origin >output 2>&1 &&
- (cd "$REMOTE_REPO" &&
- test $HEAD = $(git rev-parse --verify HEAD))
+ echo "$HEAD" >expect &&
+ git -C "$REMOTE_REPO" rev-parse --verify HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success 'non-fast-forward push show ref status' '
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 2d31fcfda1f..d7c2b670b4a 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -168,20 +168,16 @@ replace_gitfile_with_git_dir () {
# Note that this only supports submodules at the root level of the
# superproject, with the default name, i.e. same as its path.
test_git_directory_is_unchanged () {
- (
- cd ".git/modules/$1" &&
- # does core.worktree point at the right place?
- test "$(git config core.worktree)" = "../../../$1" &&
- # remove it temporarily before comparing, as
- # "$1/.git/config" lacks it...
- git config --unset core.worktree
- ) &&
+ # does core.worktree point at the right place?
+ echo "../../../$1" >expect &&
+ git -C ".git/modules/$1" config core.worktree >actual &&
+ test_cmp expect actual &&
+ # remove it temporarily before comparing, as
+ # "$1/.git/config" lacks it...
+ git -C ".git/modules/$1" config --unset core.worktree &&
diff -r ".git/modules/$1" "$1/.git" &&
- (
- # ... and then restore.
- cd ".git/modules/$1" &&
- git config core.worktree "../../../$1"
- )
+ # ... and then restore.
+ git -C ".git/modules/$1" config core.worktree "../../../$1"
}
test_git_directory_exists () {
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 68e29c904a6..53ec717cbca 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -255,7 +255,9 @@ test_expect_success 'prefix_path rejects absolute path to dir with same beginnin
test_expect_success SYMLINKS 'prefix_path works with absolute path to a symlink to work tree having same beginning as work tree' '
git init repo &&
ln -s repo repolink &&
- test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
+ echo "a" >expect &&
+ test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&
+ test_cmp expect actual
'
relative_path /foo/a/b/c/ /foo/a/b/ c/
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5a169b68d6a..f5fbb84262b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -242,12 +242,13 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
test_expect_success 'git branch -M baz bam should succeed within a worktree in which baz is checked out' '
git checkout -b baz &&
git worktree add -f bazdir baz &&
- (
- cd bazdir &&
- git branch -M baz bam &&
- test $(git rev-parse --abbrev-ref HEAD) = bam
- ) &&
- test $(git rev-parse --abbrev-ref HEAD) = bam &&
+ git -C "$bazdir" branch -M baz bam &&
+ echo "bam" >expect &&
+ git -C "$bazdir" rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual &&
+ echo "bam" >expect &&
+ git rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual &&
rm -r bazdir &&
git worktree prune
'
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 38b850c10ef..61a2342bc2c 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -15,8 +15,12 @@ test_expect_success 'preparing origin repository' '
: >file && git add . && git commit -m1 &&
git clone --bare . a.git &&
git clone --bare . x &&
- test "$(cd a.git && git config --bool core.bare)" = true &&
- test "$(cd x && git config --bool core.bare)" = true &&
+ echo true >expect &&
+ git -C a.git config --bool core.bare >actual &&
+ test_cmp expect actual &&
+ echo true >expect &&
+ git -C x config --bool core.bare >actual &&
+ test_cmp expect actual &&
git bundle create b1.bundle --all &&
git bundle create b2.bundle main &&
mkdir dir &&
@@ -28,9 +32,10 @@ test_expect_success 'preparing origin repository' '
test_expect_success 'local clone without .git suffix' '
git clone -l -s a b &&
- (cd b &&
- test "$(git config --bool core.bare)" = false &&
- git fetch)
+ echo false >expect &&
+ git -C b config --bool core.bare >actual &&
+ test_cmp expect actual &&
+ git -C b fetch
'
test_expect_success 'local clone with .git suffix' '
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index ebeca12a711..1927a862839 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -82,11 +82,19 @@ test_expect_success 'stash with a dirty submodule' '
CURRENT=$(cd submodule && git rev-parse HEAD) &&
git stash &&
test new != $(cat file) &&
- test submodule = $(git diff --name-only) &&
- test $CURRENT = $(cd submodule && git rev-parse HEAD) &&
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual &&
+
+ echo "$CURRENT" >expect &&
+ git -C submodule rev-parse HEAD >actual &&
+ test_cmp expect actual &&
+
git stash apply &&
test new = $(cat file) &&
- test $CURRENT = $(cd submodule && git rev-parse HEAD)
+ echo "$CURRENT" >expect &&
+ git -C submodule rev-parse HEAD >actual &&
+ test_cmp expect actual
'
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 5/8] tests: use "test_cmp" instead of "test" in sub-shells
2022-12-02 11:52 ` [PATCH v3 5/8] tests: use "test_cmp" instead of "test" in sub-shells Ævar Arnfjörð Bjarmason
@ 2022-12-05 0:39 ` Junio C Hamano
0 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-05 0:39 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Convert a few cases where we were using "test" inside a sub-shell, and
> were losing the exit code of "git".
That makes it sound like
(
cd there &&
a=$(git something expected to be silent) &&
test -z "$a"
) &&
...
is bad, and it can be improved somehow by using "test_cmp" instead
of "test", but I do not think that is what you meant (in fact, the
command substitution used above is safe and we catch failing git).
After looking at a few samples from the patch s/sub-shell/command
substitution/ might be what you meant, i.e.
test -z "$(git something) &&
...
is bad and we want
git something >out &&
! test -s out
to keep the exit code from "git". IOW, fixing the lossage of exit
code has little to do with the use of test vs test_cmp.
Perhaps retitle to
Subject: [PATCH] tests: avoid "test op $(git foo)" lose exit status of git
Rewrite tests that ran "git" inside command substitution and
lost exit status of "git" so that we notice failing "git".
or something like that.
> In the case of "t3200-branch.sh" some adjacent code outside of a
> sub-shell that was losing the exit code is also being converted, as
> it's within the same hunk.
Again s/sub-shell/command substitution?, I think.
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/lib-httpd.sh | 5 +++--
> t/lib-submodule-update.sh | 22 +++++++++-------------
> t/t0060-path-utils.sh | 4 +++-
> t/t3200-branch.sh | 13 +++++++------
> t/t5605-clone-local.sh | 15 ++++++++++-----
> t/t7402-submodule-rebase.sh | 14 +++++++++++---
> 6 files changed, 43 insertions(+), 30 deletions(-)
>
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 608949ea80b..31e7fa3010c 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -217,8 +217,9 @@ test_http_push_nonff () {
> git commit -a -m path2 --amend &&
>
> test_must_fail git push -v origin >output 2>&1 &&
> - (cd "$REMOTE_REPO" &&
> - test $HEAD = $(git rev-parse --verify HEAD))
> + echo "$HEAD" >expect &&
> + git -C "$REMOTE_REPO" rev-parse --verify HEAD >actual &&
> + test_cmp expect actual
> '
>
> test_expect_success 'non-fast-forward push show ref status' '
> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
> index 2d31fcfda1f..d7c2b670b4a 100644
> --- a/t/lib-submodule-update.sh
> +++ b/t/lib-submodule-update.sh
> @@ -168,20 +168,16 @@ replace_gitfile_with_git_dir () {
> # Note that this only supports submodules at the root level of the
> # superproject, with the default name, i.e. same as its path.
> test_git_directory_is_unchanged () {
> - (
> - cd ".git/modules/$1" &&
> - # does core.worktree point at the right place?
> - test "$(git config core.worktree)" = "../../../$1" &&
> - # remove it temporarily before comparing, as
> - # "$1/.git/config" lacks it...
> - git config --unset core.worktree
> - ) &&
> + # does core.worktree point at the right place?
> + echo "../../../$1" >expect &&
> + git -C ".git/modules/$1" config core.worktree >actual &&
> + test_cmp expect actual &&
> + # remove it temporarily before comparing, as
> + # "$1/.git/config" lacks it...
> + git -C ".git/modules/$1" config --unset core.worktree &&
> diff -r ".git/modules/$1" "$1/.git" &&
> - (
> - # ... and then restore.
> - cd ".git/modules/$1" &&
> - git config core.worktree "../../../$1"
> - )
> + # ... and then restore.
> + git -C ".git/modules/$1" config core.worktree "../../../$1"
> }
>
> test_git_directory_exists () {
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 68e29c904a6..53ec717cbca 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -255,7 +255,9 @@ test_expect_success 'prefix_path rejects absolute path to dir with same beginnin
> test_expect_success SYMLINKS 'prefix_path works with absolute path to a symlink to work tree having same beginning as work tree' '
> git init repo &&
> ln -s repo repolink &&
> - test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
> + echo "a" >expect &&
> + test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&
> + test_cmp expect actual
> '
>
> relative_path /foo/a/b/c/ /foo/a/b/ c/
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 5a169b68d6a..f5fbb84262b 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -242,12 +242,13 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
> test_expect_success 'git branch -M baz bam should succeed within a worktree in which baz is checked out' '
> git checkout -b baz &&
> git worktree add -f bazdir baz &&
> - (
> - cd bazdir &&
> - git branch -M baz bam &&
> - test $(git rev-parse --abbrev-ref HEAD) = bam
> - ) &&
> - test $(git rev-parse --abbrev-ref HEAD) = bam &&
> + git -C "$bazdir" branch -M baz bam &&
> + echo "bam" >expect &&
> + git -C "$bazdir" rev-parse --abbrev-ref HEAD >actual &&
> + test_cmp expect actual &&
> + echo "bam" >expect &&
> + git rev-parse --abbrev-ref HEAD >actual &&
> + test_cmp expect actual &&
> rm -r bazdir &&
> git worktree prune
> '
> diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
> index 38b850c10ef..61a2342bc2c 100755
> --- a/t/t5605-clone-local.sh
> +++ b/t/t5605-clone-local.sh
> @@ -15,8 +15,12 @@ test_expect_success 'preparing origin repository' '
> : >file && git add . && git commit -m1 &&
> git clone --bare . a.git &&
> git clone --bare . x &&
> - test "$(cd a.git && git config --bool core.bare)" = true &&
> - test "$(cd x && git config --bool core.bare)" = true &&
> + echo true >expect &&
> + git -C a.git config --bool core.bare >actual &&
> + test_cmp expect actual &&
> + echo true >expect &&
> + git -C x config --bool core.bare >actual &&
> + test_cmp expect actual &&
> git bundle create b1.bundle --all &&
> git bundle create b2.bundle main &&
> mkdir dir &&
> @@ -28,9 +32,10 @@ test_expect_success 'preparing origin repository' '
>
> test_expect_success 'local clone without .git suffix' '
> git clone -l -s a b &&
> - (cd b &&
> - test "$(git config --bool core.bare)" = false &&
> - git fetch)
> + echo false >expect &&
> + git -C b config --bool core.bare >actual &&
> + test_cmp expect actual &&
> + git -C b fetch
> '
>
> test_expect_success 'local clone with .git suffix' '
> diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
> index ebeca12a711..1927a862839 100755
> --- a/t/t7402-submodule-rebase.sh
> +++ b/t/t7402-submodule-rebase.sh
> @@ -82,11 +82,19 @@ test_expect_success 'stash with a dirty submodule' '
> CURRENT=$(cd submodule && git rev-parse HEAD) &&
> git stash &&
> test new != $(cat file) &&
> - test submodule = $(git diff --name-only) &&
> - test $CURRENT = $(cd submodule && git rev-parse HEAD) &&
> + echo submodule >expect &&
> + git diff --name-only >actual &&
> + test_cmp expect actual &&
> +
> + echo "$CURRENT" >expect &&
> + git -C submodule rev-parse HEAD >actual &&
> + test_cmp expect actual &&
> +
> git stash apply &&
> test new = $(cat file) &&
> - test $CURRENT = $(cd submodule && git rev-parse HEAD)
> + echo "$CURRENT" >expect &&
> + git -C submodule rev-parse HEAD >actual &&
> + test_cmp expect actual
>
> '
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3 6/8] tests: don't lose 'test <str> = $(cmd ...)"' exit code
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2022-12-02 11:52 ` [PATCH v3 5/8] tests: use "test_cmp" instead of "test" in sub-shells Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-02 11:52 ` [PATCH v3 7/8] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
8 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Convert some cases in the test suite where we'd lose the exit code of
a command being interpolated as one of the arguments to the "test"
builtin function to use &&-chaining and "test_cmp" instead.
This way we won't lose the exit code, and the failure output will be
more helpful.
In the case of "t0060-path-utils.sh" and
"t2005-checkout-index-symlinks.sh" convert the relevant code to using
the modern style of indentation and newline wrapping while having to
change it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-submodule-update.sh | 4 +-
t/t0001-init.sh | 9 ++-
t/t0002-gitfile.sh | 4 +-
t/t0060-path-utils.sh | 103 +++++++++++++++++++++--------
t/t0100-previous.sh | 8 ++-
t/t1504-ceiling-dirs.sh | 8 ++-
t/t2005-checkout-index-symlinks.sh | 8 ++-
t/t5522-pull-symlink.sh | 4 +-
t/t7402-submodule-rebase.sh | 9 ++-
t/t7504-commit-msg-hook.sh | 4 +-
t/t7810-grep.sh | 4 +-
11 files changed, 120 insertions(+), 45 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index d7c2b670b4a..dee14992c52 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -185,7 +185,9 @@ test_git_directory_exists () {
if test -f sub1/.git
then
# does core.worktree point at the right place?
- test "$(git -C .git/modules/$1 config core.worktree)" = "../../../$1"
+ echo "../../../$1" >expect &&
+ git -C ".git/modules/$1" config core.worktree >actual &&
+ test_cmp expect actual
fi
}
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index d479303efa0..30a6edca1d2 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -598,9 +598,14 @@ test_expect_success 'invalid default branch name' '
test_expect_success 'branch -m with the initial branch' '
git init rename-initial &&
git -C rename-initial branch -m renamed &&
- test renamed = $(git -C rename-initial symbolic-ref --short HEAD) &&
+ echo renamed >expect &&
+ git -C rename-initial symbolic-ref --short HEAD >actual &&
+ test_cmp expect actual &&
+
git -C rename-initial branch -m renamed again &&
- test again = $(git -C rename-initial symbolic-ref --short HEAD)
+ echo again >expect &&
+ git -C rename-initial symbolic-ref --short HEAD >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 26eaca095a2..e013d38f485 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -33,7 +33,9 @@ test_expect_success 'bad setup: invalid .git file path' '
test_expect_success 'final setup + check rev-parse --git-dir' '
echo "gitdir: $REAL" >.git &&
- test "$REAL" = "$(git rev-parse --git-dir)"
+ echo "$REAL" >expect &&
+ git rev-parse --git-dir >actual &&
+ test_cmp expect actual
'
test_expect_success 'check hash-object' '
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 53ec717cbca..6490ad5ca1b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -10,20 +10,27 @@ TEST_PASSES_SANITIZE_LEAK=true
norm_path() {
expected=$(test-tool path-utils print_path "$2")
- test_expect_success $3 "normalize path: $1 => $2" \
- "test \"\$(test-tool path-utils normalize_path_copy '$1')\" = '$expected'"
+ test_expect_success $3 "normalize path: $1 => $2" "
+ echo '$expected' >expect &&
+ test-tool path-utils normalize_path_copy '$1' >actual &&
+ test_cmp expect actual
+ "
}
relative_path() {
expected=$(test-tool path-utils print_path "$3")
- test_expect_success $4 "relative path: $1 $2 => $3" \
- "test \"\$(test-tool path-utils relative_path '$1' '$2')\" = '$expected'"
+ test_expect_success $4 "relative path: $1 $2 => $3" "
+ echo '$expected' >expect &&
+ test-tool path-utils relative_path '$1' '$2' >actual &&
+ test_cmp expect actual
+ "
}
test_submodule_relative_url() {
test_expect_success "test_submodule_relative_url: $1 $2 $3 => $4" "
- actual=\$(test-tool submodule resolve-relative-url '$1' '$2' '$3') &&
- test \"\$actual\" = '$4'
+ echo '$4' >expect &&
+ test-tool submodule resolve-relative-url '$1' '$2' '$3' >actual &&
+ test_cmp expect actual
"
}
@@ -64,9 +71,11 @@ ancestor() {
expected=$(($expected-$rootslash+$rootoff))
;;
esac
- test_expect_success $4 "longest ancestor: $1 $2 => $expected" \
- "actual=\$(test-tool path-utils longest_ancestor_length '$1' '$2') &&
- test \"\$actual\" = '$expected'"
+ test_expect_success $4 "longest ancestor: $1 $2 => $expected" "
+ echo '$expected' >expect &&
+ test-tool path-utils longest_ancestor_length '$1' '$2' >actual &&
+ test_cmp expect actual
+ "
}
# Some absolute path tests should be skipped on Windows due to path mangling
@@ -166,8 +175,10 @@ ancestor D:/Users/me C:/ -1 MINGW
ancestor //server/share/my-directory //server/share/ 14 MINGW
test_expect_success 'strip_path_suffix' '
- test c:/msysgit = $(test-tool path-utils strip_path_suffix \
- c:/msysgit/libexec//git-core libexec/git-core)
+ echo c:/msysgit >expect &&
+ test-tool path-utils strip_path_suffix \
+ c:/msysgit/libexec//git-core libexec/git-core >actual &&
+ test_cmp expect actual
'
test_expect_success 'absolute path rejects the empty string' '
@@ -188,35 +199,61 @@ test_expect_success 'real path rejects the empty string' '
'
test_expect_success POSIX 'real path works on absolute paths 1' '
+ echo / >expect &&
+ test-tool path-utils real_path "/" >actual &&
+ test_cmp expect actual &&
+
nopath="hopefully-absent-path" &&
- test "/" = "$(test-tool path-utils real_path "/")" &&
- test "/$nopath" = "$(test-tool path-utils real_path "/$nopath")"
+ echo "/$nopath" >expect &&
+ test-tool path-utils real_path "/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success 'real path works on absolute paths 2' '
- nopath="hopefully-absent-path" &&
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "$d")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "$d/$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "$d" >actual &&
+ test_cmp expect actual &&
+
+ nopath="hopefully-absent-path" &&
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "$d/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success POSIX 'real path removes extra leading slashes' '
+ echo "/" >expect &&
+ test-tool path-utils real_path "///" >actual &&
+ test_cmp expect actual &&
+
nopath="hopefully-absent-path" &&
- test "/" = "$(test-tool path-utils real_path "///")" &&
- test "/$nopath" = "$(test-tool path-utils real_path "///$nopath")" &&
+ echo "/$nopath" >expect &&
+ test-tool path-utils real_path "///$nopath" >actual &&
+ test_cmp expect actual &&
+
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "//$d")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "//$d/$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "//$d" >actual &&
+ test_cmp expect actual &&
+
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "//$d/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success 'real path removes other extra slashes' '
- nopath="hopefully-absent-path" &&
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "$d///")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "$d///$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "$d///" >actual &&
+ test_cmp expect actual &&
+
+ nopath="hopefully-absent-path" &&
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "$d///$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'real path works on symlinks' '
@@ -227,19 +264,29 @@ test_expect_success SYMLINKS 'real path works on symlinks' '
mkdir third &&
dir="$(cd .git && pwd -P)" &&
dir2=third/../second/other/.git &&
- test "$dir" = "$(test-tool path-utils real_path $dir2)" &&
+ echo "$dir" >expect &&
+ test-tool path-utils real_path $dir2 >actual &&
+ test_cmp expect actual &&
file="$dir"/index &&
- test "$file" = "$(test-tool path-utils real_path $dir2/index)" &&
+ echo "$file" >expect &&
+ test-tool path-utils real_path $dir2/index >actual &&
+ test_cmp expect actual &&
basename=blub &&
- test "$dir/$basename" = "$(cd .git && test-tool path-utils real_path "$basename")" &&
+ echo "$dir/$basename" >expect &&
+ test-tool -C .git path-utils real_path "$basename" >actual &&
+ test_cmp expect actual &&
ln -s ../first/file .git/syml &&
sym="$(cd first && pwd -P)"/file &&
- test "$sym" = "$(test-tool path-utils real_path "$dir2/syml")"
+ echo "$sym" >expect &&
+ test-tool path-utils real_path "$dir2/syml" >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'prefix_path works with absolute paths to work tree symlinks' '
ln -s target symlink &&
- test "$(test-tool path-utils prefix_path prefix "$(pwd)/symlink")" = "symlink"
+ echo "symlink" >expect &&
+ test-tool path-utils prefix_path prefix "$(pwd)/symlink" >actual &&
+ test_cmp expect actual
'
test_expect_success 'prefix_path works with only absolute path to work tree' '
diff --git a/t/t0100-previous.sh b/t/t0100-previous.sh
index a16cc3d2983..70a3223f219 100755
--- a/t/t0100-previous.sh
+++ b/t/t0100-previous.sh
@@ -12,7 +12,9 @@ test_expect_success 'branch -d @{-1}' '
test_commit A &&
git checkout -b junk &&
git checkout - &&
- test "$(git symbolic-ref HEAD)" = refs/heads/main &&
+ echo refs/heads/main >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
git branch -d @{-1} &&
test_must_fail git rev-parse --verify refs/heads/junk
'
@@ -21,7 +23,9 @@ test_expect_success 'branch -d @{-12} when there is not enough switches yet' '
git reflog expire --expire=now &&
git checkout -b junk2 &&
git checkout - &&
- test "$(git symbolic-ref HEAD)" = refs/heads/main &&
+ echo refs/heads/main >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
test_must_fail git branch -d @{-12} &&
git rev-parse --verify refs/heads/main
'
diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
index 0fafcf9dde3..c1679e31d8a 100755
--- a/t/t1504-ceiling-dirs.sh
+++ b/t/t1504-ceiling-dirs.sh
@@ -6,8 +6,12 @@ TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_prefix() {
- test_expect_success "$1" \
- "test '$2' = \"\$(git rev-parse --show-prefix)\""
+ local expect="$2" &&
+ test_expect_success "$1: git rev-parse --show-prefix is '$2'" '
+ echo "$expect" >expect &&
+ git rev-parse --show-prefix >actual &&
+ test_cmp expect actual
+ '
}
test_fail() {
diff --git a/t/t2005-checkout-index-symlinks.sh b/t/t2005-checkout-index-symlinks.sh
index 112682a45a1..67d18cfa104 100755
--- a/t/t2005-checkout-index-symlinks.sh
+++ b/t/t2005-checkout-index-symlinks.sh
@@ -22,8 +22,10 @@ test_expect_success \
git checkout-index symlink &&
test -f symlink'
-test_expect_success \
-'the file must be the blob we added during the setup' '
-test "$(git hash-object -t blob symlink)" = $l'
+test_expect_success 'the file must be the blob we added during the setup' '
+ echo "$l" >expect &&
+ git hash-object -t blob symlink >actual &&
+ test_cmp expect actual
+'
test_done
diff --git a/t/t5522-pull-symlink.sh b/t/t5522-pull-symlink.sh
index bcff460d0a2..9fb73a8c3eb 100755
--- a/t/t5522-pull-symlink.sh
+++ b/t/t5522-pull-symlink.sh
@@ -78,7 +78,9 @@ test_expect_success SYMLINKS 'pushing from symlinked subdir' '
git commit -m push ./file &&
git push
) &&
- test push = $(git show HEAD:subdir/file)
+ echo push >expect &&
+ git show HEAD:subdir/file >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index 1927a862839..c74798e8d24 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -55,12 +55,15 @@ chmod a+x fake-editor.sh
test_expect_success 'interactive rebase with a dirty submodule' '
- test submodule = $(git diff --name-only) &&
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual &&
HEAD=$(git rev-parse HEAD) &&
GIT_EDITOR="\"$(pwd)/fake-editor.sh\"" EDITOR_TEXT="pick $HEAD" \
git rebase -i HEAD^ &&
- test submodule = $(git diff --name-only)
-
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual
'
test_expect_success 'rebase with dirty file and submodule fails' '
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index a39de8c1126..c0f024eb1ef 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -101,7 +101,9 @@ test_expect_success 'setup: commit-msg hook that always fails' '
'
commit_msg_is () {
- test "$(git log --pretty=format:%s%b -1)" = "$1"
+ printf "%s" "$1" >expect &&
+ git log --pretty=format:%s%b -1 >actual &&
+ test_cmp expect actual
}
test_expect_success 'with failing hook' '
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 8eded6ab274..39d6d713ecb 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1001,7 +1001,9 @@ test_expect_success 'log --committer does not search in timestamp' '
test_expect_success 'grep with CE_VALID file' '
git update-index --assume-unchanged t/t &&
rm t/t &&
- test "$(git grep test)" = "t/t:test" &&
+ echo "t/t:test" >expect &&
+ git grep test >actual &&
+ test_cmp expect actual &&
git update-index --no-assume-unchanged t/t &&
git checkout t/t
'
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* [PATCH v3 7/8] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2022-12-02 11:52 ` [PATCH v3 6/8] tests: don't lose 'test <str> = $(cmd ...)"' exit code Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-02 18:31 ` René Scharfe
2022-12-02 11:52 ` [PATCH v3 8/8] tests: don't lose mist "git" exit codes Ævar Arnfjörð Bjarmason
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
8 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Change tests that would lose the "git" exit code via a negation
pattern to:
- In the case of "t0055-beyond-symlinks.sh" compare against the
expected output instead.
- For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
and use "test_must_be_empty" to assert that it's not there.
We can also remove a repeated invocation of "git ls-files" for the
last test that's being modified in that file, and search the
existing "files" output instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
t/t3700-add.sh | 18 +++++++++++++-----
2 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
index 6bada370225..c3eb1158ef9 100755
--- a/t/t0055-beyond-symlinks.sh
+++ b/t/t0055-beyond-symlinks.sh
@@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
test_must_fail git update-index --add c/d &&
- ! ( git ls-files | grep c/d )
+ cat >expect <<-\EOF &&
+ a
+ b/d
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'add beyond symlinks' '
test_must_fail git add c/d &&
- ! ( git ls-files | grep c/d )
+ cat >expect <<-\EOF &&
+ a
+ b/d
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 51afbd7b24a..82dd768944f 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
test_expect_success '.gitignore is honored' '
git add . &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones without -f' '
test_must_fail git add a.?? &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones without -f' '
test_must_fail git add d.?? &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones but add others' '
touch a.if &&
test_must_fail git add a.?? &&
- ! (git ls-files | grep "\\.ig") &&
- (git ls-files | grep a.if)
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual &&
+ grep a.if files
'
test_expect_success 'add ignored ones with -f' '
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 7/8] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-02 11:52 ` [PATCH v3 7/8] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
@ 2022-12-02 18:31 ` René Scharfe
0 siblings, 0 replies; 83+ messages in thread
From: René Scharfe @ 2022-12-02 18:31 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Eric Sunshine, Torsten Bögershausen
Am 02.12.22 um 12:52 schrieb Ævar Arnfjörð Bjarmason:
> Change tests that would lose the "git" exit code via a negation
> pattern to:
>
> - In the case of "t0055-beyond-symlinks.sh" compare against the
> expected output instead.
>
> - For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
> and use "test_must_be_empty" to assert that it's not there.
>
> We can also remove a repeated invocation of "git ls-files" for the
> last test that's being modified in that file, and search the
> existing "files" output instead.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
> t/t3700-add.sh | 18 +++++++++++++-----
> 2 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
> index 6bada370225..c3eb1158ef9 100755
> --- a/t/t0055-beyond-symlinks.sh
> +++ b/t/t0055-beyond-symlinks.sh
> @@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
>
> test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
> test_must_fail git update-index --add c/d &&
> - ! ( git ls-files | grep c/d )
> + cat >expect <<-\EOF &&
> + a
> + b/d
> + EOF
> + git ls-files >actual &&
> + test_cmp expect actual
> '
Hmm, this makes the test depend on the other files. Adding more of them
for a different test now requires updating this test as well. Why not
handle it like you did in t3700 below?
>
> test_expect_success SYMLINKS 'add beyond symlinks' '
> test_must_fail git add c/d &&
> - ! ( git ls-files | grep c/d )
> + cat >expect <<-\EOF &&
> + a
> + b/d
> + EOF
> + git ls-files >actual &&
> + test_cmp expect actual
> '
Same here.
>
> test_done
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index 51afbd7b24a..82dd768944f 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
>
> test_expect_success '.gitignore is honored' '
> git add . &&
> - ! (git ls-files | grep "\\.ig")
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual
> '
You use sed instead of grep because no match is expected and sed returns
0 even then, while grep would exit with code 1, correct? OK.
Can we use that, though? I.e. how about this?
git ls-files >files &&
test_expect_code 1 grep "\\.ig" files
It would print the unexpected lines in verbose mode like this:
expecting success of 3700.12 '.gitignore is honored':
git add . &&
git ls-files >files &&
echo foo.ig >files && # inject bogus value
test_expect_code 1 grep "\\.ig" files
foo.ig
test_expect_code: command exited with 0, we wanted 1 grep \.ig files
Or can we trust ls-files' own filtering?
git ls-files "*.ig" >actual &&
test_must_be_empty actual
Both variants are shorter and should be slightly faster, which can
matter if we use that pattern more widely.
(Didn't measure here, but from a recent foray into t3920 on Windows I
took home that removing commands has a small, but measurable impact there:
https://lore.kernel.org/git/203cb627-2423-8a35-d280-9f9ffc66e072@web.de/T/#u)
>
> test_expect_success 'error out when attempting to add ignored ones without -f' '
> test_must_fail git add a.?? &&
> - ! (git ls-files | grep "\\.ig")
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual
> '
>
> test_expect_success 'error out when attempting to add ignored ones without -f' '
> test_must_fail git add d.?? &&
> - ! (git ls-files | grep "\\.ig")
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual
> '
>
> test_expect_success 'error out when attempting to add ignored ones but add others' '
> touch a.if &&
> test_must_fail git add a.?? &&
> - ! (git ls-files | grep "\\.ig") &&
> - (git ls-files | grep a.if)
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual &&
> + grep a.if files
> '
>
> test_expect_success 'add ignored ones with -f' '
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v3 8/8] tests: don't lose mist "git" exit codes
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2022-12-02 11:52 ` [PATCH v3 7/8] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
@ 2022-12-02 11:52 ` Ævar Arnfjörð Bjarmason
2022-12-04 0:40 ` Eric Sunshine
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
8 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 11:52 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Fix a few miscellaneous cases where:
- We lost the "git" exit code via "git ... | grep"
- Likewise by having a $(git) argument to git itself
- Used "test -z" to check that a command emitted no output, we can use
"test_must_be_empty" and &&-chaining instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1401-symbolic-ref.sh | 3 ++-
t/t3701-add-interactive.sh | 8 +++++---
t/t7516-commit-races.sh | 3 ++-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index d708acdb819..5e36899d207 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -33,7 +33,8 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
reset_to_sane
test_expect_success 'symbolic-ref refuses bare sha1' '
- test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
+ rev=$(git rev-parse HEAD) &&
+ test_must_fail git symbolic-ref HEAD "$rev"
'
reset_to_sane
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 5841f280fb2..f1fe5d60677 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' '
echo content >>file &&
chmod +x file &&
printf "y\\ny\\n" | git add -p &&
- git diff --cached file | grep "new mode" &&
- git diff --cached file | grep "+content" &&
- test -z "$(git diff file)"
+ git diff --cached file >out &&
+ grep "new mode" out &&
+ grep "+content" out &&
+ git diff file >out &&
+ test_must_be_empty out
'
# end of tests disabled when filemode is not usable
diff --git a/t/t7516-commit-races.sh b/t/t7516-commit-races.sh
index f2ce14e9071..2d38a16480e 100755
--- a/t/t7516-commit-races.sh
+++ b/t/t7516-commit-races.sh
@@ -10,7 +10,8 @@ test_expect_success 'race to create orphan commit' '
test_must_fail env EDITOR=./hare-editor git commit --allow-empty -m tortoise -e &&
git show -s --pretty=format:%s >subject &&
grep hare subject &&
- test -z "$(git show -s --pretty=format:%P)"
+ git show -s --pretty=format:%P >out &&
+ test_must_be_empty out
'
test_expect_success 'race to create non-orphan commit' '
--
2.39.0.rc1.981.gf846af54b4b
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v3 8/8] tests: don't lose mist "git" exit codes
2022-12-02 11:52 ` [PATCH v3 8/8] tests: don't lose mist "git" exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-04 0:40 ` Eric Sunshine
2022-12-05 0:45 ` Junio C Hamano
0 siblings, 1 reply; 83+ messages in thread
From: Eric Sunshine @ 2022-12-04 0:40 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, René Scharfe, Torsten Bögershausen
On Fri, Dec 2, 2022 at 6:53 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> tests: don't lose mist "git" exit codes
"mist"?
> Fix a few miscellaneous cases where:
>
> - We lost the "git" exit code via "git ... | grep"
> - Likewise by having a $(git) argument to git itself
> - Used "test -z" to check that a command emitted no output, we can use
> "test_must_be_empty" and &&-chaining instead.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v3 8/8] tests: don't lose mist "git" exit codes
2022-12-04 0:40 ` Eric Sunshine
@ 2022-12-05 0:45 ` Junio C Hamano
0 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-05 0:45 UTC (permalink / raw)
To: Eric Sunshine
Cc: Ævar Arnfjörð Bjarmason, git, René Scharfe,
Torsten Bögershausen
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Fri, Dec 2, 2022 at 6:53 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> tests: don't lose mist "git" exit codes
>
> "mist"?
s/t/c/ I thought.
In any case, it seems like 5,6,7,8/8 all are about using $(cmd) as
an argument to another command (as opposed to placing it on the
right side of a variable assignment) that loses the exit status of
running $(cmd), and it is a bit confusing which piece should go into
which one of these four. It might make more sense to reorganize
these four steps along the file boundary.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 0/6] tests: fix ignored & hidden exit codes
2022-12-02 11:52 ` [PATCH v3 0/8] tests: fix ignored & hidden exit codes Ævar Arnfjörð Bjarmason
` (7 preceding siblings ...)
2022-12-02 11:52 ` [PATCH v3 8/8] tests: don't lose mist "git" exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-19 10:19 ` [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
` (7 more replies)
8 siblings, 8 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Various fixes for "git" on the LHS of a pipe, but mostly when in
"test" expressions like:
test str = "$(git some-command)" &&
Changes since v3[1]:
* The previous 1/8 and 3/8 were picked independently, and have landed
on "master". I was waiting for those to graduate before a re-roll.
* Add a missing test_expect_success to 1/6, as pointed out by René
(thanks!).
* Remove commit message mention of the now-dead test_cmp_cmd (thanks
Eric!)
* Reword commit messages, fix typos etc. (thanks Junio, and other
reviewers!)
I think this should address all of the feedback on the v3, except
Junio's suggestion of perhaps re-arranging this series around file
boundaries.
Given the potential size of that range-diff I thought it was better to
mosttly keep the same structure.
1. https://lore.kernel.org/git/cover-v3-0.8-00000000000-20221202T114733Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason (6):
auto-crlf tests: don't lose exit code in loops and outside tests
t/lib-patch-mode.sh: fix ignored exit codes
tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
tests: don't lose exit status with "test <op> $(git ...)"
tests: don't lose "git" exit codes in "! ( git ... | grep )"
tests: don't lose misc "git" exit codes
t/lib-httpd.sh | 5 +-
t/lib-patch-mode.sh | 11 ++-
t/lib-submodule-update.sh | 26 ++++---
t/t0001-init.sh | 9 ++-
t/t0002-gitfile.sh | 4 +-
t/t0027-auto-crlf.sh | 66 ++++++++++--------
t/t0055-beyond-symlinks.sh | 14 +++-
t/t0060-path-utils.sh | 107 +++++++++++++++++++++--------
t/t0100-previous.sh | 8 ++-
t/t1401-symbolic-ref.sh | 3 +-
t/t1504-ceiling-dirs.sh | 8 ++-
t/t2005-checkout-index-symlinks.sh | 8 ++-
t/t3200-branch.sh | 13 ++--
t/t3700-add.sh | 18 +++--
t/t3701-add-interactive.sh | 8 ++-
t/t5522-pull-symlink.sh | 4 +-
t/t5605-clone-local.sh | 15 ++--
t/t7402-submodule-rebase.sh | 23 +++++--
t/t7504-commit-msg-hook.sh | 4 +-
t/t7516-commit-races.sh | 3 +-
t/t7810-grep.sh | 4 +-
21 files changed, 243 insertions(+), 118 deletions(-)
Range-diff against v3:
1: 64dfec31fb3 < -: ----------- merge tests: don't ignore "rev-parse" exit code in helper
2: 394d5e46494 ! 1: 68d276dd421 auto-crlf tests: don't lose exit code in loops and outside tests
@@ t/t0027-auto-crlf.sh: commit_MIX_chkwrn () {
test_expect_success "commit file with mixed EOL onto LF crlf=$crlf attr=$attr" '
check_warning "$lfwarn" ${pfx}_LF.err
@@ t/t0027-auto-crlf.sh: checkout_files () {
+ lfmixcrlf=$1 ; shift
+ lfmixcr=$1 ; shift
+ crlfnul=$1 ; shift
+- create_gitattributes "$attr" $ident $aeol &&
+- git config core.autocrlf $crlf &&
++ test_expect_success "setup config for checkout attr=$attr ident=$ident aeol=$aeol core.autocrlf=$crlf" '
++ create_gitattributes "$attr" $ident $aeol &&
++ git config core.autocrlf $crlf
++ '
pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
do
3: 4ec075689f6 < -: ----------- diff tests: fix ignored exit codes in t4023
4: c080899dd5f ! 2: d351075f0ab t/lib-patch-mode.sh: fix ignored exit codes
@@ Commit message
t/lib-patch-mode.sh: fix ignored exit codes
Fix code added in b319ef70a94 (Add a small patch-mode testing library,
- 2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
+ 2009-08-13) to use &&-chaining.
+
This avoids losing both the exit code of a "git" and the "cat"
processes.
@@ Commit message
SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
discovered it while looking at leaks in related code.
- The "cat _head >expect" here is redundant, we could simply give
- "_head" to "test_cmp", but let's be consistent in using the "expect"
- and "actual" names for clarity.
-
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/lib-patch-mode.sh ##
5: 58ac6fe5604 ! 3: f5b2489609c tests: use "test_cmp" instead of "test" in sub-shells
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- tests: use "test_cmp" instead of "test" in sub-shells
+ tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
- Convert a few cases where we were using "test" inside a sub-shell, and
- were losing the exit code of "git".
+ Rewrite tests that ran "git" inside command substitution and lost the
+ exit status of "git" so that we notice the failing "git".
- In the case of "t3200-branch.sh" some adjacent code outside of a
- sub-shell that was losing the exit code is also being converted, as
- it's within the same hunk.
+ Have them use modern patterns such as a "test_cmp" of the expected
+ outputs instead, and avoid needlessly spawning sub-shell in favor of
+ using "git -C <dir>".
+
+ We'll fix more of these these in the subsequent commit, for now we're
+ only converting the cases where this loss of exit code was combined
+ with spawning a sub-shell. The one exception to that is the casein
+ "t3200-branch.sh" where adjacent code didn't use a sub-shell, let's
+ convert that here as it's within the same hunk.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
6: 51f32b42ce6 ! 4: da66e5bf1c1 tests: don't lose 'test <str> = $(cmd ...)"' exit code
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- tests: don't lose 'test <str> = $(cmd ...)"' exit code
+ tests: don't lose exit status with "test <op> $(git ...)"
- Convert some cases in the test suite where we'd lose the exit code of
- a command being interpolated as one of the arguments to the "test"
- builtin function to use &&-chaining and "test_cmp" instead.
-
- This way we won't lose the exit code, and the failure output will be
- more helpful.
+ As with the preceding commit, rewrite tests that ran "git" inside
+ command substitution and lost the exit status of "git" so that we
+ notice the failing "git". This time around we're converting cases that
+ didn't involve a containing sub-shell around the command substitution.
In the case of "t0060-path-utils.sh" and
"t2005-checkout-index-symlinks.sh" convert the relevant code to using
7: 307f25db831 ! 5: 9596702978e tests: don't lose "git" exit codes in "! ( git ... | grep )"
@@ Commit message
- In the case of "t0055-beyond-symlinks.sh" compare against the
expected output instead.
+ We could use the same pattern as in the "t3700-add.sh" below, doing
+ so would have the advantage that if we added an earlier test we
+ wouldn't need to adjust the "expect" output.
+
+ But as "t0055-beyond-symlinks.sh" is a small and focused test (less
+ than 40 lines in total) let's use "test_cmp" instead.
+
- For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
- and use "test_must_be_empty" to assert that it's not there.
+ and use "test_must_be_empty" to assert that it's not there. If we used
+ "grep" we'd get a non-zero exit code.
+
+ We could use "test_expect_code 1 grep", but this is more consistent
+ with existing patterns in the test suite.
We can also remove a repeated invocation of "git ls-files" for the
last test that's being modified in that file, and search the
8: 37c75f4a097 ! 6: 94df7a1764e tests: don't lose mist "git" exit codes
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- tests: don't lose mist "git" exit codes
+ tests: don't lose misc "git" exit codes
Fix a few miscellaneous cases where:
--
2.39.0.1071.g97ce8966538
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-19 12:07 ` René Scharfe
2022-12-19 10:19 ` [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
7 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
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 | 66 +++++++++++++++++++++++++-------------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index a94ac1eae37..2f57c8669cb 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
@@ -289,17 +299,17 @@ checkout_files () {
lfmixcrlf=$1 ; shift
lfmixcr=$1 ; shift
crlfnul=$1 ; shift
- create_gitattributes "$attr" $ident $aeol &&
- git config core.autocrlf $crlf &&
+ test_expect_success "setup config for checkout attr=$attr ident=$ident aeol=$aeol core.autocrlf=$crlf" '
+ create_gitattributes "$attr" $ident $aeol &&
+ git config core.autocrlf $crlf
+ '
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" '
--
2.39.0.1071.g97ce8966538
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests
2022-12-19 10:19 ` [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
@ 2022-12-19 12:07 ` René Scharfe
0 siblings, 0 replies; 83+ messages in thread
From: René Scharfe @ 2022-12-19 12:07 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Eric Sunshine, Torsten Bögershausen
Am 19.12.22 um 11:19 schrieb Ævar Arnfjörð Bjarmason:
> 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 | 66 +++++++++++++++++++++++++-------------------
> 1 file changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index a94ac1eae37..2f57c8669cb 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
> @@ -289,17 +299,17 @@ checkout_files () {
> lfmixcrlf=$1 ; shift
> lfmixcr=$1 ; shift
> crlfnul=$1 ; shift
> - create_gitattributes "$attr" $ident $aeol &&
> - git config core.autocrlf $crlf &&
> + test_expect_success "setup config for checkout attr=$attr ident=$ident aeol=$aeol core.autocrlf=$crlf" '
> + create_gitattributes "$attr" $ident $aeol &&
> + git config core.autocrlf $crlf
> + '
> 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
> + '
OK, but why have test_expect_success inside the loop here, while a simlilar
loop is inside the two test_expect_success bodies above? Do we really need
five invocations here instead of one? It's just setup code.
> done
>
> test_expect_success "ls-files --eol attr=$attr $ident aeol=$aeol core.autocrlf=$crlf core.eol=$ceol" '
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
2022-12-19 10:19 ` [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-20 0:09 ` Junio C Hamano
2022-12-27 16:40 ` Phillip Wood
2022-12-19 10:19 ` [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
7 siblings, 2 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Fix code added in b319ef70a94 (Add a small patch-mode testing library,
2009-08-13) to use &&-chaining.
This avoids losing both the exit code of a "git" and the "cat"
processes.
This fixes cases where we'd have e.g. missed memory leaks under
SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
discovered it while looking at leaks in related code.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-patch-mode.sh | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
index cfd76bf987b..89ca1f78055 100644
--- a/t/lib-patch-mode.sh
+++ b/t/lib-patch-mode.sh
@@ -29,8 +29,12 @@ set_and_save_state () {
# verify_state <path> <expected-worktree-content> <expected-index-content>
verify_state () {
- test "$(cat "$1")" = "$2" &&
- test "$(git show :"$1")" = "$3"
+ echo "$2" >expect &&
+ test_cmp expect "$1" &&
+
+ echo "$3" >expect &&
+ git show :"$1" >actual &&
+ test_cmp expect actual
}
# verify_saved_state <path>
@@ -46,5 +50,6 @@ save_head () {
}
verify_saved_head () {
- test "$(cat _head)" = "$(git rev-parse HEAD)"
+ git rev-parse HEAD >actual &&
+ test_cmp _head actual
}
--
2.39.0.1071.g97ce8966538
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-19 10:19 ` [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-20 0:09 ` Junio C Hamano
2022-12-27 16:40 ` Phillip Wood
1 sibling, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-20 0:09 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
> 2009-08-13) to use &&-chaining.
>
> This avoids losing both the exit code of a "git" and the "cat"
> processes.
>
> This fixes cases where we'd have e.g. missed memory leaks under
> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
> discovered it while looking at leaks in related code.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/lib-patch-mode.sh | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
Looks quite sensible.
> diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
> index cfd76bf987b..89ca1f78055 100644
> --- a/t/lib-patch-mode.sh
> +++ b/t/lib-patch-mode.sh
> @@ -29,8 +29,12 @@ set_and_save_state () {
>
> # verify_state <path> <expected-worktree-content> <expected-index-content>
> verify_state () {
> - test "$(cat "$1")" = "$2" &&
> - test "$(git show :"$1")" = "$3"
> + echo "$2" >expect &&
> + test_cmp expect "$1" &&
> +
> + echo "$3" >expect &&
> + git show :"$1" >actual &&
> + test_cmp expect actual
> }
>
> # verify_saved_state <path>
> @@ -46,5 +50,6 @@ save_head () {
> }
>
> verify_saved_head () {
> - test "$(cat _head)" = "$(git rev-parse HEAD)"
> + git rev-parse HEAD >actual &&
> + test_cmp _head actual
> }
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-19 10:19 ` [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
2022-12-20 0:09 ` Junio C Hamano
@ 2022-12-27 16:40 ` Phillip Wood
2022-12-27 18:14 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 83+ messages in thread
From: Phillip Wood @ 2022-12-27 16:40 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen
Hi Ævar
On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
> 2009-08-13) to use &&-chaining.
>
> This avoids losing both the exit code of a "git" and the "cat"
> processes.
>
> This fixes cases where we'd have e.g. missed memory leaks under
> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
> discovered it while looking at leaks in related code.
> [...]
> # verify_saved_state <path>
> @@ -46,5 +50,6 @@ save_head () {
> }
>
> verify_saved_head () {
> - test "$(cat _head)" = "$(git rev-parse HEAD)"
> + git rev-parse HEAD >actual &&
> + test_cmp _head actual
Aren't these two lines are re-implementing test_cmp_rev()?
Best Wishes
Phillip
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes
2022-12-27 16:40 ` Phillip Wood
@ 2022-12-27 18:14 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-27 18:14 UTC (permalink / raw)
To: phillip.wood
Cc: git, Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen
On Tue, Dec 27 2022, Phillip Wood wrote:
> Hi Ævar
>
> On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
>> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
>> 2009-08-13) to use &&-chaining.
>> This avoids losing both the exit code of a "git" and the "cat"
>> processes.
>> This fixes cases where we'd have e.g. missed memory leaks under
>> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
>> discovered it while looking at leaks in related code.
>> [...] # verify_saved_state <path>
>> @@ -46,5 +50,6 @@ save_head () {
>> }
>> verify_saved_head () {
>> - test "$(cat _head)" = "$(git rev-parse HEAD)"
>> + git rev-parse HEAD >actual &&
>> + test_cmp _head actual
>
> Aren't these two lines are re-implementing test_cmp_rev()?
It does --verify, and this does not.
Could we use it? Yes, but I wanted to narrowly focus on just fixing the
lost exit codes in this series. Once you start to untangle "save_head"
and "verify_saved_head" you'll see that whether we narrowly used a
helper here or not isn't the only thing we could improve.
But such an improvement would make use use --verify, and we'd then want
to use that "--verify" for the earlier saved_head too, etc.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
2022-12-19 10:19 ` [PATCH v4 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
2022-12-19 10:19 ` [PATCH v4 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-20 0:20 ` Junio C Hamano
2022-12-19 10:19 ` [PATCH v4 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
7 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Rewrite tests that ran "git" inside command substitution and lost the
exit status of "git" so that we notice the failing "git".
Have them use modern patterns such as a "test_cmp" of the expected
outputs instead, and avoid needlessly spawning sub-shell in favor of
using "git -C <dir>".
We'll fix more of these these in the subsequent commit, for now we're
only converting the cases where this loss of exit code was combined
with spawning a sub-shell. The one exception to that is the casein
"t3200-branch.sh" where adjacent code didn't use a sub-shell, let's
convert that here as it's within the same hunk.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-httpd.sh | 5 +++--
t/lib-submodule-update.sh | 22 +++++++++-------------
t/t0060-path-utils.sh | 4 +++-
t/t3200-branch.sh | 13 +++++++------
t/t5605-clone-local.sh | 15 ++++++++++-----
t/t7402-submodule-rebase.sh | 14 +++++++++++---
6 files changed, 43 insertions(+), 30 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 608949ea80b..31e7fa3010c 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -217,8 +217,9 @@ test_http_push_nonff () {
git commit -a -m path2 --amend &&
test_must_fail git push -v origin >output 2>&1 &&
- (cd "$REMOTE_REPO" &&
- test $HEAD = $(git rev-parse --verify HEAD))
+ echo "$HEAD" >expect &&
+ git -C "$REMOTE_REPO" rev-parse --verify HEAD >actual &&
+ test_cmp expect actual
'
test_expect_success 'non-fast-forward push show ref status' '
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 2d31fcfda1f..d7c2b670b4a 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -168,20 +168,16 @@ replace_gitfile_with_git_dir () {
# Note that this only supports submodules at the root level of the
# superproject, with the default name, i.e. same as its path.
test_git_directory_is_unchanged () {
- (
- cd ".git/modules/$1" &&
- # does core.worktree point at the right place?
- test "$(git config core.worktree)" = "../../../$1" &&
- # remove it temporarily before comparing, as
- # "$1/.git/config" lacks it...
- git config --unset core.worktree
- ) &&
+ # does core.worktree point at the right place?
+ echo "../../../$1" >expect &&
+ git -C ".git/modules/$1" config core.worktree >actual &&
+ test_cmp expect actual &&
+ # remove it temporarily before comparing, as
+ # "$1/.git/config" lacks it...
+ git -C ".git/modules/$1" config --unset core.worktree &&
diff -r ".git/modules/$1" "$1/.git" &&
- (
- # ... and then restore.
- cd ".git/modules/$1" &&
- git config core.worktree "../../../$1"
- )
+ # ... and then restore.
+ git -C ".git/modules/$1" config core.worktree "../../../$1"
}
test_git_directory_exists () {
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 68e29c904a6..53ec717cbca 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -255,7 +255,9 @@ test_expect_success 'prefix_path rejects absolute path to dir with same beginnin
test_expect_success SYMLINKS 'prefix_path works with absolute path to a symlink to work tree having same beginning as work tree' '
git init repo &&
ln -s repo repolink &&
- test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
+ echo "a" >expect &&
+ test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&
+ test_cmp expect actual
'
relative_path /foo/a/b/c/ /foo/a/b/ c/
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5a169b68d6a..f5fbb84262b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -242,12 +242,13 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
test_expect_success 'git branch -M baz bam should succeed within a worktree in which baz is checked out' '
git checkout -b baz &&
git worktree add -f bazdir baz &&
- (
- cd bazdir &&
- git branch -M baz bam &&
- test $(git rev-parse --abbrev-ref HEAD) = bam
- ) &&
- test $(git rev-parse --abbrev-ref HEAD) = bam &&
+ git -C "$bazdir" branch -M baz bam &&
+ echo "bam" >expect &&
+ git -C "$bazdir" rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual &&
+ echo "bam" >expect &&
+ git rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual &&
rm -r bazdir &&
git worktree prune
'
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 38b850c10ef..61a2342bc2c 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -15,8 +15,12 @@ test_expect_success 'preparing origin repository' '
: >file && git add . && git commit -m1 &&
git clone --bare . a.git &&
git clone --bare . x &&
- test "$(cd a.git && git config --bool core.bare)" = true &&
- test "$(cd x && git config --bool core.bare)" = true &&
+ echo true >expect &&
+ git -C a.git config --bool core.bare >actual &&
+ test_cmp expect actual &&
+ echo true >expect &&
+ git -C x config --bool core.bare >actual &&
+ test_cmp expect actual &&
git bundle create b1.bundle --all &&
git bundle create b2.bundle main &&
mkdir dir &&
@@ -28,9 +32,10 @@ test_expect_success 'preparing origin repository' '
test_expect_success 'local clone without .git suffix' '
git clone -l -s a b &&
- (cd b &&
- test "$(git config --bool core.bare)" = false &&
- git fetch)
+ echo false >expect &&
+ git -C b config --bool core.bare >actual &&
+ test_cmp expect actual &&
+ git -C b fetch
'
test_expect_success 'local clone with .git suffix' '
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index ebeca12a711..1927a862839 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -82,11 +82,19 @@ test_expect_success 'stash with a dirty submodule' '
CURRENT=$(cd submodule && git rev-parse HEAD) &&
git stash &&
test new != $(cat file) &&
- test submodule = $(git diff --name-only) &&
- test $CURRENT = $(cd submodule && git rev-parse HEAD) &&
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual &&
+
+ echo "$CURRENT" >expect &&
+ git -C submodule rev-parse HEAD >actual &&
+ test_cmp expect actual &&
+
git stash apply &&
test new = $(cat file) &&
- test $CURRENT = $(cd submodule && git rev-parse HEAD)
+ echo "$CURRENT" >expect &&
+ git -C submodule rev-parse HEAD >actual &&
+ test_cmp expect actual
'
--
2.39.0.1071.g97ce8966538
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
2022-12-19 10:19 ` [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
@ 2022-12-20 0:20 ` Junio C Hamano
0 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-20 0:20 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> - test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
> + echo "a" >expect &&
> + test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&
If we fail to cd to 'repo', "$(cd repo && pwd)/../repolink/a" would
silently expand to nonsense, but presumably "test-tool -C repo"
would fail loudly in such a case, so we should be OK here?
> @@ -28,9 +32,10 @@ test_expect_success 'preparing origin repository' '
>
> test_expect_success 'local clone without .git suffix' '
> git clone -l -s a b &&
> - (cd b &&
> - test "$(git config --bool core.bare)" = false &&
> - git fetch)
> + echo false >expect &&
> + git -C b config --bool core.bare >actual &&
> + test_cmp expect actual &&
> + git -C b fetch
> '
I am not sure if the above with full of "git -C" is strictly an
improvement over
(
cd b &&
echo false >expect &&
git config --bool core.bare >actual &&
test_cmp expect actual &&
git fetch
)
and even if it were, the reason why it is better would be vastly
different from the reason why it is better that we no longer do
"test $(cmd) = false". I very much hate the pattern described on
the commit title of this step (which by definition this patch fixes
many instances of). I.e.
(cd ...; test <op> $(git ...))
might be something you find that needs improvements, but "don't lose
exit status" ONLY applies to the "test <op> $(git ...)" part, and
the other half, i.e. (cd ...), has nothing to do with "don't lose
exit status" badness.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 4/6] tests: don't lose exit status with "test <op> $(git ...)"
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2022-12-19 10:19 ` [PATCH v4 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-26 1:14 ` Junio C Hamano
2022-12-19 10:19 ` [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
7 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
As with the preceding commit, rewrite tests that ran "git" inside
command substitution and lost the exit status of "git" so that we
notice the failing "git". This time around we're converting cases that
didn't involve a containing sub-shell around the command substitution.
In the case of "t0060-path-utils.sh" and
"t2005-checkout-index-symlinks.sh" convert the relevant code to using
the modern style of indentation and newline wrapping while having to
change it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-submodule-update.sh | 4 +-
t/t0001-init.sh | 9 ++-
t/t0002-gitfile.sh | 4 +-
t/t0060-path-utils.sh | 103 +++++++++++++++++++++--------
t/t0100-previous.sh | 8 ++-
t/t1504-ceiling-dirs.sh | 8 ++-
t/t2005-checkout-index-symlinks.sh | 8 ++-
t/t5522-pull-symlink.sh | 4 +-
t/t7402-submodule-rebase.sh | 9 ++-
t/t7504-commit-msg-hook.sh | 4 +-
t/t7810-grep.sh | 4 +-
11 files changed, 120 insertions(+), 45 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index d7c2b670b4a..dee14992c52 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -185,7 +185,9 @@ test_git_directory_exists () {
if test -f sub1/.git
then
# does core.worktree point at the right place?
- test "$(git -C .git/modules/$1 config core.worktree)" = "../../../$1"
+ echo "../../../$1" >expect &&
+ git -C ".git/modules/$1" config core.worktree >actual &&
+ test_cmp expect actual
fi
}
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index d479303efa0..30a6edca1d2 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -598,9 +598,14 @@ test_expect_success 'invalid default branch name' '
test_expect_success 'branch -m with the initial branch' '
git init rename-initial &&
git -C rename-initial branch -m renamed &&
- test renamed = $(git -C rename-initial symbolic-ref --short HEAD) &&
+ echo renamed >expect &&
+ git -C rename-initial symbolic-ref --short HEAD >actual &&
+ test_cmp expect actual &&
+
git -C rename-initial branch -m renamed again &&
- test again = $(git -C rename-initial symbolic-ref --short HEAD)
+ echo again >expect &&
+ git -C rename-initial symbolic-ref --short HEAD >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 26eaca095a2..e013d38f485 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -33,7 +33,9 @@ test_expect_success 'bad setup: invalid .git file path' '
test_expect_success 'final setup + check rev-parse --git-dir' '
echo "gitdir: $REAL" >.git &&
- test "$REAL" = "$(git rev-parse --git-dir)"
+ echo "$REAL" >expect &&
+ git rev-parse --git-dir >actual &&
+ test_cmp expect actual
'
test_expect_success 'check hash-object' '
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 53ec717cbca..6490ad5ca1b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -10,20 +10,27 @@ TEST_PASSES_SANITIZE_LEAK=true
norm_path() {
expected=$(test-tool path-utils print_path "$2")
- test_expect_success $3 "normalize path: $1 => $2" \
- "test \"\$(test-tool path-utils normalize_path_copy '$1')\" = '$expected'"
+ test_expect_success $3 "normalize path: $1 => $2" "
+ echo '$expected' >expect &&
+ test-tool path-utils normalize_path_copy '$1' >actual &&
+ test_cmp expect actual
+ "
}
relative_path() {
expected=$(test-tool path-utils print_path "$3")
- test_expect_success $4 "relative path: $1 $2 => $3" \
- "test \"\$(test-tool path-utils relative_path '$1' '$2')\" = '$expected'"
+ test_expect_success $4 "relative path: $1 $2 => $3" "
+ echo '$expected' >expect &&
+ test-tool path-utils relative_path '$1' '$2' >actual &&
+ test_cmp expect actual
+ "
}
test_submodule_relative_url() {
test_expect_success "test_submodule_relative_url: $1 $2 $3 => $4" "
- actual=\$(test-tool submodule resolve-relative-url '$1' '$2' '$3') &&
- test \"\$actual\" = '$4'
+ echo '$4' >expect &&
+ test-tool submodule resolve-relative-url '$1' '$2' '$3' >actual &&
+ test_cmp expect actual
"
}
@@ -64,9 +71,11 @@ ancestor() {
expected=$(($expected-$rootslash+$rootoff))
;;
esac
- test_expect_success $4 "longest ancestor: $1 $2 => $expected" \
- "actual=\$(test-tool path-utils longest_ancestor_length '$1' '$2') &&
- test \"\$actual\" = '$expected'"
+ test_expect_success $4 "longest ancestor: $1 $2 => $expected" "
+ echo '$expected' >expect &&
+ test-tool path-utils longest_ancestor_length '$1' '$2' >actual &&
+ test_cmp expect actual
+ "
}
# Some absolute path tests should be skipped on Windows due to path mangling
@@ -166,8 +175,10 @@ ancestor D:/Users/me C:/ -1 MINGW
ancestor //server/share/my-directory //server/share/ 14 MINGW
test_expect_success 'strip_path_suffix' '
- test c:/msysgit = $(test-tool path-utils strip_path_suffix \
- c:/msysgit/libexec//git-core libexec/git-core)
+ echo c:/msysgit >expect &&
+ test-tool path-utils strip_path_suffix \
+ c:/msysgit/libexec//git-core libexec/git-core >actual &&
+ test_cmp expect actual
'
test_expect_success 'absolute path rejects the empty string' '
@@ -188,35 +199,61 @@ test_expect_success 'real path rejects the empty string' '
'
test_expect_success POSIX 'real path works on absolute paths 1' '
+ echo / >expect &&
+ test-tool path-utils real_path "/" >actual &&
+ test_cmp expect actual &&
+
nopath="hopefully-absent-path" &&
- test "/" = "$(test-tool path-utils real_path "/")" &&
- test "/$nopath" = "$(test-tool path-utils real_path "/$nopath")"
+ echo "/$nopath" >expect &&
+ test-tool path-utils real_path "/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success 'real path works on absolute paths 2' '
- nopath="hopefully-absent-path" &&
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "$d")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "$d/$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "$d" >actual &&
+ test_cmp expect actual &&
+
+ nopath="hopefully-absent-path" &&
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "$d/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success POSIX 'real path removes extra leading slashes' '
+ echo "/" >expect &&
+ test-tool path-utils real_path "///" >actual &&
+ test_cmp expect actual &&
+
nopath="hopefully-absent-path" &&
- test "/" = "$(test-tool path-utils real_path "///")" &&
- test "/$nopath" = "$(test-tool path-utils real_path "///$nopath")" &&
+ echo "/$nopath" >expect &&
+ test-tool path-utils real_path "///$nopath" >actual &&
+ test_cmp expect actual &&
+
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "//$d")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "//$d/$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "//$d" >actual &&
+ test_cmp expect actual &&
+
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "//$d/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success 'real path removes other extra slashes' '
- nopath="hopefully-absent-path" &&
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "$d///")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "$d///$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "$d///" >actual &&
+ test_cmp expect actual &&
+
+ nopath="hopefully-absent-path" &&
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "$d///$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'real path works on symlinks' '
@@ -227,19 +264,29 @@ test_expect_success SYMLINKS 'real path works on symlinks' '
mkdir third &&
dir="$(cd .git && pwd -P)" &&
dir2=third/../second/other/.git &&
- test "$dir" = "$(test-tool path-utils real_path $dir2)" &&
+ echo "$dir" >expect &&
+ test-tool path-utils real_path $dir2 >actual &&
+ test_cmp expect actual &&
file="$dir"/index &&
- test "$file" = "$(test-tool path-utils real_path $dir2/index)" &&
+ echo "$file" >expect &&
+ test-tool path-utils real_path $dir2/index >actual &&
+ test_cmp expect actual &&
basename=blub &&
- test "$dir/$basename" = "$(cd .git && test-tool path-utils real_path "$basename")" &&
+ echo "$dir/$basename" >expect &&
+ test-tool -C .git path-utils real_path "$basename" >actual &&
+ test_cmp expect actual &&
ln -s ../first/file .git/syml &&
sym="$(cd first && pwd -P)"/file &&
- test "$sym" = "$(test-tool path-utils real_path "$dir2/syml")"
+ echo "$sym" >expect &&
+ test-tool path-utils real_path "$dir2/syml" >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'prefix_path works with absolute paths to work tree symlinks' '
ln -s target symlink &&
- test "$(test-tool path-utils prefix_path prefix "$(pwd)/symlink")" = "symlink"
+ echo "symlink" >expect &&
+ test-tool path-utils prefix_path prefix "$(pwd)/symlink" >actual &&
+ test_cmp expect actual
'
test_expect_success 'prefix_path works with only absolute path to work tree' '
diff --git a/t/t0100-previous.sh b/t/t0100-previous.sh
index a16cc3d2983..70a3223f219 100755
--- a/t/t0100-previous.sh
+++ b/t/t0100-previous.sh
@@ -12,7 +12,9 @@ test_expect_success 'branch -d @{-1}' '
test_commit A &&
git checkout -b junk &&
git checkout - &&
- test "$(git symbolic-ref HEAD)" = refs/heads/main &&
+ echo refs/heads/main >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
git branch -d @{-1} &&
test_must_fail git rev-parse --verify refs/heads/junk
'
@@ -21,7 +23,9 @@ test_expect_success 'branch -d @{-12} when there is not enough switches yet' '
git reflog expire --expire=now &&
git checkout -b junk2 &&
git checkout - &&
- test "$(git symbolic-ref HEAD)" = refs/heads/main &&
+ echo refs/heads/main >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
test_must_fail git branch -d @{-12} &&
git rev-parse --verify refs/heads/main
'
diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
index 0fafcf9dde3..c1679e31d8a 100755
--- a/t/t1504-ceiling-dirs.sh
+++ b/t/t1504-ceiling-dirs.sh
@@ -6,8 +6,12 @@ TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_prefix() {
- test_expect_success "$1" \
- "test '$2' = \"\$(git rev-parse --show-prefix)\""
+ local expect="$2" &&
+ test_expect_success "$1: git rev-parse --show-prefix is '$2'" '
+ echo "$expect" >expect &&
+ git rev-parse --show-prefix >actual &&
+ test_cmp expect actual
+ '
}
test_fail() {
diff --git a/t/t2005-checkout-index-symlinks.sh b/t/t2005-checkout-index-symlinks.sh
index 112682a45a1..67d18cfa104 100755
--- a/t/t2005-checkout-index-symlinks.sh
+++ b/t/t2005-checkout-index-symlinks.sh
@@ -22,8 +22,10 @@ test_expect_success \
git checkout-index symlink &&
test -f symlink'
-test_expect_success \
-'the file must be the blob we added during the setup' '
-test "$(git hash-object -t blob symlink)" = $l'
+test_expect_success 'the file must be the blob we added during the setup' '
+ echo "$l" >expect &&
+ git hash-object -t blob symlink >actual &&
+ test_cmp expect actual
+'
test_done
diff --git a/t/t5522-pull-symlink.sh b/t/t5522-pull-symlink.sh
index bcff460d0a2..9fb73a8c3eb 100755
--- a/t/t5522-pull-symlink.sh
+++ b/t/t5522-pull-symlink.sh
@@ -78,7 +78,9 @@ test_expect_success SYMLINKS 'pushing from symlinked subdir' '
git commit -m push ./file &&
git push
) &&
- test push = $(git show HEAD:subdir/file)
+ echo push >expect &&
+ git show HEAD:subdir/file >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index 1927a862839..c74798e8d24 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -55,12 +55,15 @@ chmod a+x fake-editor.sh
test_expect_success 'interactive rebase with a dirty submodule' '
- test submodule = $(git diff --name-only) &&
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual &&
HEAD=$(git rev-parse HEAD) &&
GIT_EDITOR="\"$(pwd)/fake-editor.sh\"" EDITOR_TEXT="pick $HEAD" \
git rebase -i HEAD^ &&
- test submodule = $(git diff --name-only)
-
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual
'
test_expect_success 'rebase with dirty file and submodule fails' '
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index 07ca46fb0d5..d1255228d5f 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -102,7 +102,9 @@ test_expect_success 'setup: commit-msg hook that always fails' '
'
commit_msg_is () {
- test "$(git log --pretty=format:%s%b -1)" = "$1"
+ printf "%s" "$1" >expect &&
+ git log --pretty=format:%s%b -1 >actual &&
+ test_cmp expect actual
}
test_expect_success 'with failing hook' '
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 8eded6ab274..39d6d713ecb 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1001,7 +1001,9 @@ test_expect_success 'log --committer does not search in timestamp' '
test_expect_success 'grep with CE_VALID file' '
git update-index --assume-unchanged t/t &&
rm t/t &&
- test "$(git grep test)" = "t/t:test" &&
+ echo "t/t:test" >expect &&
+ git grep test >actual &&
+ test_cmp expect actual &&
git update-index --no-assume-unchanged t/t &&
git checkout t/t
'
--
2.39.0.1071.g97ce8966538
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v4 4/6] tests: don't lose exit status with "test <op> $(git ...)"
2022-12-19 10:19 ` [PATCH v4 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
@ 2022-12-26 1:14 ` Junio C Hamano
0 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-26 1:14 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> As with the preceding commit, rewrite tests that ran "git" inside
> command substitution and lost the exit status of "git" so that we
> notice the failing "git". This time around we're converting cases that
> didn't involve a containing sub-shell around the command substitution.
>
> In the case of "t0060-path-utils.sh" and
> "t2005-checkout-index-symlinks.sh" convert the relevant code to using
> the modern style of indentation and newline wrapping while having to
> change it.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
Unlike the previous one, this looks sharply focused to deal with
having $(git ...) as one of the arguments to "test". Looking good.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2022-12-19 10:19 ` [PATCH v4 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-26 1:18 ` Junio C Hamano
2022-12-27 16:44 ` Phillip Wood
2022-12-19 10:19 ` [PATCH v4 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
7 siblings, 2 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Change tests that would lose the "git" exit code via a negation
pattern to:
- In the case of "t0055-beyond-symlinks.sh" compare against the
expected output instead.
We could use the same pattern as in the "t3700-add.sh" below, doing
so would have the advantage that if we added an earlier test we
wouldn't need to adjust the "expect" output.
But as "t0055-beyond-symlinks.sh" is a small and focused test (less
than 40 lines in total) let's use "test_cmp" instead.
- For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
and use "test_must_be_empty" to assert that it's not there. If we used
"grep" we'd get a non-zero exit code.
We could use "test_expect_code 1 grep", but this is more consistent
with existing patterns in the test suite.
We can also remove a repeated invocation of "git ls-files" for the
last test that's being modified in that file, and search the
existing "files" output instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
t/t3700-add.sh | 18 +++++++++++++-----
2 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
index 6bada370225..c3eb1158ef9 100755
--- a/t/t0055-beyond-symlinks.sh
+++ b/t/t0055-beyond-symlinks.sh
@@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
test_must_fail git update-index --add c/d &&
- ! ( git ls-files | grep c/d )
+ cat >expect <<-\EOF &&
+ a
+ b/d
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'add beyond symlinks' '
test_must_fail git add c/d &&
- ! ( git ls-files | grep c/d )
+ cat >expect <<-\EOF &&
+ a
+ b/d
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 51afbd7b24a..82dd768944f 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
test_expect_success '.gitignore is honored' '
git add . &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones without -f' '
test_must_fail git add a.?? &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones without -f' '
test_must_fail git add d.?? &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones but add others' '
touch a.if &&
test_must_fail git add a.?? &&
- ! (git ls-files | grep "\\.ig") &&
- (git ls-files | grep a.if)
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual &&
+ grep a.if files
'
test_expect_success 'add ignored ones with -f' '
--
2.39.0.1071.g97ce8966538
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-19 10:19 ` [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
@ 2022-12-26 1:18 ` Junio C Hamano
2022-12-27 16:44 ` Phillip Wood
1 sibling, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-26 1:18 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Change tests that would lose the "git" exit code via a negation
> pattern to:
Looking good. thanks.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-19 10:19 ` [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
2022-12-26 1:18 ` Junio C Hamano
@ 2022-12-27 16:44 ` Phillip Wood
2022-12-27 17:13 ` Phillip Wood
2022-12-27 23:16 ` Junio C Hamano
1 sibling, 2 replies; 83+ messages in thread
From: Phillip Wood @ 2022-12-27 16:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen
Hi Ævar
On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
> Change tests that would lose the "git" exit code via a negation
> pattern to:
>
> - In the case of "t0055-beyond-symlinks.sh" compare against the
> expected output instead.
>
> We could use the same pattern as in the "t3700-add.sh" below, doing
> so would have the advantage that if we added an earlier test we
> wouldn't need to adjust the "expect" output.
>
> But as "t0055-beyond-symlinks.sh" is a small and focused test (less
> than 40 lines in total) let's use "test_cmp" instead.
>
> - For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
> and use "test_must_be_empty" to assert that it's not there. If we used
> "grep" we'd get a non-zero exit code.
>
> We could use "test_expect_code 1 grep", but this is more consistent
> with existing patterns in the test suite.
It seems strange to use sed here, you could just keep using grep and
check the output is empty if you don't want to use test_expect_code.
There is also no need to redirect the input of the sed commands.
Best Wishes
Phillip
> We can also remove a repeated invocation of "git ls-files" for the
> last test that's being modified in that file, and search the
> existing "files" output instead.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
> t/t3700-add.sh | 18 +++++++++++++-----
> 2 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
> index 6bada370225..c3eb1158ef9 100755
> --- a/t/t0055-beyond-symlinks.sh
> +++ b/t/t0055-beyond-symlinks.sh
> @@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
>
> test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
> test_must_fail git update-index --add c/d &&
> - ! ( git ls-files | grep c/d )
> + cat >expect <<-\EOF &&
> + a
> + b/d
> + EOF
> + git ls-files >actual &&
> + test_cmp expect actual
> '
>
> test_expect_success SYMLINKS 'add beyond symlinks' '
> test_must_fail git add c/d &&
> - ! ( git ls-files | grep c/d )
> + cat >expect <<-\EOF &&
> + a
> + b/d
> + EOF
> + git ls-files >actual &&
> + test_cmp expect actual
> '
>
> test_done
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index 51afbd7b24a..82dd768944f 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
>
> test_expect_success '.gitignore is honored' '
> git add . &&
> - ! (git ls-files | grep "\\.ig")
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual
> '
>
> test_expect_success 'error out when attempting to add ignored ones without -f' '
> test_must_fail git add a.?? &&
> - ! (git ls-files | grep "\\.ig")
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual
> '
>
> test_expect_success 'error out when attempting to add ignored ones without -f' '
> test_must_fail git add d.?? &&
> - ! (git ls-files | grep "\\.ig")
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual
> '
>
> test_expect_success 'error out when attempting to add ignored ones but add others' '
> touch a.if &&
> test_must_fail git add a.?? &&
> - ! (git ls-files | grep "\\.ig") &&
> - (git ls-files | grep a.if)
> + git ls-files >files &&
> + sed -n "/\\.ig/p" <files >actual &&
> + test_must_be_empty actual &&
> + grep a.if files
> '
>
> test_expect_success 'add ignored ones with -f' '
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-27 16:44 ` Phillip Wood
@ 2022-12-27 17:13 ` Phillip Wood
2022-12-27 23:16 ` Junio C Hamano
1 sibling, 0 replies; 83+ messages in thread
From: Phillip Wood @ 2022-12-27 17:13 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen
On 27/12/2022 16:44, Phillip Wood wrote:
> On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
>> - For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
>> and use "test_must_be_empty" to assert that it's not there. If we used
>> "grep" we'd get a non-zero exit code.
>>
>> We could use "test_expect_code 1 grep", but this is more consistent
>> with existing patterns in the test suite.
>
> It seems strange to use sed here, you could just keep using grep and
> check the output is empty if you don't want to use test_expect_code.
Sorry ignore that, using 'sed -n' means we don't have to worry about the
exit code.
> There is also no need to redirect the input of the sed commands.
>
> Best Wishes
>
> Phillip
>
>> We can also remove a repeated invocation of "git ls-files" for the
>> last test that's being modified in that file, and search the
>> existing "files" output instead.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
>> t/t3700-add.sh | 18 +++++++++++++-----
>> 2 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
>> index 6bada370225..c3eb1158ef9 100755
>> --- a/t/t0055-beyond-symlinks.sh
>> +++ b/t/t0055-beyond-symlinks.sh
>> @@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
>> test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
>> test_must_fail git update-index --add c/d &&
>> - ! ( git ls-files | grep c/d )
>> + cat >expect <<-\EOF &&
>> + a
>> + b/d
>> + EOF
>> + git ls-files >actual &&
>> + test_cmp expect actual
>> '
>> test_expect_success SYMLINKS 'add beyond symlinks' '
>> test_must_fail git add c/d &&
>> - ! ( git ls-files | grep c/d )
>> + cat >expect <<-\EOF &&
>> + a
>> + b/d
>> + EOF
>> + git ls-files >actual &&
>> + test_cmp expect actual
>> '
>> test_done
>> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
>> index 51afbd7b24a..82dd768944f 100755
>> --- a/t/t3700-add.sh
>> +++ b/t/t3700-add.sh
>> @@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
>> test_expect_success '.gitignore is honored' '
>> git add . &&
>> - ! (git ls-files | grep "\\.ig")
>> + git ls-files >files &&
>> + sed -n "/\\.ig/p" <files >actual &&
>> + test_must_be_empty actual
>> '
>> test_expect_success 'error out when attempting to add ignored ones
>> without -f' '
>> test_must_fail git add a.?? &&
>> - ! (git ls-files | grep "\\.ig")
>> + git ls-files >files &&
>> + sed -n "/\\.ig/p" <files >actual &&
>> + test_must_be_empty actual
>> '
>> test_expect_success 'error out when attempting to add ignored ones
>> without -f' '
>> test_must_fail git add d.?? &&
>> - ! (git ls-files | grep "\\.ig")
>> + git ls-files >files &&
>> + sed -n "/\\.ig/p" <files >actual &&
>> + test_must_be_empty actual
>> '
>> test_expect_success 'error out when attempting to add ignored ones
>> but add others' '
>> touch a.if &&
>> test_must_fail git add a.?? &&
>> - ! (git ls-files | grep "\\.ig") &&
>> - (git ls-files | grep a.if)
>> + git ls-files >files &&
>> + sed -n "/\\.ig/p" <files >actual &&
>> + test_must_be_empty actual &&
>> + grep a.if files
>> '
>> test_expect_success 'add ignored ones with -f' '
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2022-12-27 16:44 ` Phillip Wood
2022-12-27 17:13 ` Phillip Wood
@ 2022-12-27 23:16 ` Junio C Hamano
1 sibling, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-27 23:16 UTC (permalink / raw)
To: Phillip Wood
Cc: Ævar Arnfjörð Bjarmason, git, René Scharfe,
Eric Sunshine, Torsten Bögershausen
Phillip Wood <phillip.wood123@gmail.com> writes:
> It seems strange to use sed here, you could just keep using grep and
> check the output is empty if you don't want to use
> test_expect_code.
I am not sure what you mean by test_expect_code, but we can do the
"! grep -e pattern file" to ensure that the unwanted pattern does
not exist. Unlike our command, we do not worry about system "grep"
being buggy, so if it yields non-zero, it is because no line in the
file matches the pattern. After all, that is what the original code
fixed by this patch wanted to check.
The "do not lose exit code of the git command" part of the goal is
achieved by breaking the pipeline already, and we can keep the test
that uses grep.
Having said that, switching to "sed" is not too bad. If we wanted
to expect N lines that matches the pattern in the file, with grep,
we need to do "! grep" dance when (and only when) N is zero. With
sed, we do not have to worry about it.
> There is also no need to redirect the input of the
> sed commands.
That's true.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v4 6/6] tests: don't lose misc "git" exit codes
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2022-12-19 10:19 ` [PATCH v4 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
@ 2022-12-19 10:19 ` Ævar Arnfjörð Bjarmason
2022-12-27 16:46 ` Phillip Wood
2022-12-20 0:06 ` [PATCH v4 0/6] tests: fix ignored & hidden " Junio C Hamano
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
7 siblings, 1 reply; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-19 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Ævar Arnfjörð Bjarmason
Fix a few miscellaneous cases where:
- We lost the "git" exit code via "git ... | grep"
- Likewise by having a $(git) argument to git itself
- Used "test -z" to check that a command emitted no output, we can use
"test_must_be_empty" and &&-chaining instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1401-symbolic-ref.sh | 3 ++-
t/t3701-add-interactive.sh | 8 +++++---
t/t7516-commit-races.sh | 3 ++-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index d708acdb819..5e36899d207 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -33,7 +33,8 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
reset_to_sane
test_expect_success 'symbolic-ref refuses bare sha1' '
- test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
+ rev=$(git rev-parse HEAD) &&
+ test_must_fail git symbolic-ref HEAD "$rev"
'
reset_to_sane
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 5841f280fb2..f1fe5d60677 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' '
echo content >>file &&
chmod +x file &&
printf "y\\ny\\n" | git add -p &&
- git diff --cached file | grep "new mode" &&
- git diff --cached file | grep "+content" &&
- test -z "$(git diff file)"
+ git diff --cached file >out &&
+ grep "new mode" out &&
+ grep "+content" out &&
+ git diff file >out &&
+ test_must_be_empty out
'
# end of tests disabled when filemode is not usable
diff --git a/t/t7516-commit-races.sh b/t/t7516-commit-races.sh
index f2ce14e9071..2d38a16480e 100755
--- a/t/t7516-commit-races.sh
+++ b/t/t7516-commit-races.sh
@@ -10,7 +10,8 @@ test_expect_success 'race to create orphan commit' '
test_must_fail env EDITOR=./hare-editor git commit --allow-empty -m tortoise -e &&
git show -s --pretty=format:%s >subject &&
grep hare subject &&
- test -z "$(git show -s --pretty=format:%P)"
+ git show -s --pretty=format:%P >out &&
+ test_must_be_empty out
'
test_expect_success 'race to create non-orphan commit' '
--
2.39.0.1071.g97ce8966538
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v4 6/6] tests: don't lose misc "git" exit codes
2022-12-19 10:19 ` [PATCH v4 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-27 16:46 ` Phillip Wood
2022-12-27 18:18 ` Ævar Arnfjörð Bjarmason
2022-12-27 23:17 ` Junio C Hamano
0 siblings, 2 replies; 83+ messages in thread
From: Phillip Wood @ 2022-12-27 16:46 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen
Hi Ævar
On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
> Fix a few miscellaneous cases where:
>
> - We lost the "git" exit code via "git ... | grep"
> - Likewise by having a $(git) argument to git itself
> - Used "test -z" to check that a command emitted no output, we can use
> "test_must_be_empty" and &&-chaining instead.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> [...]
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 5841f280fb2..f1fe5d60677 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' '
> echo content >>file &&
> chmod +x file &&
> printf "y\\ny\\n" | git add -p &&
> - git diff --cached file | grep "new mode" &&
> - git diff --cached file | grep "+content" &&
> - test -z "$(git diff file)"
> + git diff --cached file >out &&
> + grep "new mode" out &&
> + grep "+content" out &&
> + git diff file >out &&
> + test_must_be_empty out
"git diff --exit-code file" would suffice here, we don't need to
redirect the output and check that it is empty.
Best Wishes
Phillip
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 6/6] tests: don't lose misc "git" exit codes
2022-12-27 16:46 ` Phillip Wood
@ 2022-12-27 18:18 ` Ævar Arnfjörð Bjarmason
2022-12-27 23:17 ` Junio C Hamano
1 sibling, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-27 18:18 UTC (permalink / raw)
To: phillip.wood
Cc: git, Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen
On Tue, Dec 27 2022, Phillip Wood wrote:
> Hi Ævar
>
> On 19/12/2022 10:19, Ævar Arnfjörð Bjarmason wrote:
>> Fix a few miscellaneous cases where:
>> - We lost the "git" exit code via "git ... | grep"
>> - Likewise by having a $(git) argument to git itself
>> - Used "test -z" to check that a command emitted no output, we can use
>> "test_must_be_empty" and &&-chaining instead.
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> [...]
>> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
>> index 5841f280fb2..f1fe5d60677 100755
>> --- a/t/t3701-add-interactive.sh
>> +++ b/t/t3701-add-interactive.sh
>> @@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' '
>> echo content >>file &&
>> chmod +x file &&
>> printf "y\\ny\\n" | git add -p &&
>> - git diff --cached file | grep "new mode" &&
>> - git diff --cached file | grep "+content" &&
>> - test -z "$(git diff file)"
>> + git diff --cached file >out &&
>> + grep "new mode" out &&
>> + grep "+content" out &&
>> + git diff file >out &&
>> + test_must_be_empty out
>
> "git diff --exit-code file" would suffice here, we don't need to
> redirect the output and check that it is empty.
Correct.
Or even "git diff-files -s --exit-code", which might make things clearer
still, or have this use the "diff_cmp" helper defined in the test file,
as most of its siblings do.
But as with a sibling comment of mine I wanted to avoid starting to
refactoring these tests for general betterment, and just to narrowly
avoid losing the exit code.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 6/6] tests: don't lose misc "git" exit codes
2022-12-27 16:46 ` Phillip Wood
2022-12-27 18:18 ` Ævar Arnfjörð Bjarmason
@ 2022-12-27 23:17 ` Junio C Hamano
1 sibling, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-27 23:17 UTC (permalink / raw)
To: Phillip Wood
Cc: Ævar Arnfjörð Bjarmason, git, René Scharfe,
Eric Sunshine, Torsten Bögershausen
Phillip Wood <phillip.wood123@gmail.com> writes:
>> - test -z "$(git diff file)"
>> ...
>> + git diff file >out &&
>> + test_must_be_empty out
>
> "git diff --exit-code file" would suffice here, we don't need to
> redirect the output and check that it is empty.
Yes, but the test is trying to be faithful to not just the intent
but also the implementation of the original, I think.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH v4 0/6] tests: fix ignored & hidden exit codes
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2022-12-19 10:19 ` [PATCH v4 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
@ 2022-12-20 0:06 ` Junio C Hamano
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
7 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2022-12-20 0:06 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> I think this should address all of the feedback on the v3, except
> Junio's suggestion of perhaps re-arranging this series around file
> boundaries.
>
> Given the potential size of that range-diff I thought it was better to
> mosttly keep the same structure.
Sorry, I do not recall making such a suggestion but if I did, it
probably was an indication that the group of patches I suggested
such a change were unreviewable as-is, and by definition range-diff
is irrelevant if that is the case (i.e. the part that was
unreviewable would be freshly reviewed with reorganization).
In any case, I think we have been seeing patches to unhide the exit
status from more individual command invocations, a theme this series
should fit well. Thanks for working on it.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v5 0/6] tests: fix ignored & hidden exit codes
2022-12-19 10:19 ` [PATCH v4 0/6] tests: fix ignored & hidden " Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2022-12-20 0:06 ` [PATCH v4 0/6] tests: fix ignored & hidden " Junio C Hamano
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
` (6 more replies)
7 siblings, 7 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
Various fixes for "git" on the LHS of a pipe, but mostly when in
"test" expressions like:
test str = "$(git some-command)" &&
Changes since v4[1]:
* Tried to address all outstanding concernse, some with commit
message clarifications, others with code changes.
* The main code change here is to avoid converting code adjacent to
the fixed "test" code away from sub-shell use, per Junio's request.
Branch & CI for this at:
https://github.com/avar/git/tree/avar/fix-even-more-broken-test-code-hiding-failures-5
1. https://lore.kernel.org/git/cover-v4-0.6-00000000000-20221219T101240Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason (6):
auto-crlf tests: don't lose exit code in loops and outside tests
t/lib-patch-mode.sh: fix ignored exit codes
tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
tests: don't lose exit status with "test <op> $(git ...)"
tests: don't lose "git" exit codes in "! ( git ... | grep )"
tests: don't lose misc "git" exit codes
t/lib-httpd.sh | 8 ++-
t/lib-patch-mode.sh | 11 ++-
t/lib-submodule-update.sh | 26 ++++---
t/t0001-init.sh | 9 ++-
t/t0002-gitfile.sh | 4 +-
t/t0027-auto-crlf.sh | 66 ++++++++++--------
t/t0055-beyond-symlinks.sh | 14 +++-
t/t0060-path-utils.sh | 108 +++++++++++++++++++++--------
t/t0100-previous.sh | 8 ++-
t/t1401-symbolic-ref.sh | 3 +-
t/t1504-ceiling-dirs.sh | 8 ++-
t/t2005-checkout-index-symlinks.sh | 8 ++-
t/t3200-branch.sh | 8 ++-
t/t3700-add.sh | 18 +++--
t/t3701-add-interactive.sh | 8 ++-
t/t5522-pull-symlink.sh | 4 +-
t/t5605-clone-local.sh | 12 +++-
t/t7402-submodule-rebase.sh | 23 ++++--
t/t7504-commit-msg-hook.sh | 4 +-
t/t7516-commit-races.sh | 3 +-
t/t7810-grep.sh | 4 +-
21 files changed, 245 insertions(+), 112 deletions(-)
Range-diff against v4:
1: 68d276dd421 ! 1: 66d0f91e6aa auto-crlf tests: don't lose exit code in loops and outside tests
@@ Commit message
this failure under SANITIZE=leak when these invocations had errored
out, even under "--immediate".
+ For checkout_files() we could run one test_expect_success() instead of
+ the 5 we run now in a loop.
+
+ But as this function added in [1] is already taking pains to split up
+ its setup into phases (there are 5 more "test_expect_success()" at the
+ end of it already, see [1]), let's follow that existing convention.
+
+ 1. 343151dcbdf (t0027: combinations of core.autocrlf, core.eol and text, 2014-06-29)
+
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
2: d351075f0ab ! 2: 97f8b6a7b86 t/lib-patch-mode.sh: fix ignored exit codes
@@ Commit message
SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
discovered it while looking at leaks in related code.
+ For "verify_saved_head()" we could make use of "test_cmp_rev" with
+ some changes, but it uses "git rev-parse --verify", and this existing
+ test does not. I think it could safely use it, but let's avoid the
+ while-at-it change, and narrowly fix the exit code problem.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/lib-patch-mode.sh ##
3: f5b2489609c ! 3: d8f4c4a6d9e tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
@@ Commit message
exit status of "git" so that we notice the failing "git".
Have them use modern patterns such as a "test_cmp" of the expected
- outputs instead, and avoid needlessly spawning sub-shell in favor of
- using "git -C <dir>".
+ outputs instead.
We'll fix more of these these in the subsequent commit, for now we're
only converting the cases where this loss of exit code was combined
- with spawning a sub-shell. The one exception to that is the casein
- "t3200-branch.sh" where adjacent code didn't use a sub-shell, let's
- convert that here as it's within the same hunk.
+ with spawning a sub-shell.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ t/lib-httpd.sh: test_http_push_nonff () {
test_must_fail git push -v origin >output 2>&1 &&
- (cd "$REMOTE_REPO" &&
- test $HEAD = $(git rev-parse --verify HEAD))
-+ echo "$HEAD" >expect &&
-+ git -C "$REMOTE_REPO" rev-parse --verify HEAD >actual &&
-+ test_cmp expect actual
++ (
++ cd "$REMOTE_REPO" &&
++ echo "$HEAD" >expect &&
++ git rev-parse --verify HEAD >actual &&
++ test_cmp expect actual
++ )
'
test_expect_success 'non-fast-forward push show ref status' '
@@ t/t0060-path-utils.sh: test_expect_success 'prefix_path rejects absolute path to
ln -s repo repolink &&
- test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
+ echo "a" >expect &&
-+ test-tool -C repo path-utils prefix_path prefix "$(cd repo && pwd)/../repolink/a" >actual &&
++ repo_path="$(cd repo && pwd)" &&
++ test-tool -C repo path-utils prefix_path prefix "$repo_path/../repolink/a" >actual &&
+ test_cmp expect actual
'
relative_path /foo/a/b/c/ /foo/a/b/ c/
## t/t3200-branch.sh ##
-@@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
- test_expect_success 'git branch -M baz bam should succeed within a worktree in which baz is checked out' '
- git checkout -b baz &&
- git worktree add -f bazdir baz &&
-- (
-- cd bazdir &&
-- git branch -M baz bam &&
+@@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed within a worktree in w
+ (
+ cd bazdir &&
+ git branch -M baz bam &&
- test $(git rev-parse --abbrev-ref HEAD) = bam
-- ) &&
++ echo bam >expect &&
++ git rev-parse --abbrev-ref HEAD >actual &&
++ test_cmp expect actual
+ ) &&
- test $(git rev-parse --abbrev-ref HEAD) = bam &&
-+ git -C "$bazdir" branch -M baz bam &&
-+ echo "bam" >expect &&
-+ git -C "$bazdir" rev-parse --abbrev-ref HEAD >actual &&
-+ test_cmp expect actual &&
-+ echo "bam" >expect &&
++ echo bam >expect &&
+ git rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual &&
rm -r bazdir &&
@@ t/t5605-clone-local.sh: test_expect_success 'preparing origin repository' '
git bundle create b2.bundle main &&
mkdir dir &&
@@ t/t5605-clone-local.sh: test_expect_success 'preparing origin repository' '
-
test_expect_success 'local clone without .git suffix' '
git clone -l -s a b &&
-- (cd b &&
+ (cd b &&
- test "$(git config --bool core.bare)" = false &&
-- git fetch)
+ echo false >expect &&
-+ git -C b config --bool core.bare >actual &&
++ git config --bool core.bare >actual &&
+ test_cmp expect actual &&
-+ git -C b fetch
+ git fetch)
'
- test_expect_success 'local clone with .git suffix' '
## t/t7402-submodule-rebase.sh ##
@@ t/t7402-submodule-rebase.sh: test_expect_success 'stash with a dirty submodule' '
4: da66e5bf1c1 = 4: e6f56478b65 tests: don't lose exit status with "test <op> $(git ...)"
5: 9596702978e = 5: 55abdbef49f tests: don't lose "git" exit codes in "! ( git ... | grep )"
6: 94df7a1764e = 6: e7f10c0641a tests: don't lose misc "git" exit codes
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH v5 1/6] auto-crlf tests: don't lose exit code in loops and outside tests
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
6 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
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".
For checkout_files() we could run one test_expect_success() instead of
the 5 we run now in a loop.
But as this function added in [1] is already taking pains to split up
its setup into phases (there are 5 more "test_expect_success()" at the
end of it already, see [1]), let's follow that existing convention.
1. 343151dcbdf (t0027: combinations of core.autocrlf, core.eol and text, 2014-06-29)
Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t0027-auto-crlf.sh | 66 +++++++++++++++++++++++++-------------------
1 file changed, 38 insertions(+), 28 deletions(-)
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index a94ac1eae37..2f57c8669cb 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
@@ -289,17 +299,17 @@ checkout_files () {
lfmixcrlf=$1 ; shift
lfmixcr=$1 ; shift
crlfnul=$1 ; shift
- create_gitattributes "$attr" $ident $aeol &&
- git config core.autocrlf $crlf &&
+ test_expect_success "setup config for checkout attr=$attr ident=$ident aeol=$aeol core.autocrlf=$crlf" '
+ create_gitattributes "$attr" $ident $aeol &&
+ git config core.autocrlf $crlf
+ '
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" '
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply related [flat|nested] 83+ messages in thread
* [PATCH v5 2/6] t/lib-patch-mode.sh: fix ignored exit codes
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
6 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
Fix code added in b319ef70a94 (Add a small patch-mode testing library,
2009-08-13) to use &&-chaining.
This avoids losing both the exit code of a "git" and the "cat"
processes.
This fixes cases where we'd have e.g. missed memory leaks under
SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
discovered it while looking at leaks in related code.
For "verify_saved_head()" we could make use of "test_cmp_rev" with
some changes, but it uses "git rev-parse --verify", and this existing
test does not. I think it could safely use it, but let's avoid the
while-at-it change, and narrowly fix the exit code problem.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-patch-mode.sh | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
index cfd76bf987b..89ca1f78055 100644
--- a/t/lib-patch-mode.sh
+++ b/t/lib-patch-mode.sh
@@ -29,8 +29,12 @@ set_and_save_state () {
# verify_state <path> <expected-worktree-content> <expected-index-content>
verify_state () {
- test "$(cat "$1")" = "$2" &&
- test "$(git show :"$1")" = "$3"
+ echo "$2" >expect &&
+ test_cmp expect "$1" &&
+
+ echo "$3" >expect &&
+ git show :"$1" >actual &&
+ test_cmp expect actual
}
# verify_saved_state <path>
@@ -46,5 +50,6 @@ save_head () {
}
verify_saved_head () {
- test "$(cat _head)" = "$(git rev-parse HEAD)"
+ git rev-parse HEAD >actual &&
+ test_cmp _head actual
}
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply related [flat|nested] 83+ messages in thread
* [PATCH v5 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 1/6] auto-crlf tests: don't lose exit code in loops and outside tests Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 2/6] t/lib-patch-mode.sh: fix ignored exit codes Ævar Arnfjörð Bjarmason
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
6 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
Rewrite tests that ran "git" inside command substitution and lost the
exit status of "git" so that we notice the failing "git".
Have them use modern patterns such as a "test_cmp" of the expected
outputs instead.
We'll fix more of these these in the subsequent commit, for now we're
only converting the cases where this loss of exit code was combined
with spawning a sub-shell.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-httpd.sh | 8 ++++++--
t/lib-submodule-update.sh | 22 +++++++++-------------
t/t0060-path-utils.sh | 5 ++++-
t/t3200-branch.sh | 8 ++++++--
t/t5605-clone-local.sh | 12 +++++++++---
t/t7402-submodule-rebase.sh | 14 +++++++++++---
6 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 608949ea80b..b8b1d044e8b 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -217,8 +217,12 @@ test_http_push_nonff () {
git commit -a -m path2 --amend &&
test_must_fail git push -v origin >output 2>&1 &&
- (cd "$REMOTE_REPO" &&
- test $HEAD = $(git rev-parse --verify HEAD))
+ (
+ cd "$REMOTE_REPO" &&
+ echo "$HEAD" >expect &&
+ git rev-parse --verify HEAD >actual &&
+ test_cmp expect actual
+ )
'
test_expect_success 'non-fast-forward push show ref status' '
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 2d31fcfda1f..d7c2b670b4a 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -168,20 +168,16 @@ replace_gitfile_with_git_dir () {
# Note that this only supports submodules at the root level of the
# superproject, with the default name, i.e. same as its path.
test_git_directory_is_unchanged () {
- (
- cd ".git/modules/$1" &&
- # does core.worktree point at the right place?
- test "$(git config core.worktree)" = "../../../$1" &&
- # remove it temporarily before comparing, as
- # "$1/.git/config" lacks it...
- git config --unset core.worktree
- ) &&
+ # does core.worktree point at the right place?
+ echo "../../../$1" >expect &&
+ git -C ".git/modules/$1" config core.worktree >actual &&
+ test_cmp expect actual &&
+ # remove it temporarily before comparing, as
+ # "$1/.git/config" lacks it...
+ git -C ".git/modules/$1" config --unset core.worktree &&
diff -r ".git/modules/$1" "$1/.git" &&
- (
- # ... and then restore.
- cd ".git/modules/$1" &&
- git config core.worktree "../../../$1"
- )
+ # ... and then restore.
+ git -C ".git/modules/$1" config core.worktree "../../../$1"
}
test_git_directory_exists () {
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 68e29c904a6..dcc78fb6a7b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -255,7 +255,10 @@ test_expect_success 'prefix_path rejects absolute path to dir with same beginnin
test_expect_success SYMLINKS 'prefix_path works with absolute path to a symlink to work tree having same beginning as work tree' '
git init repo &&
ln -s repo repolink &&
- test "a" = "$(cd repo && test-tool path-utils prefix_path prefix "$(pwd)/../repolink/a")"
+ echo "a" >expect &&
+ repo_path="$(cd repo && pwd)" &&
+ test-tool -C repo path-utils prefix_path prefix "$repo_path/../repolink/a" >actual &&
+ test_cmp expect actual
'
relative_path /foo/a/b/c/ /foo/a/b/ c/
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5a169b68d6a..5a8a48287c1 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -245,9 +245,13 @@ test_expect_success 'git branch -M baz bam should succeed within a worktree in w
(
cd bazdir &&
git branch -M baz bam &&
- test $(git rev-parse --abbrev-ref HEAD) = bam
+ echo bam >expect &&
+ git rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual
) &&
- test $(git rev-parse --abbrev-ref HEAD) = bam &&
+ echo bam >expect &&
+ git rev-parse --abbrev-ref HEAD >actual &&
+ test_cmp expect actual &&
rm -r bazdir &&
git worktree prune
'
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 38b850c10ef..1d7b1abda1a 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -15,8 +15,12 @@ test_expect_success 'preparing origin repository' '
: >file && git add . && git commit -m1 &&
git clone --bare . a.git &&
git clone --bare . x &&
- test "$(cd a.git && git config --bool core.bare)" = true &&
- test "$(cd x && git config --bool core.bare)" = true &&
+ echo true >expect &&
+ git -C a.git config --bool core.bare >actual &&
+ test_cmp expect actual &&
+ echo true >expect &&
+ git -C x config --bool core.bare >actual &&
+ test_cmp expect actual &&
git bundle create b1.bundle --all &&
git bundle create b2.bundle main &&
mkdir dir &&
@@ -29,7 +33,9 @@ test_expect_success 'preparing origin repository' '
test_expect_success 'local clone without .git suffix' '
git clone -l -s a b &&
(cd b &&
- test "$(git config --bool core.bare)" = false &&
+ echo false >expect &&
+ git config --bool core.bare >actual &&
+ test_cmp expect actual &&
git fetch)
'
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index ebeca12a711..1927a862839 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -82,11 +82,19 @@ test_expect_success 'stash with a dirty submodule' '
CURRENT=$(cd submodule && git rev-parse HEAD) &&
git stash &&
test new != $(cat file) &&
- test submodule = $(git diff --name-only) &&
- test $CURRENT = $(cd submodule && git rev-parse HEAD) &&
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual &&
+
+ echo "$CURRENT" >expect &&
+ git -C submodule rev-parse HEAD >actual &&
+ test_cmp expect actual &&
+
git stash apply &&
test new = $(cat file) &&
- test $CURRENT = $(cd submodule && git rev-parse HEAD)
+ echo "$CURRENT" >expect &&
+ git -C submodule rev-parse HEAD >actual &&
+ test_cmp expect actual
'
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply related [flat|nested] 83+ messages in thread
* [PATCH v5 4/6] tests: don't lose exit status with "test <op> $(git ...)"
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2023-02-06 22:44 ` [PATCH v5 3/6] tests: don't lose exit status with "(cd ...; test <op> $(git ...))" Ævar Arnfjörð Bjarmason
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
6 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
As with the preceding commit, rewrite tests that ran "git" inside
command substitution and lost the exit status of "git" so that we
notice the failing "git". This time around we're converting cases that
didn't involve a containing sub-shell around the command substitution.
In the case of "t0060-path-utils.sh" and
"t2005-checkout-index-symlinks.sh" convert the relevant code to using
the modern style of indentation and newline wrapping while having to
change it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/lib-submodule-update.sh | 4 +-
t/t0001-init.sh | 9 ++-
t/t0002-gitfile.sh | 4 +-
t/t0060-path-utils.sh | 103 +++++++++++++++++++++--------
t/t0100-previous.sh | 8 ++-
t/t1504-ceiling-dirs.sh | 8 ++-
t/t2005-checkout-index-symlinks.sh | 8 ++-
t/t5522-pull-symlink.sh | 4 +-
t/t7402-submodule-rebase.sh | 9 ++-
t/t7504-commit-msg-hook.sh | 4 +-
t/t7810-grep.sh | 4 +-
11 files changed, 120 insertions(+), 45 deletions(-)
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index d7c2b670b4a..dee14992c52 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -185,7 +185,9 @@ test_git_directory_exists () {
if test -f sub1/.git
then
# does core.worktree point at the right place?
- test "$(git -C .git/modules/$1 config core.worktree)" = "../../../$1"
+ echo "../../../$1" >expect &&
+ git -C ".git/modules/$1" config core.worktree >actual &&
+ test_cmp expect actual
fi
}
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index d479303efa0..30a6edca1d2 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -598,9 +598,14 @@ test_expect_success 'invalid default branch name' '
test_expect_success 'branch -m with the initial branch' '
git init rename-initial &&
git -C rename-initial branch -m renamed &&
- test renamed = $(git -C rename-initial symbolic-ref --short HEAD) &&
+ echo renamed >expect &&
+ git -C rename-initial symbolic-ref --short HEAD >actual &&
+ test_cmp expect actual &&
+
git -C rename-initial branch -m renamed again &&
- test again = $(git -C rename-initial symbolic-ref --short HEAD)
+ echo again >expect &&
+ git -C rename-initial symbolic-ref --short HEAD >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 26eaca095a2..e013d38f485 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -33,7 +33,9 @@ test_expect_success 'bad setup: invalid .git file path' '
test_expect_success 'final setup + check rev-parse --git-dir' '
echo "gitdir: $REAL" >.git &&
- test "$REAL" = "$(git rev-parse --git-dir)"
+ echo "$REAL" >expect &&
+ git rev-parse --git-dir >actual &&
+ test_cmp expect actual
'
test_expect_success 'check hash-object' '
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index dcc78fb6a7b..0afa3d0d312 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -10,20 +10,27 @@ TEST_PASSES_SANITIZE_LEAK=true
norm_path() {
expected=$(test-tool path-utils print_path "$2")
- test_expect_success $3 "normalize path: $1 => $2" \
- "test \"\$(test-tool path-utils normalize_path_copy '$1')\" = '$expected'"
+ test_expect_success $3 "normalize path: $1 => $2" "
+ echo '$expected' >expect &&
+ test-tool path-utils normalize_path_copy '$1' >actual &&
+ test_cmp expect actual
+ "
}
relative_path() {
expected=$(test-tool path-utils print_path "$3")
- test_expect_success $4 "relative path: $1 $2 => $3" \
- "test \"\$(test-tool path-utils relative_path '$1' '$2')\" = '$expected'"
+ test_expect_success $4 "relative path: $1 $2 => $3" "
+ echo '$expected' >expect &&
+ test-tool path-utils relative_path '$1' '$2' >actual &&
+ test_cmp expect actual
+ "
}
test_submodule_relative_url() {
test_expect_success "test_submodule_relative_url: $1 $2 $3 => $4" "
- actual=\$(test-tool submodule resolve-relative-url '$1' '$2' '$3') &&
- test \"\$actual\" = '$4'
+ echo '$4' >expect &&
+ test-tool submodule resolve-relative-url '$1' '$2' '$3' >actual &&
+ test_cmp expect actual
"
}
@@ -64,9 +71,11 @@ ancestor() {
expected=$(($expected-$rootslash+$rootoff))
;;
esac
- test_expect_success $4 "longest ancestor: $1 $2 => $expected" \
- "actual=\$(test-tool path-utils longest_ancestor_length '$1' '$2') &&
- test \"\$actual\" = '$expected'"
+ test_expect_success $4 "longest ancestor: $1 $2 => $expected" "
+ echo '$expected' >expect &&
+ test-tool path-utils longest_ancestor_length '$1' '$2' >actual &&
+ test_cmp expect actual
+ "
}
# Some absolute path tests should be skipped on Windows due to path mangling
@@ -166,8 +175,10 @@ ancestor D:/Users/me C:/ -1 MINGW
ancestor //server/share/my-directory //server/share/ 14 MINGW
test_expect_success 'strip_path_suffix' '
- test c:/msysgit = $(test-tool path-utils strip_path_suffix \
- c:/msysgit/libexec//git-core libexec/git-core)
+ echo c:/msysgit >expect &&
+ test-tool path-utils strip_path_suffix \
+ c:/msysgit/libexec//git-core libexec/git-core >actual &&
+ test_cmp expect actual
'
test_expect_success 'absolute path rejects the empty string' '
@@ -188,35 +199,61 @@ test_expect_success 'real path rejects the empty string' '
'
test_expect_success POSIX 'real path works on absolute paths 1' '
+ echo / >expect &&
+ test-tool path-utils real_path "/" >actual &&
+ test_cmp expect actual &&
+
nopath="hopefully-absent-path" &&
- test "/" = "$(test-tool path-utils real_path "/")" &&
- test "/$nopath" = "$(test-tool path-utils real_path "/$nopath")"
+ echo "/$nopath" >expect &&
+ test-tool path-utils real_path "/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success 'real path works on absolute paths 2' '
- nopath="hopefully-absent-path" &&
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "$d")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "$d/$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "$d" >actual &&
+ test_cmp expect actual &&
+
+ nopath="hopefully-absent-path" &&
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "$d/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success POSIX 'real path removes extra leading slashes' '
+ echo "/" >expect &&
+ test-tool path-utils real_path "///" >actual &&
+ test_cmp expect actual &&
+
nopath="hopefully-absent-path" &&
- test "/" = "$(test-tool path-utils real_path "///")" &&
- test "/$nopath" = "$(test-tool path-utils real_path "///$nopath")" &&
+ echo "/$nopath" >expect &&
+ test-tool path-utils real_path "///$nopath" >actual &&
+ test_cmp expect actual &&
+
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "//$d")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "//$d/$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "//$d" >actual &&
+ test_cmp expect actual &&
+
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "//$d/$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success 'real path removes other extra slashes' '
- nopath="hopefully-absent-path" &&
# Find an existing top-level directory for the remaining tests:
d=$(pwd -P | sed -e "s|^\([^/]*/[^/]*\)/.*|\1|") &&
- test "$d" = "$(test-tool path-utils real_path "$d///")" &&
- test "$d/$nopath" = "$(test-tool path-utils real_path "$d///$nopath")"
+ echo "$d" >expect &&
+ test-tool path-utils real_path "$d///" >actual &&
+ test_cmp expect actual &&
+
+ nopath="hopefully-absent-path" &&
+ echo "$d/$nopath" >expect &&
+ test-tool path-utils real_path "$d///$nopath" >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'real path works on symlinks' '
@@ -227,19 +264,29 @@ test_expect_success SYMLINKS 'real path works on symlinks' '
mkdir third &&
dir="$(cd .git && pwd -P)" &&
dir2=third/../second/other/.git &&
- test "$dir" = "$(test-tool path-utils real_path $dir2)" &&
+ echo "$dir" >expect &&
+ test-tool path-utils real_path $dir2 >actual &&
+ test_cmp expect actual &&
file="$dir"/index &&
- test "$file" = "$(test-tool path-utils real_path $dir2/index)" &&
+ echo "$file" >expect &&
+ test-tool path-utils real_path $dir2/index >actual &&
+ test_cmp expect actual &&
basename=blub &&
- test "$dir/$basename" = "$(cd .git && test-tool path-utils real_path "$basename")" &&
+ echo "$dir/$basename" >expect &&
+ test-tool -C .git path-utils real_path "$basename" >actual &&
+ test_cmp expect actual &&
ln -s ../first/file .git/syml &&
sym="$(cd first && pwd -P)"/file &&
- test "$sym" = "$(test-tool path-utils real_path "$dir2/syml")"
+ echo "$sym" >expect &&
+ test-tool path-utils real_path "$dir2/syml" >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'prefix_path works with absolute paths to work tree symlinks' '
ln -s target symlink &&
- test "$(test-tool path-utils prefix_path prefix "$(pwd)/symlink")" = "symlink"
+ echo "symlink" >expect &&
+ test-tool path-utils prefix_path prefix "$(pwd)/symlink" >actual &&
+ test_cmp expect actual
'
test_expect_success 'prefix_path works with only absolute path to work tree' '
diff --git a/t/t0100-previous.sh b/t/t0100-previous.sh
index a16cc3d2983..70a3223f219 100755
--- a/t/t0100-previous.sh
+++ b/t/t0100-previous.sh
@@ -12,7 +12,9 @@ test_expect_success 'branch -d @{-1}' '
test_commit A &&
git checkout -b junk &&
git checkout - &&
- test "$(git symbolic-ref HEAD)" = refs/heads/main &&
+ echo refs/heads/main >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
git branch -d @{-1} &&
test_must_fail git rev-parse --verify refs/heads/junk
'
@@ -21,7 +23,9 @@ test_expect_success 'branch -d @{-12} when there is not enough switches yet' '
git reflog expire --expire=now &&
git checkout -b junk2 &&
git checkout - &&
- test "$(git symbolic-ref HEAD)" = refs/heads/main &&
+ echo refs/heads/main >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual &&
test_must_fail git branch -d @{-12} &&
git rev-parse --verify refs/heads/main
'
diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
index 0fafcf9dde3..c1679e31d8a 100755
--- a/t/t1504-ceiling-dirs.sh
+++ b/t/t1504-ceiling-dirs.sh
@@ -6,8 +6,12 @@ TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
test_prefix() {
- test_expect_success "$1" \
- "test '$2' = \"\$(git rev-parse --show-prefix)\""
+ local expect="$2" &&
+ test_expect_success "$1: git rev-parse --show-prefix is '$2'" '
+ echo "$expect" >expect &&
+ git rev-parse --show-prefix >actual &&
+ test_cmp expect actual
+ '
}
test_fail() {
diff --git a/t/t2005-checkout-index-symlinks.sh b/t/t2005-checkout-index-symlinks.sh
index 112682a45a1..67d18cfa104 100755
--- a/t/t2005-checkout-index-symlinks.sh
+++ b/t/t2005-checkout-index-symlinks.sh
@@ -22,8 +22,10 @@ test_expect_success \
git checkout-index symlink &&
test -f symlink'
-test_expect_success \
-'the file must be the blob we added during the setup' '
-test "$(git hash-object -t blob symlink)" = $l'
+test_expect_success 'the file must be the blob we added during the setup' '
+ echo "$l" >expect &&
+ git hash-object -t blob symlink >actual &&
+ test_cmp expect actual
+'
test_done
diff --git a/t/t5522-pull-symlink.sh b/t/t5522-pull-symlink.sh
index bcff460d0a2..9fb73a8c3eb 100755
--- a/t/t5522-pull-symlink.sh
+++ b/t/t5522-pull-symlink.sh
@@ -78,7 +78,9 @@ test_expect_success SYMLINKS 'pushing from symlinked subdir' '
git commit -m push ./file &&
git push
) &&
- test push = $(git show HEAD:subdir/file)
+ echo push >expect &&
+ git show HEAD:subdir/file >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh
index 1927a862839..c74798e8d24 100755
--- a/t/t7402-submodule-rebase.sh
+++ b/t/t7402-submodule-rebase.sh
@@ -55,12 +55,15 @@ chmod a+x fake-editor.sh
test_expect_success 'interactive rebase with a dirty submodule' '
- test submodule = $(git diff --name-only) &&
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual &&
HEAD=$(git rev-parse HEAD) &&
GIT_EDITOR="\"$(pwd)/fake-editor.sh\"" EDITOR_TEXT="pick $HEAD" \
git rebase -i HEAD^ &&
- test submodule = $(git diff --name-only)
-
+ echo submodule >expect &&
+ git diff --name-only >actual &&
+ test_cmp expect actual
'
test_expect_success 'rebase with dirty file and submodule fails' '
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index 07ca46fb0d5..d1255228d5f 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -102,7 +102,9 @@ test_expect_success 'setup: commit-msg hook that always fails' '
'
commit_msg_is () {
- test "$(git log --pretty=format:%s%b -1)" = "$1"
+ printf "%s" "$1" >expect &&
+ git log --pretty=format:%s%b -1 >actual &&
+ test_cmp expect actual
}
test_expect_success 'with failing hook' '
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 8eded6ab274..39d6d713ecb 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1001,7 +1001,9 @@ test_expect_success 'log --committer does not search in timestamp' '
test_expect_success 'grep with CE_VALID file' '
git update-index --assume-unchanged t/t &&
rm t/t &&
- test "$(git grep test)" = "t/t:test" &&
+ echo "t/t:test" >expect &&
+ git grep test >actual &&
+ test_cmp expect actual &&
git update-index --no-assume-unchanged t/t &&
git checkout t/t
'
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply related [flat|nested] 83+ messages in thread
* [PATCH v5 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )"
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2023-02-06 22:44 ` [PATCH v5 4/6] tests: don't lose exit status with "test <op> $(git ...)" Ævar Arnfjörð Bjarmason
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 22:44 ` [PATCH v5 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
2023-02-06 23:33 ` [PATCH v5 0/6] tests: fix ignored & hidden " Junio C Hamano
6 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
Change tests that would lose the "git" exit code via a negation
pattern to:
- In the case of "t0055-beyond-symlinks.sh" compare against the
expected output instead.
We could use the same pattern as in the "t3700-add.sh" below, doing
so would have the advantage that if we added an earlier test we
wouldn't need to adjust the "expect" output.
But as "t0055-beyond-symlinks.sh" is a small and focused test (less
than 40 lines in total) let's use "test_cmp" instead.
- For "t3700-add.sh" use "sed -n" to print the expected "bad" part,
and use "test_must_be_empty" to assert that it's not there. If we used
"grep" we'd get a non-zero exit code.
We could use "test_expect_code 1 grep", but this is more consistent
with existing patterns in the test suite.
We can also remove a repeated invocation of "git ls-files" for the
last test that's being modified in that file, and search the
existing "files" output instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t0055-beyond-symlinks.sh | 14 ++++++++++++--
t/t3700-add.sh | 18 +++++++++++++-----
2 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
index 6bada370225..c3eb1158ef9 100755
--- a/t/t0055-beyond-symlinks.sh
+++ b/t/t0055-beyond-symlinks.sh
@@ -15,12 +15,22 @@ test_expect_success SYMLINKS setup '
test_expect_success SYMLINKS 'update-index --add beyond symlinks' '
test_must_fail git update-index --add c/d &&
- ! ( git ls-files | grep c/d )
+ cat >expect <<-\EOF &&
+ a
+ b/d
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
'
test_expect_success SYMLINKS 'add beyond symlinks' '
test_must_fail git add c/d &&
- ! ( git ls-files | grep c/d )
+ cat >expect <<-\EOF &&
+ a
+ b/d
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
'
test_done
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 51afbd7b24a..82dd768944f 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -106,24 +106,32 @@ test_expect_success '.gitignore test setup' '
test_expect_success '.gitignore is honored' '
git add . &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones without -f' '
test_must_fail git add a.?? &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones without -f' '
test_must_fail git add d.?? &&
- ! (git ls-files | grep "\\.ig")
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual
'
test_expect_success 'error out when attempting to add ignored ones but add others' '
touch a.if &&
test_must_fail git add a.?? &&
- ! (git ls-files | grep "\\.ig") &&
- (git ls-files | grep a.if)
+ git ls-files >files &&
+ sed -n "/\\.ig/p" <files >actual &&
+ test_must_be_empty actual &&
+ grep a.if files
'
test_expect_success 'add ignored ones with -f' '
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply related [flat|nested] 83+ messages in thread
* [PATCH v5 6/6] tests: don't lose misc "git" exit codes
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2023-02-06 22:44 ` [PATCH v5 5/6] tests: don't lose "git" exit codes in "! ( git ... | grep )" Ævar Arnfjörð Bjarmason
@ 2023-02-06 22:44 ` Ævar Arnfjörð Bjarmason
2023-02-06 23:33 ` [PATCH v5 0/6] tests: fix ignored & hidden " Junio C Hamano
6 siblings, 0 replies; 83+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2023-02-06 22:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, René Scharfe, Eric Sunshine,
Torsten Bögershausen, Phillip Wood,
Ævar Arnfjörð Bjarmason
Fix a few miscellaneous cases where:
- We lost the "git" exit code via "git ... | grep"
- Likewise by having a $(git) argument to git itself
- Used "test -z" to check that a command emitted no output, we can use
"test_must_be_empty" and &&-chaining instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1401-symbolic-ref.sh | 3 ++-
t/t3701-add-interactive.sh | 8 +++++---
t/t7516-commit-races.sh | 3 ++-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index d708acdb819..5e36899d207 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -33,7 +33,8 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
reset_to_sane
test_expect_success 'symbolic-ref refuses bare sha1' '
- test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
+ rev=$(git rev-parse HEAD) &&
+ test_must_fail git symbolic-ref HEAD "$rev"
'
reset_to_sane
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 5841f280fb2..f1fe5d60677 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -296,9 +296,11 @@ test_expect_success FILEMODE 'stage mode and hunk' '
echo content >>file &&
chmod +x file &&
printf "y\\ny\\n" | git add -p &&
- git diff --cached file | grep "new mode" &&
- git diff --cached file | grep "+content" &&
- test -z "$(git diff file)"
+ git diff --cached file >out &&
+ grep "new mode" out &&
+ grep "+content" out &&
+ git diff file >out &&
+ test_must_be_empty out
'
# end of tests disabled when filemode is not usable
diff --git a/t/t7516-commit-races.sh b/t/t7516-commit-races.sh
index f2ce14e9071..2d38a16480e 100755
--- a/t/t7516-commit-races.sh
+++ b/t/t7516-commit-races.sh
@@ -10,7 +10,8 @@ test_expect_success 'race to create orphan commit' '
test_must_fail env EDITOR=./hare-editor git commit --allow-empty -m tortoise -e &&
git show -s --pretty=format:%s >subject &&
grep hare subject &&
- test -z "$(git show -s --pretty=format:%P)"
+ git show -s --pretty=format:%P >out &&
+ test_must_be_empty out
'
test_expect_success 'race to create non-orphan commit' '
--
2.39.1.1425.gac85d95d48c
^ permalink raw reply related [flat|nested] 83+ messages in thread
* Re: [PATCH v5 0/6] tests: fix ignored & hidden exit codes
2023-02-06 22:44 ` [PATCH v5 " Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2023-02-06 22:44 ` [PATCH v5 6/6] tests: don't lose misc "git" exit codes Ævar Arnfjörð Bjarmason
@ 2023-02-06 23:33 ` Junio C Hamano
6 siblings, 0 replies; 83+ messages in thread
From: Junio C Hamano @ 2023-02-06 23:33 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, René Scharfe, Eric Sunshine, Torsten Bögershausen,
Phillip Wood
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Ævar Arnfjörð Bjarmason (6):
> auto-crlf tests: don't lose exit code in loops and outside tests
> t/lib-patch-mode.sh: fix ignored exit codes
> tests: don't lose exit status with "(cd ...; test <op> $(git ...))"
> tests: don't lose exit status with "test <op> $(git ...)"
> tests: don't lose "git" exit codes in "! ( git ... | grep )"
> tests: don't lose misc "git" exit codes
The changes relative to the previous round look OK. Will queue.
Thanks.
^ permalink raw reply [flat|nested] 83+ messages in thread