"Eric Rannaud" <[email protected]> writes:
> Any comments on the testing strategy with a background fast-import?
>
> To summarize:
> - fast-import is started in the background with a command stream that
> ends with "checkpoint\nprogress checkpoint\n". fast-import keeps
> running after reaching the last command (we don't want fast-import to
> terminate).
> - In the meantime, the test is waiting to read "progress checkpoint" in
> the output of fast-import, then it checks the testing conditions.
> - Finally, the test ensures that fast-import is still running in the
> background (and thus that it has just observed the effect of
> checkpoint, and not the side effects of fast-import terminating).
> - After 10 sec, no matter what, the background fast-import is sent
> "done" and terminates.
>
> I tried to make sure that the test runs quickly and does not (typically)
> sleep.
> Upon failure, the test may take up to 10 sec to fully terminate.
Hmmmm, it certainly looks a bit too brittle with many tweakables
like 10, 50, 55, etc. that heavily depend on the load on the system.
Sorry for asking you to go through the hoops.
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 67b8c50a5ab4..b410bf3a3a7a 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -3120,4 +3120,121 @@ test_expect_success 'U: validate root delete result' '
> compare_diff_raw expect actual
> '
>
> +###
> +### series V (checkpoint)
> +###
> +
> +# To make sure you're observing the side effects of checkpoint *before*
> +# fast-import terminates (and thus writes out its state), check that the
> +# fast-import process is still running using background_import_still_running
> +# *after* evaluating the test conditions.
> +background_import_until_checkpoint () {
> + options=$1
> + input_file=$2
> + ( cat $input_file; sleep 10; echo done ) | git fast-import $options
> >V.output &
> + echo $! >V.pid
> +
> + # The loop will poll for approximately 5 seconds before giving up.
> + n=0
> + while ! test "$(cat V.output)" = "progress checkpoint"; do
Micronit. Just like you have if/then on different lines, have
while/do on different lines, i.e.
while test "$(cat V.output)" != "progress checkpoint"
do
> + if test $n -gt 55
> + then
> +...
> +background_import_still_running () {
> + if ! ps --pid "$(cat V.pid)"
"ps --pid" is probably not portable (I think we'd do "kill -0 $pid"
instead in our existing tests---and it should be kosher according to
POSIX [*1*, *2*]).
> +test_expect_success 'V: checkpoint updates refs after reset' '
> + cat >input <<-INPUT_END &&
It is not wrong per-se but we quote INPUT_END when there is no
interpolation necessary in the body of here document to help
readers, like so:
cat >input <<-\INPUT_END &&
> + reset refs/heads/V
> + from refs/heads/U
> +
> + checkpoint
> + progress checkpoint
> + INPUT_END
> +test_expect_success 'V: checkpoint updates refs and marks after commit' '
> + cat >input <<-INPUT_END &&
This we do want interpolation and the above is correct.
[Footnotes]
*1* http://pubs.opengroup.org/onlinepubs/9699919799/utilities/kill.html
lists '0' as an allowed signal number to be sent, and defers to the
description of the kill() function to explain what happens.
*2* http://pubs.opengroup.org/onlinepubs/9699919799/functions/kill.html
says """If sig is 0 (the null signal), error checking is
performed but no signal is actually sent. The null signal can be
used to check the validity of pid."""
