All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Andrei Rybak <rybak.a.v@gmail.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>, git@vger.kernel.org
Cc: "Eric Sunshine" <sunshine@sunshineco.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Felipe Contreras" <felipe.contreras@gmail.com>
Subject: Re: [PATCH v3 1/4] test-lib-functions: introduce test_line_count_cmd
Date: Thu, 24 Jun 2021 21:23:14 +0200	[thread overview]
Message-ID: <6ad6ac5a-18ac-102e-29dc-3c0987de08e7@gmail.com> (raw)
In-Reply-To: <54bf7756-5578-4fe4-dbe5-f63b368c4d63@gmail.com>

On 21/06/2021 11:08, Andrei Rybak wrote:
> On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:
>> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
>> index f0448daa74..e055a4f808 100644
>> --- a/t/test-lib-functions.sh
>> +++ b/t/test-lib-functions.sh
>> @@ -845,6 +845,86 @@ test_line_count () {
>>       fi
>>   }
>> +# test_line_count_cmd checks the exit status, and the number of lines in
>> +# the captured stdout of a command.
>> +#
>> +# SYNOPSIS:
>> +#
>> +#    test_line_count_cmd <binop> <value> [!] cmd [args...]
>> +#
>> +# Expect succeed exit status when running
>> +#
>> +#    cmd [args...]
>> +#
>> +# then, run sh's "test <# of lines in stdout> <binop> <value>"
>> +#
>> +# OPTIONS:
>> +# !:
>> +#    Instead of expecting "cmd [args...]" succeed, expect its failure.
>> +#    Note, if the command under testing is "git",
>> +#    test_must_fail should be used instead of "!".
>> +#
>> +# EXAMPLE:
>> +#    test_line_count_cmd -ge 10 git tag --no-contains v1.0.0
>> +#    test_line_count_cmd -le 10 ! grep some-text a-file
>> +#    test_line_count_cmd = 0 test_must_fail git rev-parse --verify 
>> abcd1234
>> +#
>> +# NOTE:
>> +# * a temporary file named test_line_count_cmd_.out will be created 
>> under
>> +# $TRASH_DIRECTORY/.git/trash iff $TRASH_DIRECTORY/.git/ exists.
>> +# Otherwise, created in $TRASH_DIRECTORY. This temporary file will be
>> +# cleaned by test_when_finished
>> +test_line_count_cmd () {
>> +    {
>> +        local outop outval outfile
>> +        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 &&
>> +        if test $# -lt 3
>> +        then
>> +            BUG "missing <binary-ops> and <value>"
> 
> Nit: s/binary-ops/binop/ for consistency with documentation comment
> above.  Also, technically the invocation of test_line_count_cmd could be
> missing any of its required three parameters, including "cmd".  How
> about something similar to the call to BUG in test_i18ngrep:
> 
>      BUG "too few parameters to test_line_count_cmd"
> 
> ?
> 
>> +        fi &&
>> +        outop="$1" &&
>> +        outval="$2" &&
>> +        shift 2 &&
>> +        outfile="$trashdir/test_line_count_cmd_.out" &&
>> +        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"
>> +        else
>> +            test_when_finished "rm -f '$outfile'" &&
>> +            exec 8>"$outfile"
>> +            # We need to redirect stderr to &9,
>> +            # and redirect this function's 9>&2
>> +            # in order to not messed with -x
>> +            if ! "$@" >&8 2>&9
>> +            then
>> +                actual_failure=yes
>> +            fi
>> +        fi 8>&1 &&
>> +        case "$expect_failure,$actual_failure" in
>> +        yes,)
>> +            echo >&4 "error: '$@' succeed!" &&
> 
> It seems that function error() could be used here and below instead of
> "echo >&4".

After spending some time reading t/test-lib-functions.sh, now I don't
think that using error() is a good suggestion.  Closest relatives of
test_line_count_cmd -- test_line_count and test_must_be_empty -- both
just use "echo".  Other usages of error() in t/test-lib.sh and
t/test-lib-functions.sh suggest that error() is not meant to be used
for reporting test failure messages, but internal failures. For example:

	error "You haven't built things yet, have you?"

and

	error "Internal error: $stderr disappeared."

> s/succeed/succeeded/ --- it might be worth borrowing wording from
> test_must_fail().  Something like:
> 
>      error "test_line_count_cmd: command succeeded: '$@'"

  reply	other threads:[~2021-06-24 19:23 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 17:20 [PATCH v2 0/5] t: new helper test_line_count_cmd Đoàn Trần Công Danh
2021-06-15 17:20 ` [PATCH v2 1/5] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh
2021-06-17  4:51   ` Felipe Contreras
2021-06-15 17:20 ` [PATCH v2 2/5] t6402: use find(1) builtin to filter instead of grep Đoàn Trần Công Danh
2021-06-15 17:20 ` [PATCH v2 3/5] t0041: use test_line_count_cmd to check std{out,err} Đoàn Trần Công Danh
2021-06-16  3:06   ` Junio C Hamano
2021-06-16 14:21     ` Đoàn Trần Công Danh
2021-06-17  0:18       ` Junio C Hamano
2021-06-15 17:20 ` [PATCH v2 4/5] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh
2021-06-15 17:20 ` [PATCH v2 5/5] t6402: " Đoàn Trần Công Danh
2021-06-19  1:30 ` [PATCH v3 0/4] t: new helper test_line_count_cmd Đoàn Trần Công Danh
2021-06-19  5:50   ` Eric Sunshine
2021-06-19  6:17     ` Junio C Hamano
2021-06-19  6:26       ` Eric Sunshine
2021-06-19  6:50         ` Junio C Hamano
2021-06-21 23:52           ` Đoàn Trần Công Danh
2021-06-22  0:43             ` Eric Sunshine
2021-06-19  1:30 ` [PATCH v3 1/4] test-lib-functions: introduce test_line_count_cmd Đoàn Trần Công Danh
2021-06-21  9:08   ` Andrei Rybak
2021-06-24 19:23     ` Andrei Rybak [this message]
2021-06-19  1:30 ` [PATCH v3 2/4] t6402: use find(1) builtin to filter instead of grep Đoàn Trần Công Danh
2021-06-21  8:17   ` Andrei Rybak
2021-06-21 23:54     ` Đoàn Trần Công Danh
2021-06-19  1:30 ` [PATCH v3 3/4] t6400: use test_line_count_cmd to count # of lines in stdout Đoàn Trần Công Danh
2021-06-19  1:30 ` [PATCH v3 4/4] t6402: " Đoàn Trần Công Danh
2021-06-29 13:57 ` [PATCH v4 0/2] t640{0,2}: preserve ls-files exit status code Đoàn Trần Công Danh
2021-06-29 13:57   ` [PATCH v4 1/2] t6400: preserve git " Đoàn Trần Công Danh
2021-06-29 14:11     ` Eric Sunshine
2021-06-29 22:49       ` Junio C Hamano
2021-06-30  1:57         ` Eric Sunshine
2021-06-30  3:36           ` Junio C Hamano
2021-06-30 11:01             ` Đoàn Trần Công Danh
2021-06-30 20:44               ` Junio C Hamano
2021-06-29 13:57   ` [PATCH v4 2/2] t6402: preserve git " Đoàn Trần Công Danh
2021-06-29 20:49   ` [PATCH v4 0/2] t640{0,2}: preserve ls-files " Junio C Hamano
2021-07-04  5:46 ` [PATCH v5 0/3] new test-libs-function: test_stdout_line_count Đoàn Trần Công Danh
2021-07-04  5:46   ` [PATCH v5 1/3] test-lib-functions: introduce test_stdout_line_count Đoàn Trần Công Danh
2021-07-04  5:56     ` Eric Sunshine
2021-07-06 19:24       ` Junio C Hamano
2021-07-07  3:03         ` Đoàn Trần Công Danh
2021-07-04  5:46   ` [PATCH v5 2/3] t6400: preserve git ls-files exit status code Đoàn Trần Công Danh
2021-07-04  5:46   ` [PATCH v5 3/3] t6402: preserve git " Đoàn Trần Công Danh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6ad6ac5a-18ac-102e-29dc-3c0987de08e7@gmail.com \
    --to=rybak.a.v@gmail.com \
    --cc=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.