Subject: Re: [PATCH v6 25/30] t/perf/p7519: add fsmonitor--daemon test cases

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 07 Mar 2022 12:12:20 +0100

On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:

> From: Jeff Hostetler <[email-masked]>
>
> Repeat all of the fsmonitor perf tests using `git fsmonitor--daemon` and
> the "Simple IPC" interface.
>
> Signed-off-by: Jeff Hostetler <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
>  t/perf/p7519-fsmonitor.sh | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
> index aed7b1146b0..a1c552129cc 100755
> --- a/t/perf/p7519-fsmonitor.sh
> +++ b/t/perf/p7519-fsmonitor.sh
> @@ -182,7 +182,10 @@ test_perf_w_drop_caches () {
>  }
>  
>  test_fsmonitor_suite () {
> -	if test -n "$INTEGRATION_SCRIPT"; then
> +	if test -n "$USE_FSMONITOR_DAEMON"
> +	then

...another candidate for "shell script style change" re my comment on
25/30.

> +		DESC="builtin fsmonitor--daemon"
> +	elif test -n "$INTEGRATION_SCRIPT"; then

...but it's especially odd in this case, becuse here the added code
*doesn't* follow our usual style... :)

Since this hunk-wise would conflict with the meaningful change I don't
mind if we "fix it while we're at it", but it really should use the
preferred style for new code then.

>  		DESC="fsmonitor=$(basename $INTEGRATION_SCRIPT)"
>  	else
>  		DESC="fsmonitor=disabled"
> @@ -293,4 +296,28 @@ test_expect_success "setup without fsmonitor" '
>  test_fsmonitor_suite
>  trace_stop
>  
> +#
> +# Run a full set of perf tests using the built-in fsmonitor--daemon.
> +# It does not use the Hook API, so it has a different setup.
> +# Explicitly start the daemon here and before we start client commands
> +# so that we can later add custom tracing.
> +#
> +if test_have_prereq FSMONITOR_DAEMON
> +then
> +	USE_FSMONITOR_DAEMON=t
> +
> +	trace_start fsmonitor--daemon--server
> +	git fsmonitor--daemon start
> +
> +	trace_start fsmonitor--daemon--client
> +
> +	git config core.fsmonitor true
> +	git update-index --fsmonitor
> +
> +	test_fsmonitor_suite
> +
> +	git fsmonitor--daemon stop
> +	trace_stop
> +fi

Urm, shouldn't this be in a test_perf or test_expect_success?

>  test_done