Re: [PATCH v6 25/30] t/perf/p7519: add fsmonitor--daemon test cases
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 7, 2022, 11:12 UTC
- Message-ID
- <220307.86v8wq10cb.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <b0c9c9c7a4005e2aa2a587fe5bc5588bf5a32d65.1646160212.git.gitgitgadget@gmail.com>
On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:
Show 22 quoted lines
> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Repeat all of the fsmonitor perf tests using `git fsmonitor--daemon` and
> the "Simple IPC" interface.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 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.
Show 30 quoted lines
> 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