Re: [PATCH v7 21/29] t7527: create test for fsmonitor--daemon
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 22, 2022, 18:35 UTC
- Message-ID
- <220322.86a6dhq1a6.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <4a920d0b54a25a442bf52efc171139c698d59dc7.1647972010.git.gitgitgadget@gmail.com>
On Tue, Mar 22 2022, Jeff Hostetler via GitGitGadget wrote:
Show 38 quoted lines
> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> t/t7527-builtin-fsmonitor.sh | 501 +++++++++++++++++++++++++++++++++++
> 1 file changed, 501 insertions(+)
> create mode 100755 t/t7527-builtin-fsmonitor.sh
>
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> new file mode 100755
> index 00000000000..d79635e7596
> --- /dev/null
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -0,0 +1,501 @@
> +#!/bin/sh
> +
> +test_description='built-in file system watcher'
> +
> +. ./test-lib.sh
> +
> +if ! test_have_prereq FSMONITOR_DAEMON
> +then
> + skip_all="fsmonitor--daemon is not supported on this platform"
> + test_done
> +fi
> +
> +stop_daemon_delete_repo () {
> + r=$1 &&
> + test_might_fail git -C $r fsmonitor--daemon stop &&
> + rm -rf $1
> +}
> +
> +start_daemon () {
> + r= &&
> + tf= &&
> + t2= &&
> + tk= &&I think these all have &&-chains added since previous versions, good...
Show 27 quoted lines
> + while test "$#" -ne 0 > + do > + case "$1" in > + -C) > + shift; > + test "$#" -ne 0 || BUG "error: -C requires arg" > + r="-C $1" > + shift > + ;; > + -tf) > + shift; > + test "$#" -ne 0 || BUG "error: -tf requires arg" > + tf="$1" > + shift > + ;; > + -t2) > + shift; > + test "$#" -ne 0 || BUG "error: -t2 requires arg" > + t2="$1" > + shift > + ;; > + -tk) > + shift; > + test "$#" -ne 0 || BUG "error: -tk requires arg" > + tk="$1" > + shift > + ;;
But (and IIRC I noted this in a previous iteration) if you &&-chain the "shift" here you can lose the more verbose BUG
> + start_daemon -tf "$PWD/.git/trace" &&
FWIW having an option parser take -tf to mean --tf is quite unlike our common conventions, usually it means both -t and -f.
In this case every single caller added here does provide -tf argument. Perhaps better as as unconditional $1 then?