On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:
> From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
>
> Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> t/t7527-builtin-fsmonitor.sh | 511 +++++++++++++++++++++++++++++++++++
> 1 file changed, 511 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..5f7b8e54233
> --- /dev/null
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -0,0 +1,511 @@
> +#!/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
> + git -C $r fsmonitor--daemon stop >/dev/null 2>/dev/null
Do we really need to quiet all its output? Why not just have the
--verbose option do its thing?
> + rm -rf $1
> + return 0
Missing &&-chaining
> +}
> +
> +start_daemon () {
> + case "$#" in
> + 1) r="-C $1";;
> + *) r="";
Our usual style is not to indent these.
But maybe just use the same pattern as test_commit et al use? It's a bit
more verbose, but IMO clearer.
> + esac
> +
> + git $r fsmonitor--daemon start || return $?
> + git $r fsmonitor--daemon status || return $?
Maybe don't do all this "return" and just &&-chain these instead (including the case/esac)?
> +
> + return 0
> +}
> +
> +# Is a Trace2 data event present with the given catetory and key?
> +# We do not care what the value is.
> +#
> +have_t2_data_event () {
> + c=$1
> + k=$2
> +
> + grep -e '"event":"data".*"category":"'"$c"'".*"key":"'"$k"'"'
> +}
Optional & aside: But it would be really nice to just have this amend
the test_region function into something more general, so this could be:
test_trace2 --event data --category "$c" --key "$k"
And have "test_region" then be a thin wrapper for that, and do the
appropriate "maybe only the start one, not the end one?" logic there.
> +test_expect_success 'explicit daemon start and stop' '
> + test_when_finished "stop_daemon_delete_repo test_explicit" &&
> +
> + git init test_explicit &&
> + start_daemon test_explicit &&
> +
> + git -C test_explicit fsmonitor--daemon stop &&
> + test_must_fail git -C test_explicit fsmonitor--daemon status
> +'
> +
> +test_expect_success 'implicit daemon start' '
> + test_when_finished "stop_daemon_delete_repo test_implicit" &&
> +
> + git init test_implicit &&
> + test_must_fail git -C test_implicit fsmonitor--daemon status &&
> +
> + # query will implicitly start the daemon.
> + #
> + # for test-script simplicity, we send a V1 timestamp rather than
> + # a V2 token. either way, the daemon response to any query contains
> + # a new V2 token. (the daemon may complain that we sent a V1 request,
> + # but this test case is only concerned with whether the daemon was
> + # implicitly started.)
> +
> + GIT_TRACE2_EVENT="$(pwd)/.git/trace" \
Better to use $PWD than $(pwd)
> +delete_files () {
> + rm -f delete
> + rm -f dir1/delete
> + rm -f dir2/delete
More missing &&-chaining.
> +}
> +
> +create_files () {
> + echo 1 >new
> + echo 2 >dir1/new
> + echo 3 >dir2/new
> +}
> +
> +rename_files () {
> + mv rename renamed
> + mv dir1/rename dir1/renamed
> + mv dir2/rename dir2/renamed
ditto.
> +}
> +
> +file_to_directory () {
> + rm -f delete
> + mkdir delete
> + echo 1 >delete/new
ditto.
> +}
> +
> +directory_to_file () {
> + rm -rf dir1
> + echo 1 >dir1
ditto.
> +}
> +
> +verify_status () {
This is used by nothing? Maybe it'll be used later, but that commit
could/should add it then?
Hrm, nope, just read ahead and nothing uses it at all.
> +clean_up_repo_and_stop_daemon () {
> + git reset --hard HEAD
> + git clean -fd
> + git fsmonitor--daemon stop
> + rm -f .git/trace
Missing &&-chaining (will stop noting these now, please look through the rest...)
> +}
> +
> +test_expect_success 'edit some files' '
> + test_when_finished clean_up_repo_and_stop_daemon &&
> +
> + (
> + GIT_TRACE_FSMONITOR="$(pwd)/.git/trace" &&
> + export GIT_TRACE_FSMONITOR &&
> +
> + start_daemon
Maybe have this "start_daemon" take an optional --trace argument or
something, allowing us to skip all these subsequent subshells.
> + ) &&
> +
> + edit_files &&
> +
> + test-tool fsmonitor-client query --token 0 >/dev/null 2>&1 &&
For these & the rest: just skip the quieting of the output? I.e. let the
test's --verbose do its job?
> +test_expect_success 'flush cached data' '
> + test_when_finished "stop_daemon_delete_repo test_flush" &&
> +
> + git init test_flush &&
> +
> + (
> + GIT_TEST_FSMONITOR_TOKEN=true &&
> + export GIT_TEST_FSMONITOR_TOKEN &&
> +
> + GIT_TRACE_FSMONITOR="$(pwd)/.git/trace_daemon" &&
> + export GIT_TRACE_FSMONITOR &&
> +
> + start_daemon test_flush
> + ) &&
> +
> + # The daemon should have an initial token with no events in _0 and
> + # then a few (probably platform-specific number of) events in _1.
> + # These should both have the same <token_id>.
> +
> + test-tool -C test_flush fsmonitor-client query --token "builtin:test_00000001:0" >actual_0 &&
> + nul_to_q <actual_0 >actual_q0 &&
> +
> + touch test_flush/file_1 &&
> + touch test_flush/file_2 &&
I may be missinga subtlety here, but is "touch" needed v.s. ">", i.e. we
just created "test_flush", so if we create a new file it'll have the
current timestamp.
Or did it get created by the helpers?
