From: Luke Diamand <luke@diamand.org>
Date: Mon, 16 Nov 2015 08:36:31 GMT
Subject: Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout
Message-ID: <5649958F.2010407@diamand.org>
In-Reply-To: <1447592920-89228-4-git-send-email-larsxschneider@gmail.com>

On 15/11/15 13:08, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> In rare cases kill/cleanup operations in tests fail. Retry these
> operations with a timeout to make the test less flaky.

Should there be a sleep in that retry_until_success loop so that it 
doesn't spin sending signals to p4d?

Do we need to worry about the time offset being updated (e.g. NTP) while 
this is running?

>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>   t/lib-git-p4.sh | 31 +++++++++++++++++++++++--------
>   1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> index 7548225..8d6b48f 100644
> --- a/t/lib-git-p4.sh
> +++ b/t/lib-git-p4.sh
> @@ -6,6 +6,10 @@
>   # a subdirectory called "$git"
>   TEST_NO_CREATE_REPO=NoThanks
>
> +# Some operations require multiple attempts to be successful. Define
> +# here the maximal retry timeout in seconds.
> +RETRY_TIMEOUT=60
> +
>   . ./test-lib.sh
>
>   if ! test_have_prereq PYTHON
> @@ -121,22 +125,33 @@ p4_add_user() {
>   	EOF
>   }
>
> +retry_until_success() {
> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +    until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +    do :

  sleep here?

> +    done
> +}
> +
> +retry_until_fail() {
> +    timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +    until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +    do :

  sleep here?

> +    done
> +}
> +
>   kill_p4d() {
>   	pid=$(cat "$pidfile")
> -	# it had better exist for the first kill
> -	kill $pid &&
> -	for i in 1 2 3 4 5 ; do
> -		kill $pid >/dev/null 2>&1 || break
> -		sleep 1
> -	done &&
> +	retry_until_fail kill $pid
> +	retry_until_fail kill -9 $pid
>   	# complain if it would not die
>   	test_must_fail kill $pid >/dev/null 2>&1 &&
>   	rm -rf "$db" "$cli" "$pidfile"
>   }
>
>   cleanup_git() {
> -	rm -rf "$git" &&
> -	mkdir "$git"
> +	retry_until_success rm -r "$git"
> +	test_must_fail test -d "$git" &&
> +	retry_until_success mkdir "$git"
>   }
>
>   marshal_dump() {
>