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

Pete Wyckoff [off-list ref] writes:

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

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

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

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

Pete Wyckoff [off-list ref] writes:

[..]

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.

quoted

+	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