Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout

> 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?

> 
>> 
>> 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() {