Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> The "fsmonitor--daemon" test adjusted here was added in [3]. To assert
> that it didn't run into the "--super-prefix" message it was asserting
> the output it didn't have. Let's instead assert the full output that
> we *do* have, which we can do here as this is based on a change[4] to
> make it predictable (until [4] it contained absolute paths).
>
> We could also remove the test entirely (as [5] did), but even though
> the initial reason for having it is gone we're still getting some
> marginal benefit from testing the "fsmonitor" and "submodule
> absorbgitdirs" interaction, so let's keep it.
I'm a bit ambivalent on this, especially since we're testing "submodule
absorbgitdirs" output in "fsmonitor" tests, but as you say, there is
some benefit in testing the interaction. I think we'll have to change
the test though...
> The change here to have either a NULL or non-"" string as a
> "super_prefix" instead of the previous arrangement of "" or non-"" is
> somewhat arbitrary. We could also decide to never have to check for
> NULL.
>
> As we'll be changing the rest of the "git --super-prefix" users to the
> same pattern, leaving them all consistent makes sense. Why not pick ""
> over NULL? Because that's how the "prefix" works[6], and having
> "prefix" and "super_prefix" work the same way will be less
> confusing. That "prefix" picked NULL instead of "" is itself
> arbitrary, but as it's easy to make this small bit of our overall API
> consistent, let's go with that.
Okay, I find consistency with "prefix" convincing, and I do hope that we
use "" someday :)
> index 4abc74db2bb..31526937d95 100755
> --- a/t/t7527-builtin-fsmonitor.sh
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -866,30 +866,11 @@ test_expect_success 'submodule always visited' '
> # the submodule, and someone does a `git submodule absorbgitdirs`
> # in the super, Git will recursively invoke `git submodule--helper`
> # to do the work and this may try to read the index. This will
> -# try to start the daemon in the submodule *and* pass (either
> -# directly or via inheritance) the `--super-prefix` arg to the
> -# `git fsmonitor--daemon start` command inside the submodule.
> -# This causes a warning because fsmonitor--daemon does take that
> -# global arg (see the table in git.c)
We've removed mentions of the "--super-prefix", as we should, since
fsmonitor doesn't need to care about "--super-prefix" any more...
> @@ -904,10 +885,12 @@ test_expect_success "stray submodule super-prefix warning" '
>
> test_path_is_dir super/dir_1/dir_2/sub/.git &&
>
> - GIT_TRACE2_EVENT="$PWD/super-sub.trace" \
> - git -C super submodule absorbgitdirs &&
> -
> - ! have_t2_error_event super-sub.trace
> + cat >expect <<-\EOF &&
> + Migrating git directory of '\''dir_1/dir_2/sub'\'' from '\''dir_1/dir_2/sub/.git'\'' to '\''.git/modules/dir_1/dir_2/sub'\''
> + EOF
> + git -C super submodule absorbgitdirs >out 2>actual &&
> + test_cmp expect actual &&
> + test_must_be_empty out
> '
So let's adjust the test to match what we're testing. It's not the
--super-prefix any more, it's the interaction between "submodule
absorbgitdirs" and "fsmonitor--daemon", so perhaps something like:
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 31526937d9..9f947341f5 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -868,7 +868,7 @@ test_expect_success 'submodule always visited' '
# to do the work and this may try to read the index. This will
# try to start the daemon in the submodule.
-test_expect_success "stray submodule super-prefix warning" '
+test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
test_when_finished "rm -rf super; \
rm -rf sub" &&
@@ -888,9 +888,14 @@ test_expect_success "stray submodule super-prefix warning" '
cat >expect <<-\EOF &&
Migrating git directory of '\''dir_1/dir_2/sub'\'' from '\''dir_1/dir_2/sub/.git'\'' to '\''.git/modules/dir_1/dir_2/sub'\''
EOF
- git -C super submodule absorbgitdirs >out 2>actual &&
+ GIT_TRACE2_EVENT="$PWD/.git/trace_absorbgitdirs" \
+ git -C super submodule absorbgitdirs >out 2>actual &&
test_cmp expect actual &&
- test_must_be_empty out
+ test_must_be_empty out &&
+
+ # Confirm that the trace2 log contains a record of the
+ # daemon starting.
+ test_subcommand git fsmonitor--daemon start <.git/trace_absorbgitdirs
'
# On a case-insensitive file system, confirm that the daemon
