t7527: improve implicit shutdown testing in fsmonitor--daemon

Refactor the tests that exercise implicit shutdown cases to make them more robust and less racy. The fsmonitor--daemon will implicitly shutdown in a variety of situations, such as when the ".git" directory is deleted or renamed. The existing tests would delete or rename the directory, sleep for one second, and then check the status of the daemon. This is racy, since the client/status command has no way to sync with the daemon. This was noticed occasionally on very slow CI build machines where it would cause a random test to fail. Replace the simple sleep with a sleep-and-retry loop. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff Hostetler committed May 26, 2022 at 21:47 UTC 3294ca6140875163b538eab08b56d1c8b3ccca5b
1 file changed +40 -14
t/t7527-builtin-fsmonitor.sh
+40 -14
@@ -124,6 +124,36 @@ test_expect_success 'implicit daemon start' '
124 test_must_fail git -C test_implicit fsmonitor--daemon status
125 '
126
127 +# Verify that the daemon has shutdown. Spin a few seconds to
128 +# make the test a little more robust during CI testing.
129 +#
130 +# We're looking for an implicit shutdown, such as when we delete or
131 +# rename the ".git" directory. Our delete/rename will cause a file
132 +# system event that the daemon will see and the daemon will
133 +# auto-shutdown as soon as it sees it. But this is racy with our `git
134 +# fsmonitor--daemon status` commands (and we cannot use a cookie file
135 +# here to help us). So spin a little and give the daemon a chance to
136 +# see the event. (This is primarily for underpowered CI build/test
137 +# machines (where it might take a moment to wake and reschedule the
138 +# daemon process) to avoid false alarms during test runs.)
139 +#
140 +IMPLICIT_TIMEOUT=5
141 +
142 +verify_implicit_shutdown () {
143 + r=$1 &&
144 +
145 + k=0 &&
146 + while test "$k" -lt $IMPLICIT_TIMEOUT
147 + do
148 + git -C $r fsmonitor--daemon status || return 0
149 +
150 + sleep 1
151 + k=$(( $k + 1 ))
152 + done &&
153 +
154 + return 1
155 +}
156 +
157 test_expect_success 'implicit daemon stop (delete .git)' '
158 test_when_finished "stop_daemon_delete_repo test_implicit_1" &&
159
@@ -142,10 +172,9 @@ test_expect_success 'implicit daemon stop (delete .git)' '
172 # This would make the test result dependent upon whether we
173 # were using fsmonitor on our development worktree.
174 #
145 - sleep 1 &&
175 mkdir test_implicit_1/.git &&
176
148 - test_must_fail git -C test_implicit_1 fsmonitor--daemon status
177 + verify_implicit_shutdown test_implicit_1
178 '
179
180 test_expect_success 'implicit daemon stop (rename .git)' '
@@ -160,10 +189,9 @@ test_expect_success 'implicit daemon stop (rename .git)' '
189
190 # See [1] above.
191 #
163 - sleep 1 &&
192 mkdir test_implicit_2/.git &&
193
166 - test_must_fail git -C test_implicit_2 fsmonitor--daemon status
194 + verify_implicit_shutdown test_implicit_2
195 '
196
197 # File systems on Windows may or may not have shortnames.
@@ -194,13 +222,11 @@ test_expect_success MINGW,SHORTNAMES 'implicit daemon stop (rename GIT~1)' '
222 #
223 mv test_implicit_1s/GIT~1 test_implicit_1s/.gitxyz &&
224
197 - sleep 1 &&
198 - # put it back so that our status will not crawl out to our
199 - # parent directory.
225 + # See [1] above.
226 # this moves {.gitxyz, GITXYZ~1} to {.git, GIT~1}.
227 mv test_implicit_1s/.gitxyz test_implicit_1s/.git &&
228
203 - test_must_fail git -C test_implicit_1s fsmonitor--daemon status
229 + verify_implicit_shutdown test_implicit_1s
230 '
231
232 # Here we first create a file with LONGNAME of "GIT~1" before
@@ -223,12 +249,10 @@ test_expect_success MINGW,SHORTNAMES 'implicit daemon stop (rename GIT~2)' '
249 #
250 mv test_implicit_1s2/GIT~2 test_implicit_1s2/.gitxyz &&
251
226 - sleep 1 &&
227 - # put it back so that our status will not crawl out to our
228 - # parent directory.
252 + # See [1] above.
253 mv test_implicit_1s2/.gitxyz test_implicit_1s2/.git &&
254
231 - test_must_fail git -C test_implicit_1s2 fsmonitor--daemon status
255 + verify_implicit_shutdown test_implicit_1s2
256 '
257
258 test_expect_success 'cannot start multiple daemons' '
@@ -905,9 +929,11 @@ test_expect_success CASE_INSENSITIVE_FS 'case insensitive+preserving' '
929 # Rename .git using an alternate spelling to verify that that
930 # daemon detects it and automatically shuts down.
931 mv test_insensitive/.GIT test_insensitive/.FOO &&
908 - sleep 1 &&
932 +
933 + # See [1] above.
934 mv test_insensitive/.FOO test_insensitive/.git &&
910 - test_must_fail git -C test_insensitive fsmonitor--daemon status &&
935 +
936 + verify_implicit_shutdown test_insensitive &&
937
938 # Verify that events were reported using on-disk spellings of the
939 # directories and files that we touched. We may or may not get a