Re: [PATCH 08/16] t7527: add parameters to start_daemon to handle args and subshell
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 14, 2022, 08:03 UTC
- Message-ID
- <220314.8635jlez4c.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <6693900600f5b27525dfe7de78e73900306e0ab5.1647033303.git.gitgitgadget@gmail.com>
On Fri, Mar 11 2022, Jeff Hostetler via GitGitGadget wrote:
Show 40 quoted lines
> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> fixup! t7527: create test for fsmonitor--daemon
>
> Add parameters to start_daemon() and handle subshell and logging args.
>
> Remove /dev/null from commands.
>
> Fix &&-chaining of functions.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
> t/t7527-builtin-fsmonitor.sh | 217 ++++++++++++++++++-----------------
> 1 file changed, 111 insertions(+), 106 deletions(-)
>
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> index 0ccbfb9616f..026382a0d86 100755
> --- a/t/t7527-builtin-fsmonitor.sh
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -11,30 +11,85 @@ then
> fi
>
> stop_daemon_delete_repo () {
> - r=$1
> - git -C $r fsmonitor--daemon stop >/dev/null 2>/dev/null
> + r=$1 &&
> + test_might_fail git -C $r fsmonitor--daemon stop &&
> rm -rf $1
> - return 0
> }
>
> start_daemon () {
> - case "$#" in
> - 1) r="-C $1";;
> - *) r="";
> - esac
> + r="" &&
> + tf="" &&
> + t2="" &&
> + tk="" &&We usually just do :
x= &&
Not:
x="" &&
I.e. no need for the "".
Show 10 quoted lines
>
> - git $r fsmonitor--daemon start || return $?
> - git $r fsmonitor--daemon status || return $?
> + while test "$#" -ne 0
> + do
> + case "$1" in
> + -C)
> + shift;
> + test "$#" -ne 0 ||
> + { echo >&2 "error: -C requires arg"; exit 1; }IMO this exhaustive checking is better just skipped, we e.g.don't do this in "test_commit". Can't you just...
> + r="-C $1" > + shift
...properly &&-chain these whole things and have the "shift too many" and exit code from "shift" catch this?
Show 23 quoted lines
> + ;;
> + -tf)
> + shift;
> + test "$#" -ne 0 ||
> + { echo >&2 "error: -tf requires arg"; exit 1; }
> + tf="$1"
> + shift
> + ;;
> + -t2)
> + shift;
> + test "$#" -ne 0 ||
> + { echo >&2 "error: -t2 requires arg"; exit 1; }
> + t2="$1"
> + shift
> + ;;
> + -tk)
> + shift;
> + test "$#" -ne 0 ||
> + { echo >&2 "error: -tk requires arg"; exit 1; }
> + tk="$1"
> + shift
> + ;;
> + *)ditto.
> + echo >&2 "error: unknown option: '$1'" > + exit 1
But this sort of case is needed, but should use BUG "..."
Show 6 quoted lines
> + ;; > + esac > + done && > + > + ( > + if test ! -z "$tf"
Instead of "! -z x" use "-n x" ?
Show 12 quoted lines
> + then > + GIT_TRACE_FSMONITOR="$tf" > + export GIT_TRACE_FSMONITOR > + fi && > + > + if test ! -z "$t2" > + then > + GIT_TRACE2_PERF="$t2" > + export GIT_TRACE2_PERF > + fi && > + > + if test ! -z "$tk"
ditto.
> + then > + GIT_TEST_FSMONITOR_TOKEN="$tk" > + export GIT_TEST_FSMONITOR_TOKEN
Show 5 quoted lines
> + fi && > - touch test_flush/file_1 && > - touch test_flush/file_2 && > + > test_flush/file_1 && > + > test_flush/file_2 &&
style: ">x" not "> x" (ditto below).