Re: [PATCH 11/11] test-lib: clear watchman watches at test completion

> >> +		watchman watch-list |
> > 
> > Then with the above fixed, trying to run 'watchman' triggers another
> > error if it's not installed:
> > 
> >   $ GIT_TEST_FSMONITOR="$PWD"/t7519/fsmonitor-none ./t5570-git-daemon.sh 
> >   [...]
> >   ok 21 - hostname interpolation works after LF-stripping
> >   ./t5570-git-daemon.sh: 1484: ./t5570-git-daemon.sh: watchman: not found
> >   # failed 1 among 21 test(s)
> > 
> > I think we need an additional condition to run this only if
> > 't7519/fsmonitor-watchman' is used in the tests.
> 
> The intention is to enable a test-suite-wide run using GIT_TEST_FSMONITOR,
> and that can only use watchman (currently).

> >> diff --git a/t/test-lib.sh b/t/test-lib.sh
> >> index 30b07e310f..067a432ea5 100644
> >> --- a/t/test-lib.sh
> >> +++ b/t/test-lib.sh
> >> @@ -1072,6 +1072,8 @@ test_atexit_handler () {
> >>  	# sure that the registered cleanup commands are run only once.
> >>  	test : != "$test_atexit_cleanup" || return 0
> >>  
> >> +	test_clear_watchman
> > 
> > I'm not sure where to put this call, but this is definitely not the
> > right place for it.  See that 'return 0' above in the context?  That's
> > where the test_atexit_handler function returns early when no atexit
> > handler commands are set, i.e. in all test scripts that don't involve
> > some kind of daemons, thus this call is not invoked in the majority of
> > test scripts.
> 
> Ah, I misunderstood the point of test_atexit_handler.
> 
> > Simply moving this call before that early return is not good, because
> > then it would be invoked twice.
> > 
> > An option would be to register this call as an atexit command
> > somewhere late in 'test-lib.sh' (around where GIT_TEST_GETTEXT_POISON
> > is restored, perhaps).  That way it would be invoked most of the time,
> > and it would be invoked only once, but I'm not sure how it would work
> > out with test scripts that unset GIT_TEST_FSMONITOR somewhere in the
> > middle for the remainder of the test script.  However, register the
> > atexit command only if GIT_TEST_FSMONITOR is set (to something
> > watchman-specific), so it won't be invoked at all if
> > GIT_TEST_FSMONITOR is not set, and thus it won't generate additional
> > test output and trace.
> > 
> > I don't have a better idea.
> 
> Shouldn't it be sufficient to add it into test_done? If the test fails,
> then we could leave watches open, but that's no worse than we had without
> this test_clear_watchman method.