* [PATCH 0/4] t: new helper test_line_count_cmd
@ 2021-06-12 4:27 Đoàn Trần Công Danh
2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Đoàn Trần Công Danh @ 2021-06-12 4:27 UTC (permalink / raw)
To: git
Cc: Đoàn Trần Công Danh, Eric Sunshine,
Ævar Arnfjörð Bjarmason, Junio C Hamano
This is a rewrite of a series I sent earlier [1].
Which was a response to a false positive when applying Junio's suggestion
to a series written by Ævar [2].
1: https://lore.kernel.org/git/20210421104102.3409-1-congdanhqx@gmail.com/
2: https://lore.kernel.org/git/87r1j42ffz.fsf@evledraar.gmail.com/
Đoàn Trần Công Danh (4):
test-lib-functions: introduce test_line_count_cmd
t6402: use find(1) builtin to filter instead of grep
t6400: use test_line_count_cmd to count # of lines in stdout
t6402: use test_line_count_cmd to count # of lines in stdout
t/t6400-merge-df.sh | 16 ++---
t/t6402-merge-rename.sh | 132 +++++++++++++++++++---------------------
t/test-lib-functions.sh | 64 +++++++++++++++++++
3 files changed, 134 insertions(+), 78 deletions(-)
--
2.32.0.278.gd42b80f139
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-12 4:27 [PATCH 0/4] t: new helper test_line_count_cmd Đoàn Trần Công Danh @ 2021-06-12 4:27 ` Đoàn Trần Công Danh 2021-06-13 3:10 ` Eric Sunshine ` (2 more replies) 2021-06-12 4:27 ` [PATCH 2/4] t6402: use find(1) builtin to filter instead of grep Đoàn Trần Công Danh ` (2 subsequent siblings) 3 siblings, 3 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-12 4:27 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Eric Sunshine, Ævar Arnfjörð Bjarmason, Junio C Hamano In Git project, we have multiple occasions that requires checking number of lines of text in stdout and/or stderr of a command. One of such example is t6400, which checks number of files in various states. Some of those commands are Git command, and we would like to check their exit status. In some of those checks, we pipe the stdout of those commands to "wc -l" to check for line count, thus loosing the exit status. Introduce a helper function to check for number of lines in stdout and stderr from those commands. This helper will create 2 temporary files in process, thus it may affect output of some checks. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- Notes: Theoretically, we could avoid those temporary files by this shenanigan: ! ( test $( ( test $( ( "$@" || echo "'$*' run into failure" >&3) | wc -l ) "$out_ops" "$out_val" || echo "stdout: !$outop $outval '$*'" >&3 ) 2>&1 | wc -l ) "$errop" "$errval" || echo "stderr: !$errop $errval '$*'" >&3 ) 3>&1 | grep . However, it looks too complicated. t/test-lib-functions.sh | 64 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index b823c14027..85bb31ea4c 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -817,6 +817,70 @@ test_line_count () { fi } +# test_line_count_cmd checks the number of lines of captured stdout and/or +# stderr of a command. +# +# NOTE: this helper function will create 2 temporary files named: +# * test_line_count_cmd_.out; and +# * test_line_count_cmd_.err +# +# And this helper function will remove those 2 files if the check is succeed. +# In case of failure, those files will be preserved. +test_line_count_cmd () { + local outop outval + local errop errval + + while test $# -ge 3 + do + case "$1" in + --out) + outop="$2" + outval="$3" + ;; + --err) + errop="$2" + errval="$3" + ;; + *) + break + ;; + esac + shift 3 + done && + if test $# = 0 || + { test "x$1" = "x!" && test $# = 1 ; } + then + BUG "test_line_count_cmd: no command to be run" + fi && + if test -z "$outop$errop" + then + BUG "test_line_count_cmd: check which stream?" + fi && + + if test "x$1" = "x!" + then + shift && + if "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err + then + echo "error: '$@' succeed!" + return 1 + fi + elif ! "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err + then + echo "error: '$@' run into failure!" + return 1 + fi && + if test -n "$outop" + then + test_line_count "$outop" "$outval" test_line_count_cmd_.out + fi && + if test -n "$errop" + then + test_line_count "$errop" "$errval" test_line_count_cmd_.err + fi && + rm -f test_line_count_cmd_.out test_line_count_cmd_.err +} + test_file_size () { test "$#" -ne 1 && BUG "1 param" test-tool path-utils file-size "$1" -- 2.32.0.278.gd42b80f139 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh @ 2021-06-13 3:10 ` Eric Sunshine 2021-06-13 7:36 ` Đoàn Trần Công Danh 2021-06-13 13:28 ` Ævar Arnfjörð Bjarmason 2021-06-13 13:36 ` Ævar Arnfjörð Bjarmason 2 siblings, 1 reply; 22+ messages in thread From: Eric Sunshine @ 2021-06-13 3:10 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: Git List, Ævar Arnfjörð Bjarmason, Junio C Hamano On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote: > In Git project, we have multiple occasions that requires checking number > of lines of text in stdout and/or stderr of a command. One of such > example is t6400, which checks number of files in various states. > > Some of those commands are Git command, and we would like to check their > exit status. In some of those checks, we pipe the stdout of those > commands to "wc -l" to check for line count, thus loosing the exit status. > > Introduce a helper function to check for number of lines in stdout and > stderr from those commands. > > This helper will create 2 temporary files in process, thus it may affect > output of some checks. If the presence of these files is a concern, I wonder if it would make sense to turn these into dot-files (leading dot in name) or shove them into the .git/ directory? (Not necessarily an actionable comment; just tossing around some thoughts.) > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > @@ -817,6 +817,70 @@ test_line_count () { > +# test_line_count_cmd checks the number of lines of captured stdout and/or > +# stderr of a command. > +# > +# NOTE: this helper function will create 2 temporary files named: > +# * test_line_count_cmd_.out; and > +# * test_line_count_cmd_.err > +# > +# And this helper function will remove those 2 files if the check is succeed. > +# In case of failure, those files will be preserved. > +test_line_count_cmd () { It would be good to have some usage information in the above comment so that people aren't forced to consult the implementation to learn what options this function takes. At minimum, it should mention `--out`, `--err`, and `!`, and should explain the arguments each option takes (even if just through examples). > + local outop outval > + local errop errval > + > + while test $# -ge 3 > + do > + case "$1" in > + --out) > + outop="$2" > + outval="$3" > + ;; > + --err) > + errop="$2" > + errval="$3" > + ;; > + *) > + break > + ;; > + esac > + shift 3 > + done && This is really minor, but if test_line_count_cmd() ever learns some new option and that option does not consume two arguments, then the `shift 3` at the end of the `case/esac` will need to be adjusted in some fashion. It might make more sense, therefore, to perform the `shift 3` closer to where it is needed (that is, in the `--out` case and in the `--err` case) rather than delaying it as is done here. (Not necessarily worth a re-roll.) Another minor comment: Since you're &&-chaining everything else in the function, it wouldn't hurt to also do so for the `local` declarations and the assignments within each `case` arm, and to chain `esac` with `shift 3`. Doing so could help some future programmer who might (for some reason) insert code above the `while` loop, thinking that a failure in the new code will abort the function, but not realizing that the &&-chain is not intact in this area of the code. > + if test $# = 0 || > + { test "x$1" = "x!" && test $# = 1 ; } > + then > + BUG "test_line_count_cmd: no command to be run" > + fi && > + if test -z "$outop$errop" > + then > + BUG "test_line_count_cmd: check which stream?" > + fi && > + > + if test "x$1" = "x!" > + then > + shift && > + if "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > + then > + echo "error: '$@' succeed!" > + return 1 > + fi > + elif ! "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > + then > + echo "error: '$@' run into failure!" > + return 1 > + fi && Do we care that the "!" negated case doesn't have the same semantics as test_must_fail()? If we do care, then should there be a way to tell the function whether we want test_must_fail() semantics or `!` semantics (i.e. whether we're running a Git command or a non-Git command) or should it infer it on its own? (These are genuine questions -- not necessarily requests for changes -- as I'm trying to reason through the implications of this implementation choice.) > + if test -n "$outop" > + then > + test_line_count "$outop" "$outval" test_line_count_cmd_.out > + fi && > + if test -n "$errop" > + then > + test_line_count "$errop" "$errval" test_line_count_cmd_.err > + fi && > + rm -f test_line_count_cmd_.out test_line_count_cmd_.err > +} ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 3:10 ` Eric Sunshine @ 2021-06-13 7:36 ` Đoàn Trần Công Danh 0 siblings, 0 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-13 7:36 UTC (permalink / raw) To: Eric Sunshine Cc: Git List, Ævar Arnfjörð Bjarmason, Junio C Hamano On 2021-06-12 23:10:19-0400, Eric Sunshine <sunshine@sunshineco.com> wrote: > On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh > <congdanhqx@gmail.com> wrote: > > In Git project, we have multiple occasions that requires checking number > > of lines of text in stdout and/or stderr of a command. One of such > > example is t6400, which checks number of files in various states. > > > > Some of those commands are Git command, and we would like to check their > > exit status. In some of those checks, we pipe the stdout of those > > commands to "wc -l" to check for line count, thus loosing the exit status. > > > > Introduce a helper function to check for number of lines in stdout and > > stderr from those commands. > > > > This helper will create 2 temporary files in process, thus it may affect > > output of some checks. > > If the presence of these files is a concern, I wonder if it would make > sense to turn these into dot-files (leading dot in name) or shove them > into the .git/ directory? (Not necessarily an actionable comment; just > tossing around some thoughts.) The presence of those files is concern to some command likes: * git ls-files -o; or * ls -a Shoving into ".git" would be actionable if we are testing inside a git repository. Should we testing outside, we don't have that luxury. I think we can accept that limitation (only allow this helper inside a Git worktree to avoid surpising behavior). > > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > > --- > > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > > @@ -817,6 +817,70 @@ test_line_count () { > > +# test_line_count_cmd checks the number of lines of captured stdout and/or > > +# stderr of a command. > > +# > > +# NOTE: this helper function will create 2 temporary files named: > > +# * test_line_count_cmd_.out; and > > +# * test_line_count_cmd_.err > > +# > > +# And this helper function will remove those 2 files if the check is succeed. > > +# In case of failure, those files will be preserved. > > +test_line_count_cmd () { > > It would be good to have some usage information in the above comment > so that people aren't forced to consult the implementation to learn > what options this function takes. At minimum, it should mention > `--out`, `--err`, and `!`, and should explain the arguments each > option takes (even if just through examples). Make sense! > > > + local outop outval > > + local errop errval > > + > > + while test $# -ge 3 > > + do > > + case "$1" in > > + --out) > > + outop="$2" > > + outval="$3" > > + ;; > > + --err) > > + errop="$2" > > + errval="$3" > > + ;; > > + *) > > + break > > + ;; > > + esac > > + shift 3 > > + done && > > This is really minor, but if test_line_count_cmd() ever learns some > new option and that option does not consume two arguments, then the > `shift 3` at the end of the `case/esac` will need to be adjusted in > some fashion. It might make more sense, therefore, to perform the > `shift 3` closer to where it is needed (that is, in the `--out` case > and in the `--err` case) rather than delaying it as is done here. (Not > necessarily worth a re-roll.) Make sense, too. I think I'll add another leg here for "-*" to avoid potential break when adding/removing options. Not that I imagine about new options, but I can't think of any command starts with "-" > Another minor comment: Since you're &&-chaining everything else in the > function, it wouldn't hurt to also do so for the `local` declarations > and the assignments within each `case` arm, and to chain `esac` with > `shift 3`. Doing so could help some future programmer who might (for > some reason) insert code above the `while` loop, thinking that a > failure in the new code will abort the function, but not realizing > that the &&-chain is not intact in this area of the code. Will do, too! > > + if test $# = 0 || > > + { test "x$1" = "x!" && test $# = 1 ; } > > + then > > + BUG "test_line_count_cmd: no command to be run" > > + fi && > > + if test -z "$outop$errop" > > + then > > + BUG "test_line_count_cmd: check which stream?" > > + fi && > > + > > + if test "x$1" = "x!" > > + then > > + shift && > > + if "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > > + then > > + echo "error: '$@' succeed!" > > + return 1 > > + fi > > + elif ! "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > > + then > > + echo "error: '$@' run into failure!" > > + return 1 > > + fi && > > Do we care that the "!" negated case doesn't have the same semantics > as test_must_fail()? If we do care, then should there be a way to tell > the function whether we want test_must_fail() semantics or `!` > semantics (i.e. whether we're running a Git command or a non-Git > command) or should it infer it on its own? (These are genuine > questions -- not necessarily requests for changes -- as I'm trying to > reason through the implications of this implementation choice.) I think that we shouldn't care, in my mind, I would let users to chain test_line_count and test_must_fail if they do care about that sematic. So, we have the freedom to use test_line_count_cmd with both Git and non-Git commands. E.g: ---------8<--------------- diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index ad4746d899..94c8cfc9f2 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -288,9 +288,8 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' ' ' test_expect_success 'OPT_CALLBACK() and callback errors work' ' - test_must_fail test-tool parse-options --no-length >output 2>output.err && - test_must_be_empty output && - test_must_be_empty output.err + test_line_count_cmd --out = 0 --err = 0 \ + test_must_fail test-tool parse-options --no-length ' cat >expect <<\EOF ------------>8-------------- > > > + if test -n "$outop" > > + then > > + test_line_count "$outop" "$outval" test_line_count_cmd_.out > > + fi && > > + if test -n "$errop" > > + then > > + test_line_count "$errop" "$errval" test_line_count_cmd_.err > > + fi && > > + rm -f test_line_count_cmd_.out test_line_count_cmd_.err > > +} -- Danh ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh 2021-06-13 3:10 ` Eric Sunshine @ 2021-06-13 13:28 ` Ævar Arnfjörð Bjarmason 2021-06-13 16:37 ` Đoàn Trần Công Danh 2021-06-13 18:18 ` Phillip Wood 2021-06-13 13:36 ` Ævar Arnfjörð Bjarmason 2 siblings, 2 replies; 22+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-06-13 13:28 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Eric Sunshine, Junio C Hamano On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > + { test "x$1" = "x!" && test $# = 1 ; } > [...] > + if test "x$1" = "x!" We don't use this test idiom in other places, it's OK to just use "$1" = "!". I think we're past whatever portability concern made that popular in some older shell code in the wild. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 13:28 ` Ævar Arnfjörð Bjarmason @ 2021-06-13 16:37 ` Đoàn Trần Công Danh 2021-06-13 18:18 ` Phillip Wood 1 sibling, 0 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-13 16:37 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Eric Sunshine, Junio C Hamano On 2021-06-13 15:28:58+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > > > + { test "x$1" = "x!" && test $# = 1 ; } > > [...] > > + if test "x$1" = "x!" > > We don't use this test idiom in other places, it's OK to just use "$1" = > "!". I think we're past whatever portability concern made that popular > in some older shell code in the wild. We actually use this test idiom in test_i18ngrep and test_cmp_rev. I don't know enough to see where should I cut the base line. -- Danh ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 13:28 ` Ævar Arnfjörð Bjarmason 2021-06-13 16:37 ` Đoàn Trần Công Danh @ 2021-06-13 18:18 ` Phillip Wood 2021-06-13 21:42 ` Felipe Contreras 2021-06-13 23:43 ` Eric Sunshine 1 sibling, 2 replies; 22+ messages in thread From: Phillip Wood @ 2021-06-13 18:18 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Đoàn Trần Công Danh Cc: git, Eric Sunshine, Junio C Hamano On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > >> + { test "x$1" = "x!" && test $# = 1 ; } >> [...] >> + if test "x$1" = "x!" > > We don't use this test idiom in other places, it's OK to just use "$1" = > "!". I think we're past whatever portability concern made that popular > in some older shell code in the wild. Slightly off topic but if anyone is interested in the history of this test idiom and why it is no longer needed there is a good article at https://www.vidarholen.net/contents/blog/?p=1035 Best Wishes Phillip ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 18:18 ` Phillip Wood @ 2021-06-13 21:42 ` Felipe Contreras 2021-06-13 23:43 ` Eric Sunshine 1 sibling, 0 replies; 22+ messages in thread From: Felipe Contreras @ 2021-06-13 21:42 UTC (permalink / raw) To: Phillip Wood, Ævar Arnfjörð Bjarmason, Đoàn Trần Công Danh Cc: git, Eric Sunshine, Junio C Hamano Phillip Wood wrote: > On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote: > > > > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > > > >> + { test "x$1" = "x!" && test $# = 1 ; } > >> [...] > >> + if test "x$1" = "x!" > > > > We don't use this test idiom in other places, it's OK to just use "$1" = > > "!". I think we're past whatever portability concern made that popular > > in some older shell code in the wild. > > Slightly off topic but if anyone is interested in the history of this > test idiom and why it is no longer needed there is a good article at > https://www.vidarholen.net/contents/blog/?p=1035 Very nice! I often wondered why people did that. -- Felipe Contreras ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 18:18 ` Phillip Wood 2021-06-13 21:42 ` Felipe Contreras @ 2021-06-13 23:43 ` Eric Sunshine 2021-06-14 2:56 ` Junio C Hamano 2021-06-24 23:19 ` Ævar Arnfjörð Bjarmason 1 sibling, 2 replies; 22+ messages in thread From: Eric Sunshine @ 2021-06-13 23:43 UTC (permalink / raw) To: Phillip Wood Cc: Ævar Arnfjörð Bjarmason, Đoàn Trần Công Danh, Git List, Junio C Hamano On Sun, Jun 13, 2021 at 2:18 PM Phillip Wood <phillip.wood123@gmail.com> wrote: > On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > >> + { test "x$1" = "x!" && test $# = 1 ; } > >> [...] > >> + if test "x$1" = "x!" > > > > We don't use this test idiom in other places, it's OK to just use "$1" = > > "!". I think we're past whatever portability concern made that popular > > in some older shell code in the wild. > > Slightly off topic but if anyone is interested in the history of this > test idiom and why it is no longer needed there is a good article at > https://www.vidarholen.net/contents/blog/?p=1035 Thanks for the link to the article; it was an interesting read. However, the article does seem to say that such idioms and care may still be warranted. In particular, the epilog gives an example which is still relevant on macOS today. (Indeed, I just tried it and it does error out as the article states.) Even discounting macOS, it also talks about such bugs existing as late as 2015, which isn't long ago by any stretch. (And, as someone whose primary -- indeed only -- development machine is ten years old, some of the other bugs it mentions -- which existed as recently as ten years ago -- don't seem all that long ago either.) At any rate, for those of us who are old-timers, the `"x$foo"` idiom is habit and only costs a couple extra characters, so I for one have no problem with its presence in the proposed patch. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 23:43 ` Eric Sunshine @ 2021-06-14 2:56 ` Junio C Hamano 2021-06-24 23:19 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2021-06-14 2:56 UTC (permalink / raw) To: Eric Sunshine Cc: Phillip Wood, Ævar Arnfjörð Bjarmason, Đoàn Trần Công Danh, Git List Eric Sunshine <sunshine@sunshineco.com> writes: > ... However, the article does seem to say that such idioms and care may > still be warranted. In particular, the epilog gives an example which > is still relevant on macOS today. ... > > At any rate, for those of us who are old-timers, the `"x$foo"` idiom > is habit and only costs a couple extra characters, so I for one have > no problem with its presence in the proposed patch. Yes. I think it is prudent to use the x-prefix idiom, especially for the case cited upthread like comparing $1 with an exclamation mark '!'. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 23:43 ` Eric Sunshine 2021-06-14 2:56 ` Junio C Hamano @ 2021-06-24 23:19 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 22+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-06-24 23:19 UTC (permalink / raw) To: Eric Sunshine Cc: Phillip Wood, Đoàn Trần Công Danh, Git List, Junio C Hamano On Sun, Jun 13 2021, Eric Sunshine wrote: > On Sun, Jun 13, 2021 at 2:18 PM Phillip Wood <phillip.wood123@gmail.com> wrote: >> On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote: >> > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: >> >> + { test "x$1" = "x!" && test $# = 1 ; } >> >> [...] >> >> + if test "x$1" = "x!" >> > >> > We don't use this test idiom in other places, it's OK to just use "$1" = >> > "!". I think we're past whatever portability concern made that popular >> > in some older shell code in the wild. >> >> Slightly off topic but if anyone is interested in the history of this >> test idiom and why it is no longer needed there is a good article at >> https://www.vidarholen.net/contents/blog/?p=1035 > > Thanks for the link to the article; it was an interesting read. > However, the article does seem to say that such idioms and care may > still be warranted. In particular, the epilog gives an example which > is still relevant on macOS today. (Indeed, I just tried it and it does > error out as the article states.) Even discounting macOS, it also > talks about such bugs existing as late as 2015, which isn't long ago > by any stretch. (And, as someone whose primary -- indeed only -- > development machine is ten years old, some of the other bugs it > mentions -- which existed as recently as ten years ago -- don't seem > all that long ago either.) It's only for the case where the first byte is "(" or ")" though, so e.g. the use of this to compare things like command-line options and other things that don't start with those characters is portable, if I'm understanding the article correctly. > At any rate, for those of us who are old-timers, the `"x$foo"` idiom > is habit and only costs a couple extra characters, so I for one have > no problem with its presence in the proposed patch. Indeed, but it's interesting to dig and see if there's any reason for such workarounds still. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh 2021-06-13 3:10 ` Eric Sunshine 2021-06-13 13:28 ` Ævar Arnfjörð Bjarmason @ 2021-06-13 13:36 ` Ævar Arnfjörð Bjarmason 2021-06-14 3:01 ` Junio C Hamano 2021-06-15 15:40 ` Đoàn Trần Công Danh 2 siblings, 2 replies; 22+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-06-13 13:36 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Eric Sunshine, Junio C Hamano On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > In Git project, we have multiple occasions that requires checking number > of lines of text in stdout and/or stderr of a command. One of such > example is t6400, which checks number of files in various states. Thanks for following up on this. > Some of those commands are Git command, and we would like to check their > exit status. In some of those checks, we pipe the stdout of those > commands to "wc -l" to check for line count, thus loosing the exit status. > > Introduce a helper function to check for number of lines in stdout and > stderr from those commands. > > This helper will create 2 temporary files in process, thus it may affect > output of some checks. I think it's fine to just blindly stick the name into a file in the CWD as long as it's not "actual" or some other such obviously likely to conflict name. The convention you've picked here (I'm not sure if it's existing already) of naming the temp files after the test lib function name is a good one. More generally speaking we have a bunch of helpers that have this potential issue/bug, in practice it's not a big deal. A test that's being overly specific and doing a test_cmp on unbounded "find" output or whatever is likely buggy anyway. If it ever becomes a bigger issue we can easily set up two scratch directories during the test, one for the use of the test itself, and one for the internals of the test run itself. > +# test_line_count_cmd checks the number of lines of captured stdout and/or > +# stderr of a command. > +# > +# NOTE: this helper function will create 2 temporary files named: > +# * test_line_count_cmd_.out; and > +# * test_line_count_cmd_.err > +# > +# And this helper function will remove those 2 files if the check is succeed. > +# In case of failure, those files will be preserved. > +test_line_count_cmd () { > + local outop outval > + local errop errval > + > + while test $# -ge 3 > + do > + case "$1" in > + --out) > + outop="$2" > + outval="$3" > + ;; > + --err) > + errop="$2" > + errval="$3" > + ;; It looks like the end-state of the series leaves us with no user of the --err option; Maybe it's good to have it anyway for completeness, or just skip it? ... > + *) > + break > + ;; > + esac > + shift 3 > + done && > + if test $# = 0 || > + { test "x$1" = "x!" && test $# = 1 ; } > + then > + BUG "test_line_count_cmd: no command to be run" > + fi && > + if test -z "$outop$errop" > + then > + BUG "test_line_count_cmd: check which stream?" > + fi && > + > + if test "x$1" = "x!" > + then > + shift && > + if "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > + then > + echo "error: '$@' succeed!" > + return 1 > + fi > + elif ! "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > + then > + echo "error: '$@' run into failure!" > + return 1 > + fi && ...I think it's better to not pipe to *.err if we haven't requested it, so under "-v" etc. we can get the stderr. If we're unifying them I think a better pattern is to only run that "$@" once, get $?, and then act differently on that in the "!" and "" cases. It requires less careful reading of the critical function path, especially with the differing indentation. > + if test -n "$outop" > + then > + test_line_count "$outop" "$outval" test_line_count_cmd_.out > + fi && > + if test -n "$errop" > + then > + test_line_count "$errop" "$errval" test_line_count_cmd_.err > + fi && > + rm -f test_line_count_cmd_.out test_line_count_cmd_.err Let's do that "rm -f" as a "test_when_finished" before we first pipe to them. It's fine to do that in a test lib function, see e.g. test_config. We'll get the benefit of preseving these files under -di etc. > +} > + > test_file_size () { > test "$#" -ne 1 && BUG "1 param" > test-tool path-utils file-size "$1" ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 13:36 ` Ævar Arnfjörð Bjarmason @ 2021-06-14 3:01 ` Junio C Hamano 2021-06-15 15:40 ` Đoàn Trần Công Danh 1 sibling, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2021-06-14 3:01 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Đoàn Trần Công Danh, git, Eric Sunshine Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > More generally speaking we have a bunch of helpers that have this > potential issue/bug, in practice it's not a big deal. A test that's > being overly specific and doing a test_cmp on unbounded "find" output or > whatever is likely buggy anyway. Unless we are testing ".gitignore", "ls-files -o", or "git status". Carving out $TRASH_DIRECTORY/.git/trash/ directory to store these throwaway files would be less error prone from that point of view. >> + case "$1" in >> + --out) >> + outop="$2" >> + outval="$3" >> + ;; >> + --err) >> + errop="$2" >> + errval="$3" >> + ;; > > It looks like the end-state of the series leaves us with no user of the > --err option; Maybe it's good to have it anyway for completeness, or > just skip it? ... I'd rather not to see --out added if we have not yet any use for the other one. When the time comes that we want to validate the error stream, we may find that we want to check _both_, and we'd have to discard dead code and replace with what we want, instead of just adding what we want to a working code without dead code. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd 2021-06-13 13:36 ` Ævar Arnfjörð Bjarmason 2021-06-14 3:01 ` Junio C Hamano @ 2021-06-15 15:40 ` Đoàn Trần Công Danh 1 sibling, 0 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-15 15:40 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Eric Sunshine, Junio C Hamano On 2021-06-13 15:36:11+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote: > > > In Git project, we have multiple occasions that requires checking number > > of lines of text in stdout and/or stderr of a command. One of such > > example is t6400, which checks number of files in various states. > > Thanks for following up on this. > > > Some of those commands are Git command, and we would like to check their > > exit status. In some of those checks, we pipe the stdout of those > > commands to "wc -l" to check for line count, thus loosing the exit status. > > > > Introduce a helper function to check for number of lines in stdout and > > stderr from those commands. > > > > This helper will create 2 temporary files in process, thus it may affect > > output of some checks. > > I think it's fine to just blindly stick the name into a file in the CWD > as long as it's not "actual" or some other such obviously likely to > conflict name. > > The convention you've picked here (I'm not sure if it's existing > already) of naming the temp files after the test lib function name is a > good one. > > More generally speaking we have a bunch of helpers that have this > potential issue/bug, in practice it's not a big deal. A test that's > being overly specific and doing a test_cmp on unbounded "find" output or > whatever is likely buggy anyway. I'm going to write into $TRASH_DIRECTORY/.git/trash if .git is-a-dir, otherwise to $TRASH_DIRECTORY in the next reroll. > If it ever becomes a bigger issue we can easily set up two scratch > directories during the test, one for the use of the test itself, and one > for the internals of the test run itself. > > > +# test_line_count_cmd checks the number of lines of captured stdout and/or > > +# stderr of a command. > > +# > > +# NOTE: this helper function will create 2 temporary files named: > > +# * test_line_count_cmd_.out; and > > +# * test_line_count_cmd_.err > > +# > > +# And this helper function will remove those 2 files if the check is succeed. > > +# In case of failure, those files will be preserved. > > +test_line_count_cmd () { > > + local outop outval > > + local errop errval > > + > > + while test $# -ge 3 > > + do > > + case "$1" in > > + --out) > > + outop="$2" > > + outval="$3" > > + ;; > > + --err) > > + errop="$2" > > + errval="$3" > > + ;; > > It looks like the end-state of the series leaves us with no user of the > --err option; Maybe it's good to have it anyway for completeness, or > just skip it? ... In the re-roll that being prepared, t0041 will be converted, too. So --err will have a user. > > + *) > > + break > > + ;; > > + esac > > + shift 3 > > + done && > > + if test $# = 0 || > > + { test "x$1" = "x!" && test $# = 1 ; } > > + then > > + BUG "test_line_count_cmd: no command to be run" > > + fi && > > + if test -z "$outop$errop" > > + then > > + BUG "test_line_count_cmd: check which stream?" > > + fi && > > + > > + if test "x$1" = "x!" > > + then > > + shift && > > + if "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > > + then > > + echo "error: '$@' succeed!" > > + return 1 > > + fi > > + elif ! "$@" >test_line_count_cmd_.out 2>test_line_count_cmd_.err > > + then > > + echo "error: '$@' run into failure!" > > + return 1 > > + fi && > > ...I think it's better to not pipe to *.err if we haven't requested it, > so under "-v" etc. we can get the stderr. And with t0041 converted, it reveals some difficulty when implementing this suggestion. Yes, it would be nice if we can get the stderr under -v, however, -x will make our life hard, since -x also writes into /dev/stderr, and below command is broken: test_line_count_cmd --out = 0 test_must_fail git for-each-ref --no-contains something 2>actual.err && test_i18ngrep ! usage actual.err > If we're unifying them I think a better pattern is to only run that "$@" > once, get $?, and then act differently on that in the "!" and "" > cases. It requires less careful reading of the critical function path, > especially with the differing indentation. This is definitely doable. This is a replacement patch that still has trouble with "-x", I guess I should always exec 2>*.err ---------8<-------- Subject: [PATCH] test-lib-functions: introduce test_line_count_cmd In Git project, we have multiple occasions that requires checking number of lines of text in stdout and/or stderr of a command. One of such example is t6400, which checks number of files in various states. Some of those commands are Git command, and we would like to check their exit status. In some of those checks, we pipe the stdout of those commands to "wc -l" to check for line count, thus loosing the exit status. Introduce a helper function to check for number of lines in stdout and stderr from those commands. This helper will create 2 temporary files in process, thus it may affect output of some checks. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/test-lib-functions.sh | 116 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index f0448daa74..2261de7caa 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -845,6 +845,122 @@ test_line_count () { fi } +# test_line_count_cmd checks exit status, and the number of lines in +# captured stdout and/or stderr of a command. +# +# Usage: +# +# test_line_count_cmd [--[out|err] <binop> <value>]... [--] [!] cmd [args...] +# +# Options: +# --out <binop> <value>: +# --err <binop> <value>: +# Run sh's "test <# of lines> <binop> <value>" on # of lines in stdout +# (for --out) or stderr (for --err) +# !: +# Instead of expecting "cmd [args...]" succeed, expect its failure. +# Note, if command under testing is "git", test_must_fail should be used +# instead of "!". +# +# Example: +# test_line_count_cmd --out -ge 10 --err = 0 git tag --no-contains v1.0.0 +# test_line_count_cmd --out -le 10 ! grep some-text a-file +# test_line_count_cmd --out = 0 test_must_fail git rev-parse --verify abcd1234 +# +# NOTE: +# * if "--out" is specified, a temporary file named test_line_count_cmd_.out +# will be created. +# * if "--err" is specified, a temporary file named test_line_count_cmd_.err +# will be created. +# Those temporary files will be created under $TRASH_DIRECTORY/.git/trash +# if $TRASH_DIRECTORY/.git directory existed. +# Otherwise, they will be created in $TRASH_DIRECTORY. +# Those temporary files will be cleant by test_when_finished +test_line_count_cmd () { + local outop outval outfile + local errop errval errfile + local expect_failure actual_failure + local trashdir="$TRASH_DIRECTORY" + + if test -d "$TRASH_DIRECTORY/.git" + then + trashdir="$TRASH_DIRECTORY/.git/trash" && + mkdir -p "$trashdir" + fi && + while test $# != 0 + do + case "$1" in + --out) + outop="$2" && + outval="$3" && + outfile="$trashdir/test_line_count_cmd_.out" && + shift 3 + ;; + --err) + errop="$2" && + errval="$3" && + errfile="$trashdir/test_line_count_cmd_.err" && + shift 3 + ;; + --) + shift && + break + ;; + -*) + BUG "test_line_count_cmd: unknown options: '$1'" + ;; + *) + break + ;; + esac + done && + if test "x$1" = "x!" + then + shift && + expect_failure=yes + fi && + if test $# = 0 + then + BUG "test_line_count_cmd: no command to be run" + elif test -z "$outop$errop" + then + BUG "test_line_count_cmd: check which stream?" + else + if test -n "$outfile" + then + test_when_finished "rm -f '$outfile'" && + exec 3>"$outfile" + fi && + if test -n "$errfile" + then + test_when_finished "rm -f '$errfile'" && + exec 5>"$errfile" + fi && + if ! "$@" >&3 2>&5 + then + actual_failure=yes + fi + # Don't use &4, it's used for test_must_fail + fi 3>&1 5>&2 && + case "$expect_failure,$actual_failure" in + yes,) + echo "error: '$@' succeed!" + return 1 + ;; + ,yes) + echo "error: '$@' run into failure!" + return 1 + esac && + if test -n "$outop" + then + test_line_count "$outop" "$outval" "$outfile" + fi && + if test -n "$errop" + then + test_line_count "$errop" "$errval" "$errfile" + fi +} + test_file_size () { test "$#" -ne 1 && BUG "1 param" test-tool path-utils file-size "$1" -- -- Danh ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] t6402: use find(1) builtin to filter instead of grep 2021-06-12 4:27 [PATCH 0/4] t: new helper test_line_count_cmd Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh @ 2021-06-12 4:27 ` Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 4/4] t6402: " Đoàn Trần Công Danh 3 siblings, 0 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-12 4:27 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Eric Sunshine, Ævar Arnfjörð Bjarmason, Junio C Hamano find(1) has a builtin (-prune) to filter its output, save a bit of time for invoking grep(1). In addition, in a later change, we will try to use test_line_count_cmd to count number of lines in stdout and/or stderr of a command, due to limitation of current implementation, it can handle pipe. Let's replace grep(1)'s usage with find(1) builtin filter. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/t6402-merge-rename.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index 425dad97d5..5d76cd6414 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -546,7 +546,7 @@ then test_must_fail git diff --quiet && - test 3 -eq $(find . | grep -v .git | wc -l) && + test 3 -eq $(find . -name .git -prune -o -print | wc -l) && test_path_is_file one && test_path_is_file two && @@ -565,7 +565,7 @@ else test_must_fail git diff --quiet && - test 4 -eq $(find . | grep -v .git | wc -l) && + test 4 -eq $(find . -name .git -prune -o -print | wc -l) && test_path_is_dir one && test_path_is_file one~rename-two && @@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta test_must_fail git diff --quiet && - test 3 -eq $(find . | grep -v .git | wc -l) && + test 3 -eq $(find . -name .git -prune -o -print | wc -l) && test_path_is_file one && test_path_is_file two && -- 2.32.0.278.gd42b80f139 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout 2021-06-12 4:27 [PATCH 0/4] t: new helper test_line_count_cmd Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 2/4] t6402: use find(1) builtin to filter instead of grep Đoàn Trần Công Danh @ 2021-06-12 4:27 ` Đoàn Trần Công Danh 2021-06-12 4:33 ` Bagas Sanjaya 2021-06-13 3:39 ` Eric Sunshine 2021-06-12 4:27 ` [PATCH 4/4] t6402: " Đoàn Trần Công Danh 3 siblings, 2 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-12 4:27 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Eric Sunshine, Ævar Arnfjörð Bjarmason, Junio C Hamano Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/t6400-merge-df.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh index 38700d29b5..9d7b9354c5 100755 --- a/t/t6400-merge-df.sh +++ b/t/t6400-merge-df.sh @@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' ' git checkout delete^0 && test_must_fail git merge modify && - test 5 -eq $(git ls-files -s | wc -l) && - test 4 -eq $(git ls-files -u | wc -l) && + test_line_count_cmd --out = 5 git ls-files -s && + test_line_count_cmd --out = 4 git ls-files -u && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 0 -eq $(git ls-files -o | wc -l) + test_line_count_cmd --out = 2 git ls-files -o else - test 1 -eq $(git ls-files -o | wc -l) + test_line_count_cmd --out = 3 git ls-files -o fi && test_path_is_file letters/file && @@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' test_must_fail git merge delete && - test 5 -eq $(git ls-files -s | wc -l) && - test 4 -eq $(git ls-files -u | wc -l) && + test_line_count_cmd --out = 5 git ls-files -s && + test_line_count_cmd --out = 4 git ls-files -u && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 0 -eq $(git ls-files -o | wc -l) + test_line_count_cmd --out = 2 git ls-files -o else - test 1 -eq $(git ls-files -o | wc -l) + test_line_count_cmd --out = 3 git ls-files -o fi && test_path_is_file letters/file && -- 2.32.0.278.gd42b80f139 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout 2021-06-12 4:27 ` [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh @ 2021-06-12 4:33 ` Bagas Sanjaya 2021-06-13 7:39 ` Đoàn Trần Công Danh 2021-06-13 3:39 ` Eric Sunshine 1 sibling, 1 reply; 22+ messages in thread From: Bagas Sanjaya @ 2021-06-12 4:33 UTC (permalink / raw) To: Đoàn Trần Công Danh, git Cc: Eric Sunshine, Ævar Arnfjörð Bjarmason, Junio C Hamano Hi Đoàn, > - test 5 -eq $(git ls-files -s | wc -l) && > - test 4 -eq $(git ls-files -u | wc -l) && > + test_line_count_cmd --out = 5 git ls-files -s && > + test_line_count_cmd --out = 4 git ls-files -u && I read lines above as "Formerly I tested that 5/4 should be equal to output of git ls-files -s/-u piped to wc -l, now I do the same with test_line_count_cmd". Am I right? -- An old man doll... just what I always wanted! - Clara ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout 2021-06-12 4:33 ` Bagas Sanjaya @ 2021-06-13 7:39 ` Đoàn Trần Công Danh 0 siblings, 0 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-13 7:39 UTC (permalink / raw) To: Bagas Sanjaya Cc: git, Eric Sunshine, Ævar Arnfjörð Bjarmason, Junio C Hamano On 2021-06-12 11:33:49+0700, Bagas Sanjaya <bagasdotme@gmail.com> wrote: > Hi Đoàn, It's better to call me Danh. Thanks. Don't worry, I was called Dan all the time, too ;) > > > - test 5 -eq $(git ls-files -s | wc -l) && > > - test 4 -eq $(git ls-files -u | wc -l) && > > + test_line_count_cmd --out = 5 git ls-files -s && > > + test_line_count_cmd --out = 4 git ls-files -u && > > I read lines above as "Formerly I tested that 5/4 should be equal to output > of git ls-files -s/-u piped to wc -l, now I do the same with > test_line_count_cmd". > > Am I right? Yes, you read that right. This series is mostly a cleanup for linting "test .* -o" (But that regex wouldn't be put to be used anytime soon). -- Danh ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout 2021-06-12 4:27 ` [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh 2021-06-12 4:33 ` Bagas Sanjaya @ 2021-06-13 3:39 ` Eric Sunshine 2021-06-13 7:42 ` Đoàn Trần Công Danh 1 sibling, 1 reply; 22+ messages in thread From: Eric Sunshine @ 2021-06-13 3:39 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: Git List, Ævar Arnfjörð Bjarmason, Junio C Hamano On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote: > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh > @@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' ' > - test 5 -eq $(git ls-files -s | wc -l) && > - test 4 -eq $(git ls-files -u | wc -l) && > + test_line_count_cmd --out = 5 git ls-files -s && > + test_line_count_cmd --out = 4 git ls-files -u && Nit: too many spaces before the `&&` on the second line: s/\s+/ / > @@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' > - test 5 -eq $(git ls-files -s | wc -l) && > - test 4 -eq $(git ls-files -u | wc -l) && > + test_line_count_cmd --out = 5 git ls-files -s && > + test_line_count_cmd --out = 4 git ls-files -u && Nit: too many spaces before the `&&` on both lines. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout 2021-06-13 3:39 ` Eric Sunshine @ 2021-06-13 7:42 ` Đoàn Trần Công Danh 0 siblings, 0 replies; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-13 7:42 UTC (permalink / raw) To: Eric Sunshine Cc: Git List, Ævar Arnfjörð Bjarmason, Junio C Hamano On 2021-06-12 23:39:51-0400, Eric Sunshine <sunshine@sunshineco.com> wrote: > On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh > <congdanhqx@gmail.com> wrote: > > > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > > --- > > diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh > > @@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' ' > > - test 5 -eq $(git ls-files -s | wc -l) && > > - test 4 -eq $(git ls-files -u | wc -l) && > > + test_line_count_cmd --out = 5 git ls-files -s && > > + test_line_count_cmd --out = 4 git ls-files -u && > > Nit: too many spaces before the `&&` on the second line: s/\s+/ / Thanks, I'll update them, later. In my defence, this patch was generated mechanically with: :%s/test \([0-9]\+\) -eq "*[$][(]\(.*\)|.*[)]"*/test_line_count_cmd --out = \1 \2/gc I forgot to include the space after final ")" > > @@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' ' > > - test 5 -eq $(git ls-files -s | wc -l) && > > - test 4 -eq $(git ls-files -u | wc -l) && > > + test_line_count_cmd --out = 5 git ls-files -s && > > + test_line_count_cmd --out = 4 git ls-files -u && > > Nit: too many spaces before the `&&` on both lines. -- Danh ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] t6402: use test_line_count_cmd to count # of lines in stdout 2021-06-12 4:27 [PATCH 0/4] t: new helper test_line_count_cmd Đoàn Trần Công Danh ` (2 preceding siblings ...) 2021-06-12 4:27 ` [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh @ 2021-06-12 4:27 ` Đoàn Trần Công Danh 2021-06-13 3:43 ` Eric Sunshine 3 siblings, 1 reply; 22+ messages in thread From: Đoàn Trần Công Danh @ 2021-06-12 4:27 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Eric Sunshine, Ævar Arnfjörð Bjarmason, Junio C Hamano Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/t6402-merge-rename.sh | 126 +++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 67 deletions(-) diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh index 5d76cd6414..81502dba84 100755 --- a/t/t6402-merge-rename.sh +++ b/t/t6402-merge-rename.sh @@ -105,10 +105,8 @@ test_expect_success 'pull renaming branch into unrenaming one' \ git show-branch && test_expect_code 1 git pull . white && git ls-files -s && - git ls-files -u B >b.stages && - test_line_count = 3 b.stages && - git ls-files -s N >n.stages && - test_line_count = 1 n.stages && + test_line_count_cmd --out = 3 git ls-files -u B && + test_line_count_cmd --out = 1 git ls-files -s N && sed -ne "/^g/{ p q @@ -122,10 +120,8 @@ test_expect_success 'pull renaming branch into another renaming one' \ git reset --hard && git checkout red && test_expect_code 1 git pull . white && - git ls-files -u B >b.stages && - test_line_count = 3 b.stages && - git ls-files -s N >n.stages && - test_line_count = 1 n.stages && + test_line_count_cmd --out = 3 git ls-files -u B && + test_line_count_cmd --out = 1 git ls-files -s N && sed -ne "/^g/{ p q @@ -138,10 +134,8 @@ test_expect_success 'pull unrenaming branch into renaming one' \ git reset --hard && git show-branch && test_expect_code 1 git pull . main && - git ls-files -u B >b.stages && - test_line_count = 3 b.stages && - git ls-files -s N >n.stages && - test_line_count = 1 n.stages && + test_line_count_cmd --out = 3 git ls-files -u B && + test_line_count_cmd --out = 1 git ls-files -s N && sed -ne "/^g/{ p q @@ -154,14 +148,10 @@ test_expect_success 'pull conflicting renames' \ git reset --hard && git show-branch && test_expect_code 1 git pull . blue && - git ls-files -u A >a.stages && - test_line_count = 1 a.stages && - git ls-files -u B >b.stages && - test_line_count = 1 b.stages && - git ls-files -u C >c.stages && - test_line_count = 1 c.stages && - git ls-files -s N >n.stages && - test_line_count = 1 n.stages && + test_line_count_cmd --out = 1 git ls-files -u A && + test_line_count_cmd --out = 1 git ls-files -u B && + test_line_count_cmd --out = 1 git ls-files -u C && + test_line_count_cmd --out = 1 git ls-files -s N && sed -ne "/^g/{ p q @@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' ' test_i18ngrep "Adding as dir~HEAD instead" output fi && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && + test_line_count_cmd --out = 3 git ls-files -u && + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && @@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' ' test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output fi && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && + test_line_count_cmd --out = 3 git ls-files -u && + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && @@ -374,8 +364,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in git checkout -q renamed-file-has-conflicts^0 && test_must_fail git merge --strategy=recursive dir-not-in-way && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 3 -eq "$(git ls-files -u dir | wc -l)" && + test_line_count_cmd --out = 3 git ls-files -u && + test_line_count_cmd --out = 3 git ls-files -u dir && test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && @@ -409,14 +399,15 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t git checkout -q renamed-file-has-conflicts^0 && test_must_fail git merge --strategy=recursive dir-in-way && - test 5 -eq "$(git ls-files -u | wc -l)" && + test_line_count_cmd --out = 5 git ls-files -u && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)" + test_line_count_cmd --out = 3 git ls-files -u dir~HEAD else - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" + git ls-files -u dir >out && + test_line_count_cmd --out = 3 grep -v file-in-the-way out fi && - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && @@ -432,14 +423,15 @@ test_expect_success 'Same as previous, but merged other way' ' git checkout -q dir-in-way^0 && test_must_fail git merge --strategy=recursive renamed-file-has-conflicts && - test 5 -eq "$(git ls-files -u | wc -l)" && + test_line_count_cmd --out = 5 git ls-files -u && if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)" + test_line_count_cmd --out = 3 git ls-files -u dir~renamed-file-has-conflicts else - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" + git ls-files -u dir >out && + test_line_count_cmd --out = 3 grep -v file-in-the-way out fi && - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && test_must_fail git diff --quiet && test_must_fail git diff --cached --quiet && @@ -496,9 +488,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 2 -eq "$(git ls-files -u | wc -l)" + test_line_count_cmd --out = 2 git ls-files -u else - test 1 -eq "$(git ls-files -u | wc -l)" + test_line_count_cmd --out = 1 git ls-files -u fi && test_must_fail git diff --quiet && @@ -540,9 +532,9 @@ then mkdir one && test_must_fail git merge --strategy=recursive rename-two && - test 4 -eq "$(git ls-files -u | wc -l)" && - test 2 -eq "$(git ls-files -u one | wc -l)" && - test 2 -eq "$(git ls-files -u two | wc -l)" && + test_line_count_cmd --out = 4 git ls-files -u && + test_line_count_cmd --out = 2 git ls-files -u one && + test_line_count_cmd --out = 2 git ls-files -u two && test_must_fail git diff --quiet && @@ -559,9 +551,9 @@ else mkdir one && test_must_fail git merge --strategy=recursive rename-two && - test 2 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && + test_line_count_cmd --out = 2 git ls-files -u && + test_line_count_cmd --out = 1 git ls-files -u one && + test_line_count_cmd --out = 1 git ls-files -u two && test_must_fail git diff --quiet && @@ -582,13 +574,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 4 -eq "$(git ls-files -u | wc -l)" && - test 2 -eq "$(git ls-files -u one | wc -l)" && - test 2 -eq "$(git ls-files -u two | wc -l)" + test_line_count_cmd --out = 4 git ls-files -u && + test_line_count_cmd --out = 2 git ls-files -u one && + test_line_count_cmd --out = 2 git ls-files -u two else - test 2 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" + test_line_count_cmd --out = 2 git ls-files -u && + test_line_count_cmd --out = 1 git ls-files -u one && + test_line_count_cmd --out = 1 git ls-files -u two fi && test_must_fail git diff --quiet && @@ -631,19 +623,19 @@ test_expect_success 'check handling of differently renamed file with D/F conflic if test "$GIT_TEST_MERGE_ALGORITHM" = ort then - test 5 -eq "$(git ls-files -s | wc -l)" && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" && - test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" && - test 1 -eq "$(git ls-files -u original | wc -l)" && - test 0 -eq "$(git ls-files -o | wc -l)" + test_line_count_cmd --out = 5 git ls-files -s && + test_line_count_cmd --out = 3 git ls-files -u && + test_line_count_cmd --out = 1 git ls-files -u one~HEAD && + test_line_count_cmd --out = 1 git ls-files -u two~second-rename && + test_line_count_cmd --out = 1 git ls-files -u original && + test_line_count_cmd --out = 2 git ls-files -o else - test 5 -eq "$(git ls-files -s | wc -l)" && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && - test 1 -eq "$(git ls-files -u original | wc -l)" && - test 2 -eq "$(git ls-files -o | wc -l)" + test_line_count_cmd --out = 5 git ls-files -s && + test_line_count_cmd --out = 3 git ls-files -u && + test_line_count_cmd --out = 1 git ls-files -u one && + test_line_count_cmd --out = 1 git ls-files -u two && + test_line_count_cmd --out = 1 git ls-files -u original && + test_line_count_cmd --out = 4 git ls-files -o fi && test_path_is_file one/file && @@ -679,11 +671,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic git checkout -q first-rename-redo^0 && test_must_fail git merge --strategy=recursive second-rename-redo && - test 3 -eq "$(git ls-files -u | wc -l)" && - test 1 -eq "$(git ls-files -u one | wc -l)" && - test 1 -eq "$(git ls-files -u two | wc -l)" && - test 1 -eq "$(git ls-files -u original | wc -l)" && - test 0 -eq "$(git ls-files -o | wc -l)" && + test_line_count_cmd --out = 3 git ls-files -u && + test_line_count_cmd --out = 1 git ls-files -u one && + test_line_count_cmd --out = 1 git ls-files -u two && + test_line_count_cmd --out = 1 git ls-files -u original && + test_line_count_cmd --out = 2 git ls-files -o && test_path_is_file one && test_path_is_file two && @@ -861,8 +853,8 @@ test_expect_success 'setup merge of rename + small change' ' test_expect_success 'merge rename + small change' ' git merge rename_branch && - test 1 -eq $(git ls-files -s | wc -l) && - test 0 -eq $(git ls-files -o | wc -l) && + test_line_count_cmd --out = 1 git ls-files -s && + test_line_count_cmd --out = 2 git ls-files -o && test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file) ' -- 2.32.0.278.gd42b80f139 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] t6402: use test_line_count_cmd to count # of lines in stdout 2021-06-12 4:27 ` [PATCH 4/4] t6402: " Đoàn Trần Công Danh @ 2021-06-13 3:43 ` Eric Sunshine 0 siblings, 0 replies; 22+ messages in thread From: Eric Sunshine @ 2021-06-13 3:43 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: Git List, Ævar Arnfjörð Bjarmason, Junio C Hamano On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote: > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh > @@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' ' > - test 3 -eq "$(git ls-files -u | wc -l)" && > - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && > + test_line_count_cmd --out = 3 git ls-files -u && > + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && Nit: too many spaces before `&&`: s/\s+/ / > @@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' ' > - test 3 -eq "$(git ls-files -u | wc -l)" && > - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && > + test_line_count_cmd --out = 3 git ls-files -u && > + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && Ditto. > @@ -374,8 +364,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in > - test 3 -eq "$(git ls-files -u | wc -l)" && > - test 3 -eq "$(git ls-files -u dir | wc -l)" && > + test_line_count_cmd --out = 3 git ls-files -u && > + test_line_count_cmd --out = 3 git ls-files -u dir && Ditto. > @@ -409,14 +399,15 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t > - test 5 -eq "$(git ls-files -u | wc -l)" && > + test_line_count_cmd --out = 5 git ls-files -u && > if test "$GIT_TEST_MERGE_ALGORITHM" = ort > then > - test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)" > + test_line_count_cmd --out = 3 git ls-files -u dir~HEAD > else > - test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" > + git ls-files -u dir >out && > + test_line_count_cmd --out = 3 grep -v file-in-the-way out > fi && > - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && > + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && And again. > @@ -432,14 +423,15 @@ test_expect_success 'Same as previous, but merged other way' ' > - test 5 -eq "$(git ls-files -u | wc -l)" && > + test_line_count_cmd --out = 5 git ls-files -u && Here too. > - test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" && > + test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way && Blorp. And the rest of the changes suffer the same problem, so I won't continue commenting... ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-06-24 23:22 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-12 4:27 [PATCH 0/4] t: new helper test_line_count_cmd Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh 2021-06-13 3:10 ` Eric Sunshine 2021-06-13 7:36 ` Đoàn Trần Công Danh 2021-06-13 13:28 ` Ævar Arnfjörð Bjarmason 2021-06-13 16:37 ` Đoàn Trần Công Danh 2021-06-13 18:18 ` Phillip Wood 2021-06-13 21:42 ` Felipe Contreras 2021-06-13 23:43 ` Eric Sunshine 2021-06-14 2:56 ` Junio C Hamano 2021-06-24 23:19 ` Ævar Arnfjörð Bjarmason 2021-06-13 13:36 ` Ævar Arnfjörð Bjarmason 2021-06-14 3:01 ` Junio C Hamano 2021-06-15 15:40 ` Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 2/4] t6402: use find(1) builtin to filter instead of grep Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh 2021-06-12 4:33 ` Bagas Sanjaya 2021-06-13 7:39 ` Đoàn Trần Công Danh 2021-06-13 3:39 ` Eric Sunshine 2021-06-13 7:42 ` Đoàn Trần Công Danh 2021-06-12 4:27 ` [PATCH 4/4] t6402: " Đoàn Trần Công Danh 2021-06-13 3:43 ` Eric Sunshine
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.