On Wed, Jan 11, 2023 at 7:00 PM Andrei Rybak <rybak.a.v@xxxxxxxxx> wrote: > Most tests in t7527-builtin-fsmonitor.sh that start a daemon, use the > helper function test_when_finished with stop_daemon_delete_repo. > Function stop_daemon_delete_repo explicitly stops the daemon. Calling > it via test_when_finished is needed for tests that don't check daemon's > automatic shutdown logic [1] and it is needed to avoid daemons being > left running in case of breakage of the logic of automatic shutdown of > the daemon. > > Unlike these tests, test 'case insensitive+preserving' added in [2] has > a call to function test_when_finished commented out. It was commented > out in all versions of the patch [2] during development [3]. This seems > to not be intentional, because neither commit message in [2], nor the > comment above the test mention this line being commented out. Compare > it, for example, to "# unicode_debug=true" which is explicitly described > by a documentation comment above it. > > Uncomment test_when_finished for stop_daemon_delete_repo in test 'case > insensitive+preserving' to ensure that daemons are not left running in > cases when automatic shutdown logic of daemon itself is broken. > > Signed-off-by: Andrei Rybak <rybak.a.v@xxxxxxxxx> > --- > diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh > @@ -910,7 +910,7 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" ' > test_expect_success CASE_INSENSITIVE_FS 'case insensitive+preserving' ' > -# test_when_finished "stop_daemon_delete_repo test_insensitive" && > + test_when_finished "stop_daemon_delete_repo test_insensitive" && Nice. Thanks for working on this (and the series as a whole) in response to a very tangential comment of mine[1] when replying to an unrelated patch of yours. [1]: https://lore.kernel.org/git/CAPig+cTgUPWxMox_nSka52dML6_GHUUoY4HCtcq7+7J0oEyeNw@xxxxxxxxxxxxxx/
