Re: [PATCH 1/8] git-p4: test script

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 7, 2011, 02:22 UTC
Message-ID: <7v1v3kwpm9.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110205225105.GB30963@arf.padd.com>

Pete Wyckoff <pw@padd.com> writes:

Show 26 quoted lines

> Add a basic test script for git-p4.
>
> Signed-off-by: Pete Wyckoff <pw@padd.com>
> ---
>  t/t9800-git-p4.sh |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 62 insertions(+), 0 deletions(-)
>  create mode 100755 t/t9800-git-p4.sh
>
> diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
> new file mode 100755
> index 0000000..f4956b7
> --- /dev/null
> +++ b/t/t9800-git-p4.sh
> @@ -0,0 +1,62 @@
> +#!/bin/sh
> +
> +test_description='git-p4 tests'
> +
> +. ./test-lib.sh
> +
> +p4 -h >/dev/null 2>&1
> +retc=$?
> +p4d -h >/dev/null 2>&1
> +retd=$?
> +if test $retc -ne 0 -o $retd -ne 0
> +then

Use of two global variables with short names makes me feel "yeek!".

	(p4 -h && p4d -h) >/dev/null 2>/dev/null ||
	{
		...
                test_done
	}

Show 13 quoted lines

> +GITP4=$GIT_BUILD_DIR/contrib/fast-import/git-p4
> +P4DPORT=10669
> +
> +db="$TRASH_DIRECTORY/db"
> +cli="$TRASH_DIRECTORY/cli"
> +git="$TRASH_DIRECTORY/git"
> +
> +test_debug 'echo p4d -q -d -r "$db" -p $P4DPORT'
> +test_expect_success setup '
> +	mkdir -p "$db" &&
> +	p4d -q -d -r "$db" -p $P4DPORT &&
> +	# wait for it to finish its initialization
> +	sleep 1 &&

Is there a guarantee that "1" is sufficiently long for everybody?

Otherwise this will be a flaky test that sometimes passes and sometimes doesn't, which we try to avoid.

If the answer is "empirically 1 second is sufficient for 99.9% of people", then I would have to guess that it is 0.8 second too long for majority of people, in which case I would like to see us try harder to make it both reliable and efficient.

Isn't there a "noop" command a client can issue against a working server that fails when the server is not ready (or waits until the server becomes ready)?