On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>
> Repeat all of the fsmonitor perf tests using `git fsmonitor--daemon` and
> the "Simple IPC" interface.
>
> Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> 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
