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

From: Pete Wyckoff <pw@padd.com>
Date: Feb 7, 2011, 22:26 UTC
Message-ID: <20110207222623.GB19851@arf.padd.com>
In-Reply-To: <7v1v3kwpm9.fsf@alter.siamese.dyndns.org>

gitster@pobox.com wrote on Sun, 06 Feb 2011 18:22 -0800:

> Pete Wyckoff <pw@padd.com> writes:

[..]

Show 7 quoted lines

> Use of two global variables with short names makes me feel "yeek!".
> 
> 	(p4 -h && p4d -h) >/dev/null 2>/dev/null ||
> 	{
> 		...
>                 test_done
> 	}

Much nicer.  Thanks.

Show 17 quoted lines

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

There is a noop ("p4 info") that I can use to test. But turns out I was wrong in even needing to sleep or wait for the "info" test to complete. In trying to get it to race, I found that p4d is well-behaved. Strace confirms that it does bind/listen before daemonizing. So that sleep can be removed.

I'll wait a while in case other comments come in, then send the updated series to you.

		-- Pete