Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> (+cc: Erik, Ilari, Duy)
> Hi,
>
> Clemens Buchacher wrote:
>
>> [Subject: daemon: add tests]
>
> Can't believe I missed this. That seems like a worthy cause ---
> can someone remind me why this is dropped, or if there are any
> tweaks I can help with to get it picked up again?
Thanks for your interest in this.
>> diff --git a/t/lib-daemon.sh b/t/lib-daemon.sh
>> new file mode 100644
>> index 0000000..30a89ea
>> --- /dev/null
>> +++ b/t/lib-daemon.sh
>> @@ -0,0 +1,52 @@
>> +#!/bin/sh
>> +
>> +if test -z "$GIT_TEST_DAEMON"
>> +then
>> + skip_all="Daemon testing disabled (define GIT_TEST_DAEMON to enable)"
>> + test_done
>> +fi
>> +
>> +LIB_DAEMON_PORT=${LIB_DAEMON_PORT-'8121'}
In lib-httpd.sh, LIB_HTTPD_PORT is defined in a similar way, but that is
always overridden by the users and the convention there is to use the test
numbers (cf. "git grep LIB_HTTPD_PORT t/"), which should be followed here
as well.
I am not very keen on the "lib-daemon.sh", GIT_TEST_DAEMON, etc. naming to
pretend as if "git daemon" will forever be the only daemon we will ever
ship, by the way. We might one day want to add an inotify daemon, a
daemon for the git-pubsub protocol or somesuch.
>> +DAEMON_PID=
>> +DAEMON_DOCUMENT_ROOT_PATH="$PWD"/repo
>> +DAEMON_URL=git://127.0.0.1:$LIB_DAEMON_PORT
>> +
>> +start_daemon() {
>> + if test -n "$DAEMON_PID"
>> + then
>> + error "start_daemon already called"
>> + fi
>> +
>> + mkdir -p "$DAEMON_DOCUMENT_ROOT_PATH"
>> +
>> + trap 'code=$?; stop_daemon; (exit $code); die' EXIT
>> +
>> + say >&3 "Starting git daemon ..."
>> + git daemon --listen=127.0.0.1 --port="$LIB_DAEMON_PORT" \
>> + --reuseaddr --verbose \
>> + --base-path="$DAEMON_DOCUMENT_ROOT_PATH" \
>> + "$@" "$DAEMON_DOCUMENT_ROOT_PATH" \
>> + >&3 2>&4 &
>> + DAEMON_PID=$!
>> +}
>> +
>> +stop_daemon() {
>> + if test -z "$DAEMON_PID"
>> + then
>> + return
>> + fi
>> +
>> + trap 'die' EXIT
>> +
>> + # kill git-daemon child of git
>> + say >&3 "Stopping git daemon ..."
>> + pkill -P "$DAEMON_PID"
How portable is this one (I usually do not trust use of pkill anywhere)?
>> + wait "$DAEMON_PID"
>> + ret=$?
# Please comment what 143 is on this line.
>> + if test $ret -ne 143
>> + then
>> + error "git daemon exited with status: $ret"
>> + fi
>> + DAEMON_PID=
>> +}
>> ...
>> +test_expect_success 'prepare pack objects' '
>> + cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
>> + (cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
>> + git --bare repack &&
As the later tests assume there will be only one pack, don't you want at
least "-a" and possibly "-a -d" here?
>> + git --bare prune-packed
>> + )
>> +'
>> +
>> +test_expect_success 'fetch notices corrupt pack' '
>> + cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
>> + (cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
>> + p=`ls objects/pack/pack-*.pack` &&
>> + chmod u+w $p &&
>> + printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
>> + ) &&
>> + mkdir repo_bad1.git &&
>> + (cd repo_bad1.git &&
>> + git --bare init &&
>> + test_must_fail git --bare fetch $DAEMON_URL/repo_bad1.git &&
>> + test 0 = `ls objects/pack/pack-*.pack | wc -l`
>> + )
>> +'
>> +
>> +test_expect_success 'fetch notices corrupt idx' '
>> + cp -R "$DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
>> + (cd "$DAEMON_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
>> + p=`ls objects/pack/pack-*.idx` &&
>> + chmod u+w $p &&
>> + printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
>> + ) &&
>> + mkdir repo_bad2.git &&
>> + (cd repo_bad2.git &&
>> + git --bare init &&
>> + test_must_fail git --bare fetch $DAEMON_URL/repo_bad2.git &&
>> + test 0 = `ls objects/pack | wc -l`
>> + )
>> +'
>> +
>> +test_remote_error()
>> +{
>> + do_export=YesPlease
>> + while test $# -gt 0
>> + do
>> + case $1 in
>> + -x)
>> + shift
>> + chmod -X "$DAEMON_DOCUMENT_ROOT_PATH/repo.git"
I find the use of cap X here dubious; it makes your intention unclear.
Are you interested in the current status of 'x' bits on that directory, or
are you more interested in dropping the executable/searchable bits from
the directory no matter what its current status is (rhetorical: I fully
expect that the answer is the latter)? The same comment applies to the use
of "chmod +X" at the end of this helper function.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html