Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout
- From
Luke Diamand <luke@diamand.org>
- Date
- Nov 16, 2015, 08:36 UTC
- 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?
Show 29 quoted lines
>
> 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?
Show 7 quoted lines
> + done
> +}
> +
> +retry_until_fail() {
> + timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> + until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> + do :sleep here?
Show 28 quoted lines
> + 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() {
>