Re: [PATCH v5 3/8] upload-pack: test negotiation with changing repository

[Brandon Williams <bmwill@google.com>]

On 06/26, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> 
> > diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh
> > new file mode 100644
> > index 000000000..8a9a5aca0
> > --- /dev/null
> > +++ b/t/lib-httpd/one-time-sed.sh
> > @@ -0,0 +1,22 @@
> > +#!/bin/sh
> > +
> > +# If "one-time-sed" exists in $HTTPD_ROOT_PATH, run sed on the HTTP response,
> > +# using the contents of "one-time-sed" as the sed command to be run. If the
> > +# response was modified as a result, delete "one-time-sed" so that subsequent
> > +# HTTP responses are no longer modified.
> 
> ;-) clever.
> 
> > +# 
> > +# This can be used to simulate the effects of the repository changing in
> > +# between HTTP request-response pairs.
> > +if [ -e one-time-sed ]; then "$GIT_EXEC_PATH/git-http-backend" >out
> 
> Style (cf. Documentation/CodingGuidelines).
> 
> > +
> > +	sed "$(cat one-time-sed)" <out >out_modified
> > +
> > +	if diff out out_modified >/dev/null; then
> > +		cat out
> > +	else
> > +		cat out_modified
> > +		rm one-time-sed
> > +	fi
> > +else
> > +	"$GIT_EXEC_PATH/git-http-backend"
> > +fi
> 
> OK.  I was worried if the removal-after-use was about _this_ script
> (in which case it is a bad hygiene), but this is not a one-time
> script, but merely the mechanism to use the one-time sed script.
> 
> Perhaps rename this to t/lib-httpd/apply-one-time-sed.sh or something
> to avoid confusion?

Sure, I'll go ahead and rename it to that.

> 
> > diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
> > index 0ef182970..a4fe0e7e4 100755
> > --- a/t/t5703-upload-pack-ref-in-want.sh
> > +++ b/t/t5703-upload-pack-ref-in-want.sh
> > @@ -150,4 +150,72 @@ test_expect_success 'want-ref with ref we already have commit for' '
> >  	check_output
> >  '
> >  
> > +. "$TEST_DIRECTORY"/lib-httpd.sh
> > +start_httpd
> > +
> > +REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo"
> > +LOCAL_PRISTINE="$(pwd)/local_pristine"
> > +
> > +test_expect_success 'setup repos for change-while-negotiating test' '
> > +	(
> > +		git init "$REPO" &&
> > +		cd "$REPO" &&
> > +		>.git/git-daemon-export-ok &&
> > +		test_commit m1 &&
> > +		git tag -d m1 &&
> > +
> > +		# Local repo with many commits (so that negotiation will take
> > +		# more than 1 request/response pair)
> > +		git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" &&
> > +		cd "$LOCAL_PRISTINE" &&
> > +		git checkout -b side &&
> > +		for i in $(seq 1 33); do test_commit s$i; done &&
> > +
> > +		# Add novel commits to upstream
> > +		git checkout master &&
> > +		cd "$REPO" &&
> > +		test_commit m2 &&
> > +		test_commit m3 &&
> > +		git tag -d m2 m3
> > +	) &&
> > +	git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo" &&
> > +	git -C "$LOCAL_PRISTINE" config protocol.version 2
> > +'
> > +
> > +inconsistency() {
> 
> Style. "inconsistency () {"
> 
> > +	# Simulate that the server initially reports $2 as the ref
> > +	# corresponding to $1, and after that, $1 as the ref corresponding to
> > +	# $1. This corresponds to the real-life situation where the server's
> > +	# repository appears to change during negotiation, for example, when
> > +	# different servers in a load-balancing arrangement serve (stateless)
> > +	# RPCs during a single negotiation.
> > +	printf "s/%s/%s/" \
> > +	       $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
> > +	       $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
> > +	       >"$HTTPD_ROOT_PATH/one-time-sed"
> > +}
> > +
> > +test_expect_success 'server is initially ahead - no ref in want' '
> > +	git -C "$REPO" config uploadpack.allowRefInWant false &&
> > +	rm -rf local &&
> > +	cp -r "$LOCAL_PRISTINE" local &&
> > +	inconsistency master 1234567890123456789012345678901234567890 &&
> > +	test_must_fail git -C local fetch 2>err &&
> > +	grep "ERR upload-pack: not our ref" err
> > +'
> > +
> > +test_expect_success 'server is initially behind - no ref in want' '
> > +	git -C "$REPO" config uploadpack.allowRefInWant false &&
> > +	rm -rf local &&
> > +	cp -r "$LOCAL_PRISTINE" local &&
> > +	inconsistency master "master^" &&
> > +	git -C local fetch &&
> > +
> > +	git -C "$REPO" rev-parse --verify "master^" >expected &&
> > +	git -C local rev-parse --verify refs/remotes/origin/master >actual &&
> > +	test_cmp expected actual
> > +'
> > +
> > +stop_httpd
> > +
> >  test_done

-- 
Brandon Williams