[PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[Tarmigan Casebolt]

This should introduce no functional change in the tests or the amount
of test coverage.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/t5560-http-backend.sh          |  135 +-------------------------------------
 t/t5561-http-backend-noserver.sh |   52 +++++++++++++++
 t/t556x_common                   |  105 +++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 134 deletions(-)
 create mode 100755 t/t5561-http-backend-noserver.sh
 create mode 100755 t/t556x_common

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index ed034bc..b0d08e2 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -12,16 +12,6 @@ LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5560'}
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-find_file() {
-	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	find $1 -type f |
-	sed -e 1q
-}
-
-config() {
-	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
-}
-
 GET() {
 	curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
 	tr '\015' Q <out |
@@ -52,130 +42,7 @@ log_div() {
 	echo "###" >>"$HTTPD_ROOT_PATH"/access.log
 }
 
-test_expect_success 'setup repository' '
-	echo content >file &&
-	git add file &&
-	git commit -m one &&
-
-	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git --bare init &&
-	 : >objects/info/alternates &&
-	 : >objects/info/http-alternates
-	) &&
-	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	git push public master:master &&
-
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git repack -a -d
-	) &&
-
-	echo other >file &&
-	git add file &&
-	git commit -m two &&
-	git push public master:master &&
-
-	LOOSE_URL=$(find_file objects/??) &&
-	PACK_URL=$(find_file objects/pack/*.pack) &&
-	IDX_URL=$(find_file objects/pack/*.idx)
-'
-
-get_static_files() {
-	GET HEAD "$1" &&
-	GET info/refs "$1" &&
-	GET objects/info/packs "$1" &&
-	GET objects/info/alternates "$1" &&
-	GET objects/info/http-alternates "$1" &&
-	GET $LOOSE_URL "$1" &&
-	GET $PACK_URL "$1" &&
-	GET $IDX_URL "$1"
-}
-
-test_expect_success 'direct refs/heads/master not found' '
-	log_div "refs/heads/master"
-	GET refs/heads/master "404 Not Found"
-'
-test_expect_success 'static file is ok' '
-	log_div "getanyfile default"
-	get_static_files "200 OK"
-'
-test_expect_success 'static file if http.getanyfile true is ok' '
-	log_div "getanyfile true"
-	config http.getanyfile true &&
-	get_static_files "200 OK"
-'
-test_expect_success 'static file if http.getanyfile false fails' '
-	log_div "getanyfile false"
-	config http.getanyfile false &&
-	get_static_files "403 Forbidden"
-'
-
-test_expect_success 'http.uploadpack default enabled' '
-	log_div "uploadpack default"
-	GET info/refs?service=git-upload-pack "200 OK"  &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack true' '
-	log_div "uploadpack true"
-	config http.uploadpack true &&
-	GET info/refs?service=git-upload-pack "200 OK" &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack false' '
-	log_div "uploadpack false"
-	config http.uploadpack false &&
-	GET info/refs?service=git-upload-pack "403 Forbidden" &&
-	POST git-upload-pack 0000 "403 Forbidden"
-'
-
-test_expect_success 'http.receivepack default disabled' '
-	log_div "receivepack default"
-	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-test_expect_success 'http.receivepack true' '
-	log_div "receivepack true"
-	config http.receivepack true &&
-	GET info/refs?service=git-receive-pack "200 OK" &&
-	POST git-receive-pack 0000 "200 OK"
-'
-test_expect_success 'http.receivepack false' '
-	log_div "receivepack false"
-	config http.receivepack false &&
-	GET info/refs?service=git-receive-pack "403 Forbidden" &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
-	git http-backend >act.out 2>act.err
-}
-
-path_info() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		test_must_fail run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
-test_expect_success 'http-backend blocks bad PATH_INFO' '
-	config http.getanyfile true &&
-
-	run_backend 0 /repo.git/HEAD &&
-
-	run_backend 1 /repo.git/../HEAD &&
-	run_backend 1 /../etc/passwd &&
-	run_backend 1 ../etc/passwd &&
-	run_backend 1 /etc//passwd &&
-	run_backend 1 /etc/./passwd &&
-	run_backend 1 /etc/.../passwd &&
-	run_backend 1 //domain/data.txt
-'
+. "$TEST_DIRECTORY"/t556x_common
 
 cat >exp <<EOF
 
diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
new file mode 100755
index 0000000..501b328
--- /dev/null
+++ b/t/t5561-http-backend-noserver.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='test git-http-backend-noserver'
+. ./test-lib.sh
+
+HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
+
+GET() {
+    return 0
+}
+
+POST() {
+    return 0
+}
+
+logdiv() {
+    return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+run_backend() {
+	REQUEST_METHOD=GET \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="$2" \
+	git http-backend >act.out 2>act.err
+}
+
+path_info() {
+	if test $1 = 0; then
+		run_backend "$2"
+	else
+		test_must_fail run_backend "$2" &&
+		echo "fatal: '$2': aliased" >exp.err &&
+		test_cmp exp.err act.err
+	fi
+}
+
+test_expect_success 'http-backend blocks bad PATH_INFO' '
+	config http.getanyfile true &&
+
+	run_backend 0 /repo.git/HEAD &&
+
+	run_backend 1 /repo.git/../HEAD &&
+	run_backend 1 /../etc/passwd &&
+	run_backend 1 ../etc/passwd &&
+	run_backend 1 /etc//passwd &&
+	run_backend 1 /etc/./passwd &&
+	run_backend 1 /etc/.../passwd &&
+	run_backend 1 //domain/data.txt
+'
+test_done
diff --git a/t/t556x_common b/t/t556x_common
new file mode 100755
index 0000000..1845072
--- /dev/null
+++ b/t/t556x_common
@@ -0,0 +1,105 @@
+#!/bin/sh
+
+find_file() {
+	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	find $1 -type f |
+	sed -e 1q
+}
+
+config() {
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
+}
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 : >objects/info/alternates &&
+	 : >objects/info/http-alternates
+	) &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master &&
+
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git repack -a -d
+	) &&
+
+	echo other >file &&
+	git add file &&
+	git commit -m two &&
+	git push public master:master &&
+
+	LOOSE_URL=$(find_file objects/??) &&
+	PACK_URL=$(find_file objects/pack/*.pack) &&
+	IDX_URL=$(find_file objects/pack/*.idx)
+'
+
+get_static_files() {
+	GET HEAD "$1" &&
+	GET info/refs "$1" &&
+	GET objects/info/packs "$1" &&
+	GET objects/info/alternates "$1" &&
+	GET objects/info/http-alternates "$1" &&
+	GET $LOOSE_URL "$1" &&
+	GET $PACK_URL "$1" &&
+	GET $IDX_URL "$1"
+}
+
+test_expect_success 'direct refs/heads/master not found' '
+	log_div "refs/heads/master"
+	GET refs/heads/master "404 Not Found"
+'
+test_expect_success 'static file is ok' '
+	log_div "getanyfile default"
+	get_static_files "200 OK"
+'
+test_expect_success 'static file if http.getanyfile true is ok' '
+	log_div "getanyfile true"
+	config http.getanyfile true &&
+	get_static_files "200 OK"
+'
+test_expect_success 'static file if http.getanyfile false fails' '
+	log_div "getanyfile false"
+	config http.getanyfile false &&
+	get_static_files "403 Forbidden"
+'
+
+test_expect_success 'http.uploadpack default enabled' '
+	log_div "uploadpack default"
+	GET info/refs?service=git-upload-pack "200 OK"  &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack true' '
+	log_div "uploadpack true"
+	config http.uploadpack true &&
+	GET info/refs?service=git-upload-pack "200 OK" &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack false' '
+	log_div "uploadpack false"
+	config http.uploadpack false &&
+	GET info/refs?service=git-upload-pack "403 Forbidden" &&
+	POST git-upload-pack 0000 "403 Forbidden"
+'
+
+test_expect_success 'http.receivepack default disabled' '
+	log_div "receivepack default"
+	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
+test_expect_success 'http.receivepack true' '
+	log_div "receivepack true"
+	config http.receivepack true &&
+	GET info/refs?service=git-receive-pack "200 OK" &&
+	POST git-receive-pack 0000 "200 OK"
+'
+test_expect_success 'http.receivepack false' '
+	log_div "receivepack false"
+	config http.receivepack false &&
+	GET info/refs?service=git-receive-pack "403 Forbidden" &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
-- 
1.6.6.62.g67314

[PATCH RFC 2/2] Smart-http tests: Test http-backend without curl or a webserver

[Tarmigan Casebolt]

This reuses many of the tests from t5560 but runs those tests without curl
or a webserver.  This will hopefully increase the testing coverage for
http-backend because it does not require users to set GIT_TEST_HTTPD.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/t5561-http-backend-noserver.sh |   45 ++++++++++++++++++++++----------------
 1 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
index 501b328..6371e97 100755
--- a/t/t5561-http-backend-noserver.sh
+++ b/t/t5561-http-backend-noserver.sh
@@ -5,12 +5,35 @@ test_description='test git-http-backend-noserver'
 
 HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
 
+run_backend() {
+        echo "$3"| \
+        QUERY_STRING="${2#*\?}" \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="${2%%\?*}" \
+	git http-backend >act.out 2>act.err
+}
+
 GET() {
-    return 0
+    REQUEST_METHOD="GET" \
+    run_backend 0 "/repo.git/$1" &&
+    grep "Status" act.out >act
+    if [ $? -ne 1 ];
+    then
+	printf "Status: $2\r\n" > exp &&
+	test_cmp exp act
+    fi
 }
 
 POST() {
-    return 0
+    REQUEST_METHOD="POST" \
+    CONTENT_TYPE="application/x-$1-request" \
+    run_backend 0 "/repo.git/$1" "$2" &&
+    grep "Status" act.out >act
+    if [ $? -ne 1 ];
+    then
+	printf "Status: $3\r\n" > exp &&
+	test_cmp exp act
+    fi
 }
 
 logdiv() {
@@ -19,26 +42,10 @@ logdiv() {
 
 . "$TEST_DIRECTORY"/t556x_common
 
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
-	git http-backend >act.out 2>act.err
-}
-
-path_info() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		test_must_fail run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
 test_expect_success 'http-backend blocks bad PATH_INFO' '
 	config http.getanyfile true &&
 
+	REQUEST_METHOD="GET" &&
 	run_backend 0 /repo.git/HEAD &&
 
 	run_backend 1 /repo.git/../HEAD &&
-- 
1.6.6.62.g67314

Re: [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[Junio C Hamano]

Tarmigan Casebolt <tarmigan+git@gmail.com> writes:

> This should introduce no functional change in the tests or the amount
> of test coverage.
>
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>

This seems to crash rather badly with your own "Smart-http: check if
repository is OK to export before serving it".

Re: [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[Tarmigan]

On Wed, Dec 30, 2009 at 12:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tarmigan Casebolt <tarmigan+git@gmail.com> writes:
>
>> This should introduce no functional change in the tests or the amount
>> of test coverage.
>>
>> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
>
> This seems to crash rather badly with your own "Smart-http: check if
> repository is OK to export before serving it".

Yes, they were both separately based on 'master' a few days ago.

If you think the goal (more about that is in the commit log of 2/2) is
worthwhile, I am happy to rebase on top of pu and resend.

One reason it's labeled RFC is that I'm not very confident in my
ability to write portable shell script.  It works for me with bash,
but I'm not completely confident that is would work on ksh or dash.
So it would be nice if you could specifically take a look at the new
POST() and GET() and see if you notice anything obviously wrong there.

Thanks,
Tarmigan

[PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[Tarmigan Casebolt]

This should introduce no functional change in the tests or the amount
of test coverage.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
This is rebased on top of today's pu and the conflicts should be fixed.

 t/t5560-http-backend.sh          |  148 +-------------------------------------
 t/t5561-http-backend-noserver.sh |   52 +++++++++++++
 t/t556x_common                   |  119 ++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+), 147 deletions(-)
 create mode 100755 t/t5561-http-backend-noserver.sh
 create mode 100755 t/t556x_common

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 04a9896..dd844d3 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -12,16 +12,6 @@ LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5560'}
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-find_file() {
-	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	find $1 -type f |
-	sed -e 1q
-}
-
-config() {
-	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
-}
-
 GET() {
 	curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
 	tr '\015' Q <out |
@@ -52,143 +42,7 @@ log_div() {
 	echo "###" >>"$HTTPD_ROOT_PATH"/access.log
 }
 
-test_expect_success 'setup repository' '
-	echo content >file &&
-	git add file &&
-	git commit -m one &&
-
-	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git --bare init &&
-	 : >objects/info/alternates &&
-	 : >objects/info/http-alternates
-	) &&
-	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	git push public master:master &&
-
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git repack -a -d
-	) &&
-
-	echo other >file &&
-	git add file &&
-	git commit -m two &&
-	git push public master:master &&
-
-	LOOSE_URL=$(find_file objects/??) &&
-	PACK_URL=$(find_file objects/pack/*.pack) &&
-	IDX_URL=$(find_file objects/pack/*.idx)
-'
-
-get_static_files() {
-	GET HEAD "$1" &&
-	GET info/refs "$1" &&
-	GET objects/info/packs "$1" &&
-	GET objects/info/alternates "$1" &&
-	GET objects/info/http-alternates "$1" &&
-	GET $LOOSE_URL "$1" &&
-	GET $PACK_URL "$1" &&
-	GET $IDX_URL "$1"
-}
-
-SMART=smart
-test_expect_success 'direct refs/heads/master not found' '
-	log_div "refs/heads/master"
-	GET refs/heads/master "404 Not Found"
-'
-test_expect_success 'static file is ok' '
-	log_div "getanyfile default"
-	get_static_files "200 OK"
-'
-SMART=smart_noexport
-test_expect_success 'no export by default' '
-	log_div "no git-daemon-export-ok"
-	get_static_files "404 Not Found"
-'
-test_expect_success 'export if git-daemon-export-ok' '
-	log_div "git-daemon-export-ok"
-        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 touch git-daemon-export-ok
-	) &&
-        get_static_files "200 OK"
-'
-SMART=smart
-test_expect_success 'static file if http.getanyfile true is ok' '
-	log_div "getanyfile true"
-	config http.getanyfile true &&
-	get_static_files "200 OK"
-'
-test_expect_success 'static file if http.getanyfile false fails' '
-	log_div "getanyfile false"
-	config http.getanyfile false &&
-	get_static_files "403 Forbidden"
-'
-
-test_expect_success 'http.uploadpack default enabled' '
-	log_div "uploadpack default"
-	GET info/refs?service=git-upload-pack "200 OK"  &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack true' '
-	log_div "uploadpack true"
-	config http.uploadpack true &&
-	GET info/refs?service=git-upload-pack "200 OK" &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack false' '
-	log_div "uploadpack false"
-	config http.uploadpack false &&
-	GET info/refs?service=git-upload-pack "403 Forbidden" &&
-	POST git-upload-pack 0000 "403 Forbidden"
-'
-
-test_expect_success 'http.receivepack default disabled' '
-	log_div "receivepack default"
-	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-test_expect_success 'http.receivepack true' '
-	log_div "receivepack true"
-	config http.receivepack true &&
-	GET info/refs?service=git-receive-pack "200 OK" &&
-	POST git-receive-pack 0000 "200 OK"
-'
-test_expect_success 'http.receivepack false' '
-	log_div "receivepack false"
-	config http.receivepack false &&
-	GET info/refs?service=git-receive-pack "403 Forbidden" &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
-	git http-backend >act.out 2>act.err
-}
-
-path_info() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		test_must_fail run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
-test_expect_success 'http-backend blocks bad PATH_INFO' '
-	config http.getanyfile true &&
-
-	run_backend 0 /repo.git/HEAD &&
-
-	run_backend 1 /repo.git/../HEAD &&
-	run_backend 1 /../etc/passwd &&
-	run_backend 1 ../etc/passwd &&
-	run_backend 1 /etc//passwd &&
-	run_backend 1 /etc/./passwd &&
-	run_backend 1 /etc/.../passwd &&
-	run_backend 1 //domain/data.txt
-'
+. "$TEST_DIRECTORY"/t556x_common
 
 cat >exp <<EOF
 
diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
new file mode 100755
index 0000000..501b328
--- /dev/null
+++ b/t/t5561-http-backend-noserver.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='test git-http-backend-noserver'
+. ./test-lib.sh
+
+HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
+
+GET() {
+    return 0
+}
+
+POST() {
+    return 0
+}
+
+logdiv() {
+    return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+run_backend() {
+	REQUEST_METHOD=GET \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="$2" \
+	git http-backend >act.out 2>act.err
+}
+
+path_info() {
+	if test $1 = 0; then
+		run_backend "$2"
+	else
+		test_must_fail run_backend "$2" &&
+		echo "fatal: '$2': aliased" >exp.err &&
+		test_cmp exp.err act.err
+	fi
+}
+
+test_expect_success 'http-backend blocks bad PATH_INFO' '
+	config http.getanyfile true &&
+
+	run_backend 0 /repo.git/HEAD &&
+
+	run_backend 1 /repo.git/../HEAD &&
+	run_backend 1 /../etc/passwd &&
+	run_backend 1 ../etc/passwd &&
+	run_backend 1 /etc//passwd &&
+	run_backend 1 /etc/./passwd &&
+	run_backend 1 /etc/.../passwd &&
+	run_backend 1 //domain/data.txt
+'
+test_done
diff --git a/t/t556x_common b/t/t556x_common
new file mode 100755
index 0000000..1b4921c
--- /dev/null
+++ b/t/t556x_common
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+find_file() {
+	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	find $1 -type f |
+	sed -e 1q
+}
+
+config() {
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
+}
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 : >objects/info/alternates &&
+	 : >objects/info/http-alternates
+	) &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master &&
+
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git repack -a -d
+	) &&
+
+	echo other >file &&
+	git add file &&
+	git commit -m two &&
+	git push public master:master &&
+
+	LOOSE_URL=$(find_file objects/??) &&
+	PACK_URL=$(find_file objects/pack/*.pack) &&
+	IDX_URL=$(find_file objects/pack/*.idx)
+'
+
+get_static_files() {
+	GET HEAD "$1" &&
+	GET info/refs "$1" &&
+	GET objects/info/packs "$1" &&
+	GET objects/info/alternates "$1" &&
+	GET objects/info/http-alternates "$1" &&
+	GET $LOOSE_URL "$1" &&
+	GET $PACK_URL "$1" &&
+	GET $IDX_URL "$1"
+}
+
+SMART=smart
+test_expect_success 'direct refs/heads/master not found' '
+	log_div "refs/heads/master"
+	GET refs/heads/master "404 Not Found"
+'
+test_expect_success 'static file is ok' '
+	log_div "getanyfile default"
+	get_static_files "200 OK"
+'
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+	log_div "no git-daemon-export-ok"
+	get_static_files "404 Not Found"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+	log_div "git-daemon-export-ok"
+        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 touch git-daemon-export-ok
+	) &&
+        get_static_files "200 OK"
+'
+SMART=smart
+test_expect_success 'static file if http.getanyfile true is ok' '
+	log_div "getanyfile true"
+	config http.getanyfile true &&
+	get_static_files "200 OK"
+'
+test_expect_success 'static file if http.getanyfile false fails' '
+	log_div "getanyfile false"
+	config http.getanyfile false &&
+	get_static_files "403 Forbidden"
+'
+
+test_expect_success 'http.uploadpack default enabled' '
+	log_div "uploadpack default"
+	GET info/refs?service=git-upload-pack "200 OK"  &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack true' '
+	log_div "uploadpack true"
+	config http.uploadpack true &&
+	GET info/refs?service=git-upload-pack "200 OK" &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack false' '
+	log_div "uploadpack false"
+	config http.uploadpack false &&
+	GET info/refs?service=git-upload-pack "403 Forbidden" &&
+	POST git-upload-pack 0000 "403 Forbidden"
+'
+
+test_expect_success 'http.receivepack default disabled' '
+	log_div "receivepack default"
+	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
+test_expect_success 'http.receivepack true' '
+	log_div "receivepack true"
+	config http.receivepack true &&
+	GET info/refs?service=git-receive-pack "200 OK" &&
+	POST git-receive-pack 0000 "200 OK"
+'
+test_expect_success 'http.receivepack false' '
+	log_div "receivepack false"
+	config http.receivepack false &&
+	GET info/refs?service=git-receive-pack "403 Forbidden" &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
-- 
1.6.6

[PATCH RFC 2/2] Smart-http tests: Test http-backend without curl or a webserver

[Tarmigan Casebolt]

This reuses many of the tests from t5560 but runs those tests without curl
or a webserver.  This will hopefully increase the testing coverage for
http-backend because it does not require users to set GIT_TEST_HTTPD.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
In addition to the rebase, I fixed a bunch of bugs in this version.  I'm
still leaving this as RFC as there may be some more.

 t/t5561-http-backend-noserver.sh |   49 ++++++++++++++++++++++---------------
 t/t556x_common                   |    3 ++
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
index 501b328..bda5029 100755
--- a/t/t5561-http-backend-noserver.sh
+++ b/t/t5561-http-backend-noserver.sh
@@ -5,40 +5,49 @@ test_description='test git-http-backend-noserver'
 
 HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
 
+run_backend() {
+        echo "$3"| \
+        QUERY_STRING="${2#*\?}" \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="${2%%\?*}" \
+	git http-backend >act.out 2>act.err
+}
+
 GET() {
-    return 0
+    REQUEST_METHOD="GET" \
+    run_backend 0 "/repo.git/$1" &&
+    grep "Status" act.out >act
+    if [ $? -eq 1 ];
+    then
+	printf "Status: 200 OK\r\n" > act
+    fi
+    printf "Status: $2\r\n" > exp &&
+    test_cmp exp act
 }
 
 POST() {
-    return 0
+    REQUEST_METHOD="POST" \
+    CONTENT_TYPE="application/x-$1-request" \
+    run_backend 0 "/repo.git/$1" "$2" &&
+    grep "Status" act.out >act
+    if [ $? -eq 1 ];
+    then
+	printf "Status: 200 OK\r\n" > act
+    fi
+    printf "Status: $3\r\n" > exp &&
+    test_cmp exp act
 }
 
-logdiv() {
+log_div() {
     return 0
 }
 
 . "$TEST_DIRECTORY"/t556x_common
 
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
-	git http-backend >act.out 2>act.err
-}
-
-path_info() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		test_must_fail run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
 test_expect_success 'http-backend blocks bad PATH_INFO' '
 	config http.getanyfile true &&
 
+	REQUEST_METHOD="GET" &&
 	run_backend 0 /repo.git/HEAD &&
 
 	run_backend 1 /repo.git/../HEAD &&
diff --git a/t/t556x_common b/t/t556x_common
index 1b4921c..be024e5 100755
--- a/t/t556x_common
+++ b/t/t556x_common
@@ -50,6 +50,7 @@ get_static_files() {
 }
 
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'direct refs/heads/master not found' '
 	log_div "refs/heads/master"
 	GET refs/heads/master "404 Not Found"
@@ -59,6 +60,7 @@ test_expect_success 'static file is ok' '
 	get_static_files "200 OK"
 '
 SMART=smart_noexport
+unset GIT_HTTP_EXPORT_ALL
 test_expect_success 'no export by default' '
 	log_div "no git-daemon-export-ok"
 	get_static_files "404 Not Found"
@@ -71,6 +73,7 @@ test_expect_success 'export if git-daemon-export-ok' '
         get_static_files "200 OK"
 '
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'static file if http.getanyfile true is ok' '
 	log_div "getanyfile true"
 	config http.getanyfile true &&
-- 
1.6.6

Re: [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[Junio C Hamano]

Tarmigan <tarmigan+git@gmail.com> writes:

> One reason it's labeled RFC is that I'm not very confident in my
> ability to write portable shell script.  It works for me with bash,
> but I'm not completely confident that is would work on ksh or dash.
> So it would be nice if you could specifically take a look at the new
> POST() and GET() and see if you notice anything obviously wrong there.

Looked Ok to me from a cursory reading, even though I wonder what the
first argument to run_backend function is good for...

Re: [PATCH RFC 1/2] Smart-http tests: Break test t5560-http-backend into pieces

[Tarmigan]

On Thu, Dec 31, 2009 at 9:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tarmigan <tarmigan+git@gmail.com> writes:
>
>> One reason it's labeled RFC is that I'm not very confident in my
>> ability to write portable shell script.  It works for me with bash,
>> but I'm not completely confident that is would work on ksh or dash.
>> So it would be nice if you could specifically take a look at the new
>> POST() and GET() and see if you notice anything obviously wrong there.
>
> Looked Ok to me from a cursory reading, even though I wonder what the
> first argument to run_backend function is good for...

Thanks for looking.  I used the run_backend that was introduced in
34b6cb8bb, but looking more closely, it seems that the "http-backend
blocks bad PATH_INFO" test wasn't actually checking anything.  It
seems the path_info function was intended to be used, but never
actually was.  I'm embarrassed to say that I was so focused on "no
changes to existing tests" that I wasn't thinking about the bigger
picture.

I have made a patch to address this, and have made it the first in the
series.  I would like to have Shawn's ack on at least that first
patch, as I was trying to guess at his original intention with that
test.

Thanks,
Tarmigan

[PATCH v3 RFC 1/3] Smart-http tests: Improve coverage in test t5560

[Tarmigan Casebolt]

Commit 34b6cb8bb ("http-backend: Protect GIT_PROJECT_ROOT from /../
requests") added the path_info helper function to test t5560 but did
not use it.  We should use it as it provides another level of error
checking.

The /etc/.../passwd case is one that is not special (and the test
fails for reasons other than being aliased), so we remove that test
case.

Also rename the function from 'path_info' to 'expect_aliased'.

cc: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>

---
One thing to note is that
	expect_aliased 0 /repo.git/HEAD
test still does not actually test any results back from http-backend,
but that's also how it was before as well.
---
 t/t5560-http-backend.sh |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 04a9896..0e4dc4b 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -162,15 +162,15 @@ test_expect_success 'http.receivepack false' '
 run_backend() {
 	REQUEST_METHOD=GET \
 	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
+	PATH_INFO="$1" \
 	git http-backend >act.out 2>act.err
 }
 
-path_info() {
+expect_aliased() {
 	if test $1 = 0; then
 		run_backend "$2"
 	else
-		test_must_fail run_backend "$2" &&
+		run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
@@ -179,15 +179,14 @@ path_info() {
 test_expect_success 'http-backend blocks bad PATH_INFO' '
 	config http.getanyfile true &&
 
-	run_backend 0 /repo.git/HEAD &&
+	expect_aliased 0 /repo.git/HEAD &&
 
-	run_backend 1 /repo.git/../HEAD &&
-	run_backend 1 /../etc/passwd &&
-	run_backend 1 ../etc/passwd &&
-	run_backend 1 /etc//passwd &&
-	run_backend 1 /etc/./passwd &&
-	run_backend 1 /etc/.../passwd &&
-	run_backend 1 //domain/data.txt
+	expect_aliased 1 /repo.git/../HEAD &&
+	expect_aliased 1 /../etc/passwd &&
+	expect_aliased 1 ../etc/passwd &&
+	expect_aliased 1 /etc//passwd &&
+	expect_aliased 1 /etc/./passwd &&
+	expect_aliased 1 //domain/data.txt
 '
 
 cat >exp <<EOF
-- 
1.6.6

[PATCH v3 2/3] Smart-http tests: Break test t5560-http-backend into pieces

[Tarmigan Casebolt]

This should introduce no functional change in the tests or the amount
of test coverage.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/t5560-http-backend.sh          |  147 +-------------------------------------
 t/t5561-http-backend-noserver.sh |   52 +++++++++++++
 t/t556x_common                   |  119 ++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+), 146 deletions(-)
 create mode 100755 t/t5561-http-backend-noserver.sh
 create mode 100755 t/t556x_common

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 0e4dc4b..dd844d3 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -12,16 +12,6 @@ LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5560'}
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-find_file() {
-	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	find $1 -type f |
-	sed -e 1q
-}
-
-config() {
-	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
-}
-
 GET() {
 	curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
 	tr '\015' Q <out |
@@ -52,142 +42,7 @@ log_div() {
 	echo "###" >>"$HTTPD_ROOT_PATH"/access.log
 }
 
-test_expect_success 'setup repository' '
-	echo content >file &&
-	git add file &&
-	git commit -m one &&
-
-	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git --bare init &&
-	 : >objects/info/alternates &&
-	 : >objects/info/http-alternates
-	) &&
-	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	git push public master:master &&
-
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git repack -a -d
-	) &&
-
-	echo other >file &&
-	git add file &&
-	git commit -m two &&
-	git push public master:master &&
-
-	LOOSE_URL=$(find_file objects/??) &&
-	PACK_URL=$(find_file objects/pack/*.pack) &&
-	IDX_URL=$(find_file objects/pack/*.idx)
-'
-
-get_static_files() {
-	GET HEAD "$1" &&
-	GET info/refs "$1" &&
-	GET objects/info/packs "$1" &&
-	GET objects/info/alternates "$1" &&
-	GET objects/info/http-alternates "$1" &&
-	GET $LOOSE_URL "$1" &&
-	GET $PACK_URL "$1" &&
-	GET $IDX_URL "$1"
-}
-
-SMART=smart
-test_expect_success 'direct refs/heads/master not found' '
-	log_div "refs/heads/master"
-	GET refs/heads/master "404 Not Found"
-'
-test_expect_success 'static file is ok' '
-	log_div "getanyfile default"
-	get_static_files "200 OK"
-'
-SMART=smart_noexport
-test_expect_success 'no export by default' '
-	log_div "no git-daemon-export-ok"
-	get_static_files "404 Not Found"
-'
-test_expect_success 'export if git-daemon-export-ok' '
-	log_div "git-daemon-export-ok"
-        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 touch git-daemon-export-ok
-	) &&
-        get_static_files "200 OK"
-'
-SMART=smart
-test_expect_success 'static file if http.getanyfile true is ok' '
-	log_div "getanyfile true"
-	config http.getanyfile true &&
-	get_static_files "200 OK"
-'
-test_expect_success 'static file if http.getanyfile false fails' '
-	log_div "getanyfile false"
-	config http.getanyfile false &&
-	get_static_files "403 Forbidden"
-'
-
-test_expect_success 'http.uploadpack default enabled' '
-	log_div "uploadpack default"
-	GET info/refs?service=git-upload-pack "200 OK"  &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack true' '
-	log_div "uploadpack true"
-	config http.uploadpack true &&
-	GET info/refs?service=git-upload-pack "200 OK" &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack false' '
-	log_div "uploadpack false"
-	config http.uploadpack false &&
-	GET info/refs?service=git-upload-pack "403 Forbidden" &&
-	POST git-upload-pack 0000 "403 Forbidden"
-'
-
-test_expect_success 'http.receivepack default disabled' '
-	log_div "receivepack default"
-	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-test_expect_success 'http.receivepack true' '
-	log_div "receivepack true"
-	config http.receivepack true &&
-	GET info/refs?service=git-receive-pack "200 OK" &&
-	POST git-receive-pack 0000 "200 OK"
-'
-test_expect_success 'http.receivepack false' '
-	log_div "receivepack false"
-	config http.receivepack false &&
-	GET info/refs?service=git-receive-pack "403 Forbidden" &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$1" \
-	git http-backend >act.out 2>act.err
-}
-
-expect_aliased() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
-test_expect_success 'http-backend blocks bad PATH_INFO' '
-	config http.getanyfile true &&
-
-	expect_aliased 0 /repo.git/HEAD &&
-
-	expect_aliased 1 /repo.git/../HEAD &&
-	expect_aliased 1 /../etc/passwd &&
-	expect_aliased 1 ../etc/passwd &&
-	expect_aliased 1 /etc//passwd &&
-	expect_aliased 1 /etc/./passwd &&
-	expect_aliased 1 //domain/data.txt
-'
+. "$TEST_DIRECTORY"/t556x_common
 
 cat >exp <<EOF
 
diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
new file mode 100755
index 0000000..a9ba2d9
--- /dev/null
+++ b/t/t5561-http-backend-noserver.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='test git-http-backend-noserver'
+. ./test-lib.sh
+
+HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
+
+run_backend() {
+	REQUEST_METHOD=GET \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="$1" \
+	git http-backend >act.out 2>act.err
+}
+
+GET() {
+	return 0
+}
+
+POST() {
+	return 0
+}
+
+log_div() {
+	return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+expect_aliased() {
+	if test $1 = 0; then
+		run_backend "$2"
+	else
+		run_backend "$2" &&
+		echo "fatal: '$2': aliased" >exp.err &&
+		test_cmp exp.err act.err
+	fi
+}
+
+test_expect_success 'http-backend blocks bad PATH_INFO' '
+	config http.getanyfile true &&
+
+	expect_aliased 0 /repo.git/HEAD &&
+
+	expect_aliased 1 /repo.git/../HEAD &&
+	expect_aliased 1 /../etc/passwd &&
+	expect_aliased 1 ../etc/passwd &&
+	expect_aliased 1 /etc//passwd &&
+	expect_aliased 1 /etc/./passwd &&
+	expect_aliased 1 //domain/data.txt
+'
+
+test_done
diff --git a/t/t556x_common b/t/t556x_common
new file mode 100755
index 0000000..1b4921c
--- /dev/null
+++ b/t/t556x_common
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+find_file() {
+	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	find $1 -type f |
+	sed -e 1q
+}
+
+config() {
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
+}
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 : >objects/info/alternates &&
+	 : >objects/info/http-alternates
+	) &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master &&
+
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git repack -a -d
+	) &&
+
+	echo other >file &&
+	git add file &&
+	git commit -m two &&
+	git push public master:master &&
+
+	LOOSE_URL=$(find_file objects/??) &&
+	PACK_URL=$(find_file objects/pack/*.pack) &&
+	IDX_URL=$(find_file objects/pack/*.idx)
+'
+
+get_static_files() {
+	GET HEAD "$1" &&
+	GET info/refs "$1" &&
+	GET objects/info/packs "$1" &&
+	GET objects/info/alternates "$1" &&
+	GET objects/info/http-alternates "$1" &&
+	GET $LOOSE_URL "$1" &&
+	GET $PACK_URL "$1" &&
+	GET $IDX_URL "$1"
+}
+
+SMART=smart
+test_expect_success 'direct refs/heads/master not found' '
+	log_div "refs/heads/master"
+	GET refs/heads/master "404 Not Found"
+'
+test_expect_success 'static file is ok' '
+	log_div "getanyfile default"
+	get_static_files "200 OK"
+'
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+	log_div "no git-daemon-export-ok"
+	get_static_files "404 Not Found"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+	log_div "git-daemon-export-ok"
+        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 touch git-daemon-export-ok
+	) &&
+        get_static_files "200 OK"
+'
+SMART=smart
+test_expect_success 'static file if http.getanyfile true is ok' '
+	log_div "getanyfile true"
+	config http.getanyfile true &&
+	get_static_files "200 OK"
+'
+test_expect_success 'static file if http.getanyfile false fails' '
+	log_div "getanyfile false"
+	config http.getanyfile false &&
+	get_static_files "403 Forbidden"
+'
+
+test_expect_success 'http.uploadpack default enabled' '
+	log_div "uploadpack default"
+	GET info/refs?service=git-upload-pack "200 OK"  &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack true' '
+	log_div "uploadpack true"
+	config http.uploadpack true &&
+	GET info/refs?service=git-upload-pack "200 OK" &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack false' '
+	log_div "uploadpack false"
+	config http.uploadpack false &&
+	GET info/refs?service=git-upload-pack "403 Forbidden" &&
+	POST git-upload-pack 0000 "403 Forbidden"
+'
+
+test_expect_success 'http.receivepack default disabled' '
+	log_div "receivepack default"
+	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
+test_expect_success 'http.receivepack true' '
+	log_div "receivepack true"
+	config http.receivepack true &&
+	GET info/refs?service=git-receive-pack "200 OK" &&
+	POST git-receive-pack 0000 "200 OK"
+'
+test_expect_success 'http.receivepack false' '
+	log_div "receivepack false"
+	config http.receivepack false &&
+	GET info/refs?service=git-receive-pack "403 Forbidden" &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
-- 
1.6.6

[PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Tarmigan Casebolt]

This reuses many of the tests from t5560 but runs those tests without
curl or a webserver.  This will hopefully increase the testing
coverage for http-backend because it does not require users to set
GIT_TEST_HTTPD.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/t5561-http-backend-noserver.sh |   30 ++++++++++++++++++++++++------
 t/t556x_common                   |    3 +++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/t/t5561-http-backend-noserver.sh b/t/t5561-http-backend-noserver.sh
index a9ba2d9..9013824 100755
--- a/t/t5561-http-backend-noserver.sh
+++ b/t/t5561-http-backend-noserver.sh
@@ -6,18 +6,36 @@ test_description='test git-http-backend-noserver'
 HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
 
 run_backend() {
-	REQUEST_METHOD=GET \
+	echo "$2"| \
+	QUERY_STRING="${1#*\?}" \
 	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$1" \
+	PATH_INFO="${1%%\?*}" \
 	git http-backend >act.out 2>act.err
 }
 
 GET() {
-	return 0
+	REQUEST_METHOD="GET" \
+	run_backend "/repo.git/$1" &&
+	grep "Status" act.out >act
+	if [ $? -eq 1 ];
+	then
+		printf "Status: 200 OK\r\n" > act
+	fi
+	printf "Status: $2\r\n" > exp &&
+	test_cmp exp act
 }
 
 POST() {
-	return 0
+	REQUEST_METHOD="POST" \
+	CONTENT_TYPE="application/x-$1-request" \
+	run_backend "/repo.git/$1" "$2" &&
+	grep "Status" act.out >act
+	if [ $? -eq 1 ];
+	then
+		printf "Status: 200 OK\r\n" > act
+	fi
+	printf "Status: $3\r\n" > exp &&
+	test_cmp exp act
 }
 
 log_div() {
@@ -28,9 +46,9 @@ log_div() {
 
 expect_aliased() {
 	if test $1 = 0; then
-		run_backend "$2"
+		REQUEST_METHOD=GET run_backend "$2"
 	else
-		run_backend "$2" &&
+		REQUEST_METHOD=GET run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
diff --git a/t/t556x_common b/t/t556x_common
index 1b4921c..be024e5 100755
--- a/t/t556x_common
+++ b/t/t556x_common
@@ -50,6 +50,7 @@ get_static_files() {
 }
 
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'direct refs/heads/master not found' '
 	log_div "refs/heads/master"
 	GET refs/heads/master "404 Not Found"
@@ -59,6 +60,7 @@ test_expect_success 'static file is ok' '
 	get_static_files "200 OK"
 '
 SMART=smart_noexport
+unset GIT_HTTP_EXPORT_ALL
 test_expect_success 'no export by default' '
 	log_div "no git-daemon-export-ok"
 	get_static_files "404 Not Found"
@@ -71,6 +73,7 @@ test_expect_success 'export if git-daemon-export-ok' '
         get_static_files "200 OK"
 '
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'static file if http.getanyfile true is ok' '
 	log_div "getanyfile true"
 	config http.getanyfile true &&
-- 
1.6.6

Re: [PATCH v3 RFC 1/3] Smart-http tests: Improve coverage in test t5560

[Shawn O. Pearce]

Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> Commit 34b6cb8bb ("http-backend: Protect GIT_PROJECT_ROOT from /../
> requests") added the path_info helper function to test t5560 but did
> not use it.  We should use it as it provides another level of error
> checking.
> 
> The /etc/.../passwd case is one that is not special (and the test
> fails for reasons other than being aliased), so we remove that test
> case.
> 
> Also rename the function from 'path_info' to 'expect_aliased'.
> 
> cc: Shawn O. Pearce <spearce@spearce.org>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

-- 
Shawn.

Re: [PATCH v3 2/3] Smart-http tests: Break test t5560-http-backend into pieces

[Shawn O. Pearce]

Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> This should introduce no functional change in the tests or the amount
> of test coverage.
> 
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
> ---
>  t/t5560-http-backend.sh          |  147 +-------------------------------------
>  t/t5561-http-backend-noserver.sh |   52 +++++++++++++

These should be reversed.  We'd want to find out the backend doesn't
work by itself before we see it fail under Apache.

Otherwise, Acked-by: Shawn O. Pearce <spearce@spearce.org>

-- 
Shawn.

Re: [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Shawn O. Pearce]

Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
>  GET() {
> -	return 0
> +	REQUEST_METHOD="GET" \
> +	run_backend "/repo.git/$1" &&
> +	grep "Status" act.out >act
> +	if [ $? -eq 1 ];

I think this should be spelled as:

	if ! grep "Status" act.out >act; then
		printf "Status: 200 OK\r\n" > act
	fi

-- 
Shawn.

Re: [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Tarmigan]

On Sat, Jan 2, 2010 at 1:03 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
>>  GET() {
>> -     return 0
>> +     REQUEST_METHOD="GET" \
>> +     run_backend "/repo.git/$1" &&
>> +     grep "Status" act.out >act
>> +     if [ $? -eq 1 ];
>
> I think this should be spelled as:
>
>        if ! grep "Status" act.out >act; then
>                printf "Status: 200 OK\r\n" > act
>        fi

OK.  I had figured the "1" would mean "no matches not found"  rather
than some other error like "file does not exist".  Not sure how
portable that error value convention is for greps though, so I've
changed it as you suggested.

Thanks,
Tarmigan

[PATCH v4 1/3] Smart-http tests: Improve coverage in test t5560

[Tarmigan Casebolt]

Commit 34b6cb8bb ("http-backend: Protect GIT_PROJECT_ROOT from /../
requests") added the path_info helper function to test t5560 but did
not use it.  We should use it as it provides another level of error
checking.

The /etc/.../passwd case is one that is not special (and the test
fails for reasons other than being aliased), so we remove that test
case.

Also rename the function from 'path_info' to 'expect_aliased'.

Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>

---
One thing to note is that
	expect_aliased 0 /repo.git/HEAD
test still does not actually test any results back from http-backend,
but that's also how it was before as well.
---
 t/t5560-http-backend.sh |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
index 04a9896..0e4dc4b 100755
--- a/t/t5560-http-backend.sh
+++ b/t/t5560-http-backend.sh
@@ -162,15 +162,15 @@ test_expect_success 'http.receivepack false' '
 run_backend() {
 	REQUEST_METHOD=GET \
 	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$2" \
+	PATH_INFO="$1" \
 	git http-backend >act.out 2>act.err
 }
 
-path_info() {
+expect_aliased() {
 	if test $1 = 0; then
 		run_backend "$2"
 	else
-		test_must_fail run_backend "$2" &&
+		run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
@@ -179,15 +179,14 @@ path_info() {
 test_expect_success 'http-backend blocks bad PATH_INFO' '
 	config http.getanyfile true &&
 
-	run_backend 0 /repo.git/HEAD &&
+	expect_aliased 0 /repo.git/HEAD &&
 
-	run_backend 1 /repo.git/../HEAD &&
-	run_backend 1 /../etc/passwd &&
-	run_backend 1 ../etc/passwd &&
-	run_backend 1 /etc//passwd &&
-	run_backend 1 /etc/./passwd &&
-	run_backend 1 /etc/.../passwd &&
-	run_backend 1 //domain/data.txt
+	expect_aliased 1 /repo.git/../HEAD &&
+	expect_aliased 1 /../etc/passwd &&
+	expect_aliased 1 ../etc/passwd &&
+	expect_aliased 1 /etc//passwd &&
+	expect_aliased 1 /etc/./passwd &&
+	expect_aliased 1 //domain/data.txt
 '
 
 cat >exp <<EOF
-- 
1.6.6.236.gc56f3

[PATCH v4 2/3] Smart-http tests: Break test t5560-http-backend into pieces

[Tarmigan Casebolt]

This should introduce no functional change in the tests or the amount
of test coverage.

Acked-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/t5560-http-backend-noserver.sh |   52 +++++++
 t/t5560-http-backend.sh          |  294 --------------------------------------
 t/t5561-http-backend.sh          |  149 +++++++++++++++++++
 t/t556x_common                   |  119 +++++++++++++++
 4 files changed, 320 insertions(+), 294 deletions(-)
 create mode 100755 t/t5560-http-backend-noserver.sh
 delete mode 100755 t/t5560-http-backend.sh
 create mode 100755 t/t5561-http-backend.sh
 create mode 100755 t/t556x_common

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
new file mode 100755
index 0000000..a9ba2d9
--- /dev/null
+++ b/t/t5560-http-backend-noserver.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='test git-http-backend-noserver'
+. ./test-lib.sh
+
+HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
+
+run_backend() {
+	REQUEST_METHOD=GET \
+	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
+	PATH_INFO="$1" \
+	git http-backend >act.out 2>act.err
+}
+
+GET() {
+	return 0
+}
+
+POST() {
+	return 0
+}
+
+log_div() {
+	return 0
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+expect_aliased() {
+	if test $1 = 0; then
+		run_backend "$2"
+	else
+		run_backend "$2" &&
+		echo "fatal: '$2': aliased" >exp.err &&
+		test_cmp exp.err act.err
+	fi
+}
+
+test_expect_success 'http-backend blocks bad PATH_INFO' '
+	config http.getanyfile true &&
+
+	expect_aliased 0 /repo.git/HEAD &&
+
+	expect_aliased 1 /repo.git/../HEAD &&
+	expect_aliased 1 /../etc/passwd &&
+	expect_aliased 1 ../etc/passwd &&
+	expect_aliased 1 /etc//passwd &&
+	expect_aliased 1 /etc/./passwd &&
+	expect_aliased 1 //domain/data.txt
+'
+
+test_done
diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh
deleted file mode 100755
index 0e4dc4b..0000000
--- a/t/t5560-http-backend.sh
+++ /dev/null
@@ -1,294 +0,0 @@
-#!/bin/sh
-
-test_description='test git-http-backend'
-. ./test-lib.sh
-
-if test -n "$NO_CURL"; then
-	say 'skipping test, git built without http support'
-	test_done
-fi
-
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5560'}
-. "$TEST_DIRECTORY"/lib-httpd.sh
-start_httpd
-
-find_file() {
-	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	find $1 -type f |
-	sed -e 1q
-}
-
-config() {
-	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
-}
-
-GET() {
-	curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
-	tr '\015' Q <out |
-	sed '
-		s/Q$//
-		1q
-	' >act &&
-	echo "HTTP/1.1 $2" >exp &&
-	test_cmp exp act
-}
-
-POST() {
-	curl --include --data "$2" \
-	--header "Content-Type: application/x-$1-request" \
-	"$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
-	tr '\015' Q <out |
-	sed '
-		s/Q$//
-		1q
-	' >act &&
-	echo "HTTP/1.1 $3" >exp &&
-	test_cmp exp act
-}
-
-log_div() {
-	echo >>"$HTTPD_ROOT_PATH"/access.log
-	echo "###  $1" >>"$HTTPD_ROOT_PATH"/access.log
-	echo "###" >>"$HTTPD_ROOT_PATH"/access.log
-}
-
-test_expect_success 'setup repository' '
-	echo content >file &&
-	git add file &&
-	git commit -m one &&
-
-	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git --bare init &&
-	 : >objects/info/alternates &&
-	 : >objects/info/http-alternates
-	) &&
-	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	git push public master:master &&
-
-	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 git repack -a -d
-	) &&
-
-	echo other >file &&
-	git add file &&
-	git commit -m two &&
-	git push public master:master &&
-
-	LOOSE_URL=$(find_file objects/??) &&
-	PACK_URL=$(find_file objects/pack/*.pack) &&
-	IDX_URL=$(find_file objects/pack/*.idx)
-'
-
-get_static_files() {
-	GET HEAD "$1" &&
-	GET info/refs "$1" &&
-	GET objects/info/packs "$1" &&
-	GET objects/info/alternates "$1" &&
-	GET objects/info/http-alternates "$1" &&
-	GET $LOOSE_URL "$1" &&
-	GET $PACK_URL "$1" &&
-	GET $IDX_URL "$1"
-}
-
-SMART=smart
-test_expect_success 'direct refs/heads/master not found' '
-	log_div "refs/heads/master"
-	GET refs/heads/master "404 Not Found"
-'
-test_expect_success 'static file is ok' '
-	log_div "getanyfile default"
-	get_static_files "200 OK"
-'
-SMART=smart_noexport
-test_expect_success 'no export by default' '
-	log_div "no git-daemon-export-ok"
-	get_static_files "404 Not Found"
-'
-test_expect_success 'export if git-daemon-export-ok' '
-	log_div "git-daemon-export-ok"
-        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-	 touch git-daemon-export-ok
-	) &&
-        get_static_files "200 OK"
-'
-SMART=smart
-test_expect_success 'static file if http.getanyfile true is ok' '
-	log_div "getanyfile true"
-	config http.getanyfile true &&
-	get_static_files "200 OK"
-'
-test_expect_success 'static file if http.getanyfile false fails' '
-	log_div "getanyfile false"
-	config http.getanyfile false &&
-	get_static_files "403 Forbidden"
-'
-
-test_expect_success 'http.uploadpack default enabled' '
-	log_div "uploadpack default"
-	GET info/refs?service=git-upload-pack "200 OK"  &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack true' '
-	log_div "uploadpack true"
-	config http.uploadpack true &&
-	GET info/refs?service=git-upload-pack "200 OK" &&
-	POST git-upload-pack 0000 "200 OK"
-'
-test_expect_success 'http.uploadpack false' '
-	log_div "uploadpack false"
-	config http.uploadpack false &&
-	GET info/refs?service=git-upload-pack "403 Forbidden" &&
-	POST git-upload-pack 0000 "403 Forbidden"
-'
-
-test_expect_success 'http.receivepack default disabled' '
-	log_div "receivepack default"
-	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-test_expect_success 'http.receivepack true' '
-	log_div "receivepack true"
-	config http.receivepack true &&
-	GET info/refs?service=git-receive-pack "200 OK" &&
-	POST git-receive-pack 0000 "200 OK"
-'
-test_expect_success 'http.receivepack false' '
-	log_div "receivepack false"
-	config http.receivepack false &&
-	GET info/refs?service=git-receive-pack "403 Forbidden" &&
-	POST git-receive-pack 0000 "403 Forbidden"
-'
-run_backend() {
-	REQUEST_METHOD=GET \
-	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$1" \
-	git http-backend >act.out 2>act.err
-}
-
-expect_aliased() {
-	if test $1 = 0; then
-		run_backend "$2"
-	else
-		run_backend "$2" &&
-		echo "fatal: '$2': aliased" >exp.err &&
-		test_cmp exp.err act.err
-	fi
-}
-
-test_expect_success 'http-backend blocks bad PATH_INFO' '
-	config http.getanyfile true &&
-
-	expect_aliased 0 /repo.git/HEAD &&
-
-	expect_aliased 1 /repo.git/../HEAD &&
-	expect_aliased 1 /../etc/passwd &&
-	expect_aliased 1 ../etc/passwd &&
-	expect_aliased 1 /etc//passwd &&
-	expect_aliased 1 /etc/./passwd &&
-	expect_aliased 1 //domain/data.txt
-'
-
-cat >exp <<EOF
-
-###  refs/heads/master
-###
-GET  /smart/repo.git/refs/heads/master HTTP/1.1 404 -
-
-###  getanyfile default
-###
-GET  /smart/repo.git/HEAD HTTP/1.1 200
-GET  /smart/repo.git/info/refs HTTP/1.1 200
-GET  /smart/repo.git/objects/info/packs HTTP/1.1 200
-GET  /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
-GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
-GET  /smart/repo.git/$LOOSE_URL HTTP/1.1 200
-GET  /smart/repo.git/$PACK_URL HTTP/1.1 200
-GET  /smart/repo.git/$IDX_URL HTTP/1.1 200
-
-###  no git-daemon-export-ok
-###
-GET  /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 -
-GET  /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 -
-
-###  git-daemon-export-ok
-###
-GET  /smart_noexport/repo.git/HEAD HTTP/1.1 200
-GET  /smart_noexport/repo.git/info/refs HTTP/1.1 200
-GET  /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
-GET  /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
-GET  /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
-GET  /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
-GET  /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
-GET  /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
-
-###  getanyfile true
-###
-GET  /smart/repo.git/HEAD HTTP/1.1 200
-GET  /smart/repo.git/info/refs HTTP/1.1 200
-GET  /smart/repo.git/objects/info/packs HTTP/1.1 200
-GET  /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
-GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
-GET  /smart/repo.git/$LOOSE_URL HTTP/1.1 200
-GET  /smart/repo.git/$PACK_URL HTTP/1.1 200
-GET  /smart/repo.git/$IDX_URL HTTP/1.1 200
-
-###  getanyfile false
-###
-GET  /smart/repo.git/HEAD HTTP/1.1 403 -
-GET  /smart/repo.git/info/refs HTTP/1.1 403 -
-GET  /smart/repo.git/objects/info/packs HTTP/1.1 403 -
-GET  /smart/repo.git/objects/info/alternates HTTP/1.1 403 -
-GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 403 -
-GET  /smart/repo.git/$LOOSE_URL HTTP/1.1 403 -
-GET  /smart/repo.git/$PACK_URL HTTP/1.1 403 -
-GET  /smart/repo.git/$IDX_URL HTTP/1.1 403 -
-
-###  uploadpack default
-###
-GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
-POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
-
-###  uploadpack true
-###
-GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
-POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
-
-###  uploadpack false
-###
-GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 403 -
-POST /smart/repo.git/git-upload-pack HTTP/1.1 403 -
-
-###  receivepack default
-###
-GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
-POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
-
-###  receivepack true
-###
-GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
-POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
-
-###  receivepack false
-###
-GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
-POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
-EOF
-test_expect_success 'server request log matches test results' '
-	sed -e "
-		s/^.* \"//
-		s/\"//
-		s/ [1-9][0-9]*\$//
-		s/^GET /GET  /
-	" >act <"$HTTPD_ROOT_PATH"/access.log &&
-	test_cmp exp act
-'
-
-stop_httpd
-test_done
diff --git a/t/t5561-http-backend.sh b/t/t5561-http-backend.sh
new file mode 100755
index 0000000..8c6d0b2
--- /dev/null
+++ b/t/t5561-http-backend.sh
@@ -0,0 +1,149 @@
+#!/bin/sh
+
+test_description='test git-http-backend'
+. ./test-lib.sh
+
+if test -n "$NO_CURL"; then
+	say 'skipping test, git built without http support'
+	test_done
+fi
+
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5561'}
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+GET() {
+	curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null &&
+	tr '\015' Q <out |
+	sed '
+		s/Q$//
+		1q
+	' >act &&
+	echo "HTTP/1.1 $2" >exp &&
+	test_cmp exp act
+}
+
+POST() {
+	curl --include --data "$2" \
+	--header "Content-Type: application/x-$1-request" \
+	"$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null &&
+	tr '\015' Q <out |
+	sed '
+		s/Q$//
+		1q
+	' >act &&
+	echo "HTTP/1.1 $3" >exp &&
+	test_cmp exp act
+}
+
+log_div() {
+	echo >>"$HTTPD_ROOT_PATH"/access.log
+	echo "###  $1" >>"$HTTPD_ROOT_PATH"/access.log
+	echo "###" >>"$HTTPD_ROOT_PATH"/access.log
+}
+
+. "$TEST_DIRECTORY"/t556x_common
+
+cat >exp <<EOF
+
+###  refs/heads/master
+###
+GET  /smart/repo.git/refs/heads/master HTTP/1.1 404 -
+
+###  getanyfile default
+###
+GET  /smart/repo.git/HEAD HTTP/1.1 200
+GET  /smart/repo.git/info/refs HTTP/1.1 200
+GET  /smart/repo.git/objects/info/packs HTTP/1.1 200
+GET  /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET  /smart/repo.git/$LOOSE_URL HTTP/1.1 200
+GET  /smart/repo.git/$PACK_URL HTTP/1.1 200
+GET  /smart/repo.git/$IDX_URL HTTP/1.1 200
+
+###  no git-daemon-export-ok
+###
+GET  /smart_noexport/repo.git/HEAD HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/info/refs HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 -
+GET  /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 -
+
+###  git-daemon-export-ok
+###
+GET  /smart_noexport/repo.git/HEAD HTTP/1.1 200
+GET  /smart_noexport/repo.git/info/refs HTTP/1.1 200
+GET  /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200
+GET  /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET  /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET  /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200
+GET  /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200
+GET  /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200
+
+###  getanyfile true
+###
+GET  /smart/repo.git/HEAD HTTP/1.1 200
+GET  /smart/repo.git/info/refs HTTP/1.1 200
+GET  /smart/repo.git/objects/info/packs HTTP/1.1 200
+GET  /smart/repo.git/objects/info/alternates HTTP/1.1 200 -
+GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 200 -
+GET  /smart/repo.git/$LOOSE_URL HTTP/1.1 200
+GET  /smart/repo.git/$PACK_URL HTTP/1.1 200
+GET  /smart/repo.git/$IDX_URL HTTP/1.1 200
+
+###  getanyfile false
+###
+GET  /smart/repo.git/HEAD HTTP/1.1 403 -
+GET  /smart/repo.git/info/refs HTTP/1.1 403 -
+GET  /smart/repo.git/objects/info/packs HTTP/1.1 403 -
+GET  /smart/repo.git/objects/info/alternates HTTP/1.1 403 -
+GET  /smart/repo.git/objects/info/http-alternates HTTP/1.1 403 -
+GET  /smart/repo.git/$LOOSE_URL HTTP/1.1 403 -
+GET  /smart/repo.git/$PACK_URL HTTP/1.1 403 -
+GET  /smart/repo.git/$IDX_URL HTTP/1.1 403 -
+
+###  uploadpack default
+###
+GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
+POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
+
+###  uploadpack true
+###
+GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 200
+POST /smart/repo.git/git-upload-pack HTTP/1.1 200 -
+
+###  uploadpack false
+###
+GET  /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 403 -
+POST /smart/repo.git/git-upload-pack HTTP/1.1 403 -
+
+###  receivepack default
+###
+GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
+POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
+
+###  receivepack true
+###
+GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 200
+POST /smart/repo.git/git-receive-pack HTTP/1.1 200 -
+
+###  receivepack false
+###
+GET  /smart/repo.git/info/refs?service=git-receive-pack HTTP/1.1 403 -
+POST /smart/repo.git/git-receive-pack HTTP/1.1 403 -
+EOF
+test_expect_success 'server request log matches test results' '
+	sed -e "
+		s/^.* \"//
+		s/\"//
+		s/ [1-9][0-9]*\$//
+		s/^GET /GET  /
+	" >act <"$HTTPD_ROOT_PATH"/access.log &&
+	test_cmp exp act
+'
+
+stop_httpd
+test_done
diff --git a/t/t556x_common b/t/t556x_common
new file mode 100755
index 0000000..1b4921c
--- /dev/null
+++ b/t/t556x_common
@@ -0,0 +1,119 @@
+#!/bin/sh
+
+find_file() {
+	cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	find $1 -type f |
+	sed -e 1q
+}
+
+config() {
+	git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config $1 $2
+}
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one &&
+
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 : >objects/info/alternates &&
+	 : >objects/info/http-alternates
+	) &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master &&
+
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git repack -a -d
+	) &&
+
+	echo other >file &&
+	git add file &&
+	git commit -m two &&
+	git push public master:master &&
+
+	LOOSE_URL=$(find_file objects/??) &&
+	PACK_URL=$(find_file objects/pack/*.pack) &&
+	IDX_URL=$(find_file objects/pack/*.idx)
+'
+
+get_static_files() {
+	GET HEAD "$1" &&
+	GET info/refs "$1" &&
+	GET objects/info/packs "$1" &&
+	GET objects/info/alternates "$1" &&
+	GET objects/info/http-alternates "$1" &&
+	GET $LOOSE_URL "$1" &&
+	GET $PACK_URL "$1" &&
+	GET $IDX_URL "$1"
+}
+
+SMART=smart
+test_expect_success 'direct refs/heads/master not found' '
+	log_div "refs/heads/master"
+	GET refs/heads/master "404 Not Found"
+'
+test_expect_success 'static file is ok' '
+	log_div "getanyfile default"
+	get_static_files "200 OK"
+'
+SMART=smart_noexport
+test_expect_success 'no export by default' '
+	log_div "no git-daemon-export-ok"
+	get_static_files "404 Not Found"
+'
+test_expect_success 'export if git-daemon-export-ok' '
+	log_div "git-daemon-export-ok"
+        (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 touch git-daemon-export-ok
+	) &&
+        get_static_files "200 OK"
+'
+SMART=smart
+test_expect_success 'static file if http.getanyfile true is ok' '
+	log_div "getanyfile true"
+	config http.getanyfile true &&
+	get_static_files "200 OK"
+'
+test_expect_success 'static file if http.getanyfile false fails' '
+	log_div "getanyfile false"
+	config http.getanyfile false &&
+	get_static_files "403 Forbidden"
+'
+
+test_expect_success 'http.uploadpack default enabled' '
+	log_div "uploadpack default"
+	GET info/refs?service=git-upload-pack "200 OK"  &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack true' '
+	log_div "uploadpack true"
+	config http.uploadpack true &&
+	GET info/refs?service=git-upload-pack "200 OK" &&
+	POST git-upload-pack 0000 "200 OK"
+'
+test_expect_success 'http.uploadpack false' '
+	log_div "uploadpack false"
+	config http.uploadpack false &&
+	GET info/refs?service=git-upload-pack "403 Forbidden" &&
+	POST git-upload-pack 0000 "403 Forbidden"
+'
+
+test_expect_success 'http.receivepack default disabled' '
+	log_div "receivepack default"
+	GET info/refs?service=git-receive-pack "403 Forbidden"  &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
+test_expect_success 'http.receivepack true' '
+	log_div "receivepack true"
+	config http.receivepack true &&
+	GET info/refs?service=git-receive-pack "200 OK" &&
+	POST git-receive-pack 0000 "200 OK"
+'
+test_expect_success 'http.receivepack false' '
+	log_div "receivepack false"
+	config http.receivepack false &&
+	GET info/refs?service=git-receive-pack "403 Forbidden" &&
+	POST git-receive-pack 0000 "403 Forbidden"
+'
-- 
1.6.6.236.gc56f3

[PATCH v4 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Tarmigan Casebolt]

This reuses many of the tests from the old t5560 but runs those tests
without curl or a webserver.  This will hopefully increase the testing
coverage for http-backend because it does not require users to set
GIT_TEST_HTTPD.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
As a side note, I am very impressed that git rebase was able to
apply this patch properly even after I renamed the tests earlier in
the series.
---
 t/t5560-http-backend-noserver.sh |   30 ++++++++++++++++++++++++------
 t/t556x_common                   |    3 +++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index a9ba2d9..9013824 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -6,18 +6,36 @@ test_description='test git-http-backend-noserver'
 HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
 
 run_backend() {
-	REQUEST_METHOD=GET \
+	echo "$2"| \
+	QUERY_STRING="${1#*\?}" \
 	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$1" \
+	PATH_INFO="${1%%\?*}" \
 	git http-backend >act.out 2>act.err
 }
 
 GET() {
-	return 0
+	REQUEST_METHOD="GET" \
+	run_backend "/repo.git/$1" &&
+	grep "Status" act.out >act
+	if [ $? -eq 1 ];
+	then
+		printf "Status: 200 OK\r\n" > act
+	fi
+	printf "Status: $2\r\n" > exp &&
+	test_cmp exp act
 }
 
 POST() {
-	return 0
+	REQUEST_METHOD="POST" \
+	CONTENT_TYPE="application/x-$1-request" \
+	run_backend "/repo.git/$1" "$2" &&
+	grep "Status" act.out >act
+	if [ $? -eq 1 ];
+	then
+		printf "Status: 200 OK\r\n" > act
+	fi
+	printf "Status: $3\r\n" > exp &&
+	test_cmp exp act
 }
 
 log_div() {
@@ -28,9 +46,9 @@ log_div() {
 
 expect_aliased() {
 	if test $1 = 0; then
-		run_backend "$2"
+		REQUEST_METHOD=GET run_backend "$2"
 	else
-		run_backend "$2" &&
+		REQUEST_METHOD=GET run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
diff --git a/t/t556x_common b/t/t556x_common
index 1b4921c..be024e5 100755
--- a/t/t556x_common
+++ b/t/t556x_common
@@ -50,6 +50,7 @@ get_static_files() {
 }
 
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'direct refs/heads/master not found' '
 	log_div "refs/heads/master"
 	GET refs/heads/master "404 Not Found"
@@ -59,6 +60,7 @@ test_expect_success 'static file is ok' '
 	get_static_files "200 OK"
 '
 SMART=smart_noexport
+unset GIT_HTTP_EXPORT_ALL
 test_expect_success 'no export by default' '
 	log_div "no git-daemon-export-ok"
 	get_static_files "404 Not Found"
@@ -71,6 +73,7 @@ test_expect_success 'export if git-daemon-export-ok' '
         get_static_files "200 OK"
 '
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'static file if http.getanyfile true is ok' '
 	log_div "getanyfile true"
 	config http.getanyfile true &&
-- 
1.6.6.236.gc56f3

Re: [PATCH v3 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Shawn O. Pearce]

Tarmigan <tarmigan+git@gmail.com> wrote:
> On Sat, Jan 2, 2010 at 1:03 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:
> >> ?GET() {
> >> - ? ? return 0
> >> + ? ? REQUEST_METHOD="GET" \
> >> + ? ? run_backend "/repo.git/$1" &&
> >> + ? ? grep "Status" act.out >act
> >> + ? ? if [ $? -eq 1 ];
> >
> > I think this should be spelled as:
> >
> > ? ? ? ?if ! grep "Status" act.out >act; then
> > ? ? ? ? ? ? ? ?printf "Status: 200 OK\r\n" > act
> > ? ? ? ?fi
> 
> OK.  I had figured the "1" would mean "no matches not found"  rather
> than some other error like "file does not exist".  Not sure how
> portable that error value convention is for greps though, so I've
> changed it as you suggested.

Your v3 series still shows it the old way...

-- 
Shawn.

[PATCH v4 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Tarmigan Casebolt]

This reuses many of the tests from the old t5560 but runs those tests
without curl or a webserver.  This will hopefully increase the testing
coverage for http-backend because it does not require users to set
GIT_TEST_HTTPD.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Oops forgot to commit the grep changes.

Only resending 3/3 of the series.
---
 t/t5560-http-backend-noserver.sh |   26 ++++++++++++++++++++------
 t/t556x_common                   |    3 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index a9ba2d9..a63d3ec 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -6,18 +6,32 @@ test_description='test git-http-backend-noserver'
 HTTPD_DOCUMENT_ROOT_PATH="$TRASH_DIRECTORY"
 
 run_backend() {
-	REQUEST_METHOD=GET \
+	echo "$2"| \
+	QUERY_STRING="${1#*\?}" \
 	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
-	PATH_INFO="$1" \
+	PATH_INFO="${1%%\?*}" \
 	git http-backend >act.out 2>act.err
 }
 
 GET() {
-	return 0
+	REQUEST_METHOD="GET" \
+	run_backend "/repo.git/$1" &&
+	if ! grep "Status" act.out >act; then
+		printf "Status: 200 OK\r\n" > act
+	fi
+	printf "Status: $2\r\n" > exp &&
+	test_cmp exp act
 }
 
 POST() {
-	return 0
+	REQUEST_METHOD="POST" \
+	CONTENT_TYPE="application/x-$1-request" \
+	run_backend "/repo.git/$1" "$2" &&
+	if ! grep "Status" act.out >act; then
+		printf "Status: 200 OK\r\n" > act
+	fi
+	printf "Status: $3\r\n" > exp &&
+	test_cmp exp act
 }
 
 log_div() {
@@ -28,9 +42,9 @@ log_div() {
 
 expect_aliased() {
 	if test $1 = 0; then
-		run_backend "$2"
+		REQUEST_METHOD=GET run_backend "$2"
 	else
-		run_backend "$2" &&
+		REQUEST_METHOD=GET run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
diff --git a/t/t556x_common b/t/t556x_common
index 1b4921c..be024e5 100755
--- a/t/t556x_common
+++ b/t/t556x_common
@@ -50,6 +50,7 @@ get_static_files() {
 }
 
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'direct refs/heads/master not found' '
 	log_div "refs/heads/master"
 	GET refs/heads/master "404 Not Found"
@@ -59,6 +60,7 @@ test_expect_success 'static file is ok' '
 	get_static_files "200 OK"
 '
 SMART=smart_noexport
+unset GIT_HTTP_EXPORT_ALL
 test_expect_success 'no export by default' '
 	log_div "no git-daemon-export-ok"
 	get_static_files "404 Not Found"
@@ -71,6 +73,7 @@ test_expect_success 'export if git-daemon-export-ok' '
         get_static_files "200 OK"
 '
 SMART=smart
+export GIT_HTTP_EXPORT_ALL=1
 test_expect_success 'static file if http.getanyfile true is ok' '
 	log_div "getanyfile true"
 	config http.getanyfile true &&
-- 
1.6.6.236.gc56f3

Re: [PATCH v4 3/3] Smart-http tests: Test http-backend without curl or a webserver

[Michael Haggerty]

Tarmigan Casebolt wrote:
> This reuses many of the tests from the old t5560 but runs those tests
> without curl or a webserver.  This will hopefully increase the testing
> coverage for http-backend because it does not require users to set
> GIT_TEST_HTTPD.
> 
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>

I'm not sure which version of this patch is currently committed, but in
any case test t/t5560-http-backend-noserver.sh is broken for me in
"next".  According to git-bisect, the guilty commit is

fd0a8c2e6428acb883bf4707de54b3e026c57455 'Smart-http tests: Test
http-backend without curl or a webserver'

I'm not familiar with this part of the code and I haven't done any
special setup (e.g., I have not set GIT_TEST_HTTPD, whatever that is).
Simply checking out, "make"ing, then running
t5560-http-backend-noserver.sh results in 13/14 failures starting with
test 2:

> 
> * expecting success: 
> 	log_div "refs/heads/master"
> 	GET refs/heads/master "404 Not Found"
> 
> --- exp	2010-01-14 05:21:34.000000000 +0000
> +++ act	2010-01-14 05:21:34.000000000 +0000
> @@ -1 +1 @@
> -Status: 404 Not Found
> +Status: 500 Internal Server Error
> * FAIL 2: direct refs/heads/master not found

This failure is on Ubuntu Linux 8.04 "hardy".

Please let me know if I am doing something wrong or if you would like
any more information.

Michael

[PATCH] Test t5560: Fix test when run with dash

[Tarmigan Casebolt]

dash is more finicky than some shells and this change seems to make it
happier.

Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>

---
Michael, thanks for the report.

Ubuntu's /bin/sh is dash, which I had not tested with.  Installing
dash on my machine, I was able to reproduce and this patch fixes the
problem for me.

Could you please see if this works for you?

Thanks,
Tarmigan
---
 t/t5560-http-backend-noserver.sh |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index 5f8c88e..f2e413d 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -10,12 +10,13 @@ run_backend() {
 	QUERY_STRING="${1#*\?}" \
 	GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \
 	PATH_INFO="${1%%\?*}" \
+	REQUEST_METHOD="$3" \
+	CONTENT_TYPE="$4" \
 	git http-backend >act.out 2>act.err
 }
 
 GET() {
-	REQUEST_METHOD="GET" \
-	run_backend "/repo.git/$1" &&
+	run_backend "/repo.git/$1" "" "GET" &&
 	if ! grep "Status" act.out >act
 	then
 		printf "Status: 200 OK\r\n" >act
@@ -25,9 +26,7 @@ GET() {
 }
 
 POST() {
-	REQUEST_METHOD="POST" \
-	CONTENT_TYPE="application/x-$1-request" \
-	run_backend "/repo.git/$1" "$2" &&
+	run_backend "/repo.git/$1" "$2" "POST" "application/x-$1-request" &&
 	if ! grep "Status" act.out >act
 	then
 		printf "Status: 200 OK\r\n" >act
-- 
1.6.6.376.gabe8e

Re: [PATCH] Test t5560: Fix test when run with dash

[Michael Haggerty]

Tarmigan Casebolt wrote:
> dash is more finicky than some shells and this change seems to make it
> happier.
> 
> Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
> 
> ---
> Michael, thanks for the report.
> 
> Ubuntu's /bin/sh is dash, which I had not tested with.  Installing
> dash on my machine, I was able to reproduce and this patch fixes the
> problem for me.
> 
> Could you please see if this works for you?
> [...]

Yes, that fixes it.  Thanks!

Michael

Re: [PATCH] Test t5560: Fix test when run with dash

[Junio C Hamano]

Tarmigan Casebolt <tarmigan+git@gmail.com> writes:

> dash is more finicky than some shells and this change seems to make it
> happier.

"Finicky" won't help other people to learn what to avoid making the same
mistake in the future.  Please spell the problem out more explicitly.

I think the issue is that some shells do not like the "Run this command
(and only this command) under these environment variable settings"

	VAR1=VAL1 VAR2=VAL2 command

if "command" is a shell function.

Does that match your understanding of the problem?

[PATCH v2] Test t5560: Fix test when run with dash

[Tarmigan Casebolt]

The dash shell is more finicky than some others.

In particular, it does not seem to like the pattern of setting an
environment variable on the same line as you call a shell function
like this:

        REQUEST_METHOD="GET" some_shell_function

as you might use to set a variable only for one command if that
command were an executable or a shell builtin.

Reported-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Junio, that description matches my understanding of the problem.
I can't tell from my reading of the POSIX spec whether my usage was
wrong or if dash is wrong, which is why I shied away from an
explanation.  As a practical matter though, this patch does fix the
issue.

This version takes a slighty different approach that I think leaves
things clearer and doesn't pass in tons of arguements to the shell
function.  If you prefer the old approach, I can send a patch that way
instead.
---
 t/t5560-http-backend-noserver.sh |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index 5f8c88e..44885b8 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -14,8 +14,9 @@ run_backend() {
 }
 
 GET() {
-	REQUEST_METHOD="GET" \
+	export REQUEST_METHOD="GET" &&
 	run_backend "/repo.git/$1" &&
+	unset REQUEST_METHOD &&
 	if ! grep "Status" act.out >act
 	then
 		printf "Status: 200 OK\r\n" >act
@@ -25,9 +26,11 @@ GET() {
 }
 
 POST() {
-	REQUEST_METHOD="POST" \
-	CONTENT_TYPE="application/x-$1-request" \
+	export REQUEST_METHOD="POST" &&
+	export CONTENT_TYPE="application/x-$1-request" &&
 	run_backend "/repo.git/$1" "$2" &&
+	unset REQUEST_METHOD &&
+	unset CONTENT_TYPE &&
 	if ! grep "Status" act.out >act
 	then
 		printf "Status: 200 OK\r\n" >act
@@ -43,13 +46,15 @@ log_div() {
 . "$TEST_DIRECTORY"/t556x_common
 
 expect_aliased() {
+	export REQUEST_METHOD="GET" &&
 	if test $1 = 0; then
-		REQUEST_METHOD=GET run_backend "$2"
+		run_backend "$2"
 	else
-		REQUEST_METHOD=GET run_backend "$2" &&
+		run_backend "$2" &&
 		echo "fatal: '$2': aliased" >exp.err &&
 		test_cmp exp.err act.err
 	fi
+	unset REQUEST_METHOD
 }
 
 test_expect_success 'http-backend blocks bad PATH_INFO' '
-- 
1.6.6

Re: [PATCH v2] Test t5560: Fix test when run with dash

[Johannes Sixt]

Tarmigan Casebolt schrieb:
>         REQUEST_METHOD="GET" some_shell_function

> I can't tell from my reading of the POSIX spec whether my usage was
> wrong or if dash is wrong,

According to POSIX, variables set as shown above for shell functions are
not exported and retain their value after the function returns. I would
not be surprised if dash got this right, and the tests fail because they
were written for bash, which gets it wrong.

In particular,

f() { sh -c 'echo +$v+'; }
v=x f
echo +$v+
sh -c 'echo +$v+'"

Should print

++
+x+
++

I can test only ash, bash, zsh, and ksh, of which only ksh gets it right.

-- Hannes

Re: [PATCH v2] Test t5560: Fix test when run with dash

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Tarmigan Casebolt schrieb:
>>         REQUEST_METHOD="GET" some_shell_function
>
>> I can't tell from my reading of the POSIX spec whether my usage was
>> wrong or if dash is wrong,
>
> According to POSIX, variables set as shown above for shell functions are
> not exported and retain their value after the function returns.

I actually looked for this yesterday, but didn't find a relevant
definition.  But "2.9.5 Function Definition Command" [*1*] seems to
address the issue: "When a function is executed, it shall have the
syntax-error and variable-assignment properties described for special
built-in utilities...".

And "2.14 Special Built-in Utilities" section [*2*] says "2. Variable
assignments specified with special built-in utilities remain in effect
after the built-in completes...".  Taking both together, it seems that
the assignment should be in effect after the function returns.

Does my reading match yours, or do you have more definitive descriptions
you can point at in POSIX.1, so that the log message can be improved to
help people avoid this issue in the future?

Yesterday, I saw rebase--interactive has a few codepaths where "output"
shell function was used with the single-shot export; perhaps they need to
also be fixed.

[References]

*1* http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_05
*2* http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_14

Re: [PATCH v2] Test t5560: Fix test when run with dash

[Johannes Sixt]

On Freitag, 15. Januar 2010, Junio C Hamano wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> > Tarmigan Casebolt schrieb:
> >>         REQUEST_METHOD="GET" some_shell_function
> >>
> >> I can't tell from my reading of the POSIX spec whether my usage was
> >> wrong or if dash is wrong,
> >
> > According to POSIX, variables set as shown above for shell functions are
> > not exported and retain their value after the function returns.
>
> I actually looked for this yesterday, but didn't find a relevant
> definition.  But "2.9.5 Function Definition Command" [*1*] seems to
> address the issue: "When a function is executed, it shall have the
> syntax-error and variable-assignment properties described for special
> built-in utilities...".
>
> And "2.14 Special Built-in Utilities" section [*2*] says "2. Variable
> assignments specified with special built-in utilities remain in effect
> after the built-in completes...".  Taking both together, it seems that
> the assignment should be in effect after the function returns.
>
> Does my reading match yours,

These are exactly the definitions that I meant. The statement that variables 
are not exported is in "2.9.1 Simple Commands" 
http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01

"[If there is a command name], the variable assignments shall be exported for 
the execution environment of the command and shall not affect the current 
execution environment (except for special built-ins)."

> Yesterday, I saw rebase--interactive has a few codepaths where "output"
> shell function was used with the single-shot export; perhaps they need to
> also be fixed.

I knew these spots, and they were discussed when that code was introduced. 
Before I sent out the mail you were responding to, I tried various ways to 
show the failure in rebase--interactive, but it didn't fail...

-- Hannes

Re: [PATCH v2] Test t5560: Fix test when run with dash

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> These are exactly the definitions that I meant. The statement that variables 
> are not exported is in "2.9.1 Simple Commands" 
> http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01
>
> "[If there is a command name], the variable assignments shall be exported for 
> the execution environment of the command and shall not affect the current 
> execution environment (except for special built-ins)."

Note the "except for special built-ins".  I was unsure yesterday because
it does not say "except for special built-ins and shell functions".

Re: [PATCH v2] Test t5560: Fix test when run with dash

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

>> Yesterday, I saw rebase--interactive has a few codepaths where "output"
>> shell function was used with the single-shot export; perhaps they need to
>> also be fixed.
>
> I knew these spots, and they were discussed when that code was introduced. 
> Before I sent out the mail you were responding to, I tried various ways to 
> show the failure in rebase--interactive, but it didn't fail...

It may be the case that the single-shot-ness of these GIT_AUTHOR_NAME
exports do not matter at all in that program, even though the original
versions may have been written carefully not to leak the value suitable
for the current commit to later rounds.

I think the recent updates from Michael actually depends on the
distinction not to matter.  For example, do_with_author() in 7756ecf
(rebase -i: Extract function do_with_author, 2010-01-14) invokes "$@"
that could be a shell function.

Re: [PATCH v2] Test t5560: Fix test when run with dash

[Michael Haggerty]

Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
>>> Yesterday, I saw rebase--interactive has a few codepaths where "output"
>>> shell function was used with the single-shot export; perhaps they need to
>>> also be fixed.
>> I knew these spots, and they were discussed when that code was introduced. 
>> Before I sent out the mail you were responding to, I tried various ways to 
>> show the failure in rebase--interactive, but it didn't fail...
> 
> It may be the case that the single-shot-ness of these GIT_AUTHOR_NAME
> exports do not matter at all in that program, even though the original
> versions may have been written carefully not to leak the value suitable
> for the current commit to later rounds.
> 
> I think the recent updates from Michael actually depends on the
> distinction not to matter.  For example, do_with_author() in 7756ecf
> (rebase -i: Extract function do_with_author, 2010-01-14) invokes "$@"
> that could be a shell function.

I have to say that I am a little bit over my head here.  I didn't try to
follow the complete data path of the GIT_AUTHOR_* shell variables, nor
do I know exactly what git commands they affect.  I just tried to
locally refactor the code based on my mistaken assumption that shell
functions are treated much like external commands WRT export of shell
variables.

The use of the GIT_AUTHOR_* variables in git-rebase--interactive.sh were
and are a bit peculiar anyway, since the variables are already set
before do_with_author() is invoked, and the values are left to hang
around afterwards.  The do_with_author() function only tries to export
these already-set variables.

So I suppose that the simplest solution is to export these variables
explicitly in do_with_author(), something like this (similar to the
third code block that was replaced by the do_with_author() function):

do_with_author() {
	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE &&
	"$@"
}

But to ensure that this is a correct solution would require verification
that these now-exported variables don't cause unwanted side-effects
during any other external command invocations.  Alternatively, I suppose
that the variables could be exported within a subshell that also invokes
the "$@" command; this subshell could even source the $AUTHOR_SCRIPT
file if it were thought advantageous not to set the GIT_AUTHOR_*
variables in the git-rebase--interactive.sh script at all.

Help would be most appreciated; I probably won't have time to work on
this myself for a week or two.

Michael