git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/6] t5400: allow individual tests to fail

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 9, 2009, 21:39 UTC
Message-ID
<7vab8vthlj.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<7v4oz3v36a.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 9 quoted lines
> Jeff King <peff@peff.net> writes:
>
>> All minor things, of course, but while we're cleaning up... :)
>
> Sure.  This was made as a quick-fix to a mess others created, so I did not
> study them very deeply.
>
> Will reroll if I have the time but it is likely that I may be tending
> other topics first.
-- >8 --
Subject: [PATCH] Modernize t5400 test script

Many tests checked for failure by hand without using test_must_fail (they probably predate the shell function).

When we know the desired outcome, explicitly check for it, instead of checking if the result does not match one possible incorrect outcome. E.g. if you expect a push to be refused, you do not test if the result is different from what was pushed. Instead, make sure that the ref did not before and after the push.

The test sequence chdir'ed around and any failure at one point could have started the next test in an unexpected directory. Fix this problem by using subshells as necessary.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5400-send-pack.sh |  180 ++++++++++++++++++++++++++------------------------
 1 files changed, 94 insertions(+), 86 deletions(-)
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index b21317d..785c2f4 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -32,9 +32,7 @@ test_expect_success setup '
 	done &&
 	git update-ref HEAD "$commit" &&
 	git clone ./. victim &&
-	cd victim &&
-	git log &&
-	cd .. &&
+	( cd victim && git log ) &&
 	git update-ref HEAD "$zero" &&
 	parent=$zero &&
 	i=0 &&
@@ -59,88 +57,84 @@ test_expect_success 'pack the source repository' '
 '
 
 test_expect_success 'pack the destination repository' '
+    (
 	cd victim &&
 	git repack -a -d &&
-	git prune &&
-	cd ..
+	git prune
+    )
 '
 
-test_expect_success \
-        'pushing rewound head should not barf but require --force' '
-	# should not fail but refuse to update.
-	if git send-pack ./victim/.git/ master
-	then
-		# now it should fail with Pasky patch
-		echo >&2 Gaah, it should have failed.
-		false
-	else
-		echo >&2 Thanks, it correctly failed.
-		true
-	fi &&
-	if cmp victim/.git/refs/heads/master .git/refs/heads/master
-	then
-		# should have been left as it was!
-		false
-	else
-		true
-	fi &&
+test_expect_success 'refuse pushing rewound head without --force' '
+	pushed_head=$(git rev-parse --verify master) &&
+	victim_orig=$(cd victim && git rev-parse --verify master) &&
+	test_must_fail git send-pack ./victim master &&
+	victim_head=$(cd victim && git rev-parse --verify master) &&
+	test "$victim_head" = "$victim_orig" &&
 	# this should update
-	git send-pack --force ./victim/.git/ master &&
-	cmp victim/.git/refs/heads/master .git/refs/heads/master
+	git send-pack --force ./victim master &&
+	victim_head=$(cd victim && git rev-parse --verify master) &&
+	test "$victim_head" = "$pushed_head"
 '
 
 test_expect_success \
         'push can be used to delete a ref' '
-	cd victim &&
-	git branch extra master &&
-	cd .. &&
-	test -f victim/.git/refs/heads/extra &&
-	git send-pack ./victim/.git/ :extra master &&
-	! test -f victim/.git/refs/heads/extra
+	( cd victim && git branch extra master ) &&
+	git send-pack ./victim :extra master &&
+	( cd victim &&
+	  test_must_fail git rev-parse --verify extra )
 '
 
-unset GIT_CONFIG
-HOME=`pwd`/no-such-directory
-export HOME ;# this way we force the victim/.git/config to be used.
-
-test_expect_success \
-	'pushing a delete should be denied with denyDeletes' '
-	cd victim &&
-	git config receive.denyDeletes true &&
-	git branch extra master &&
-	cd .. &&
-	test -f victim/.git/refs/heads/extra &&
-	test_must_fail git send-pack ./victim/.git/ :extra master
+test_expect_success 'refuse deleting push with denyDeletes' '
+	(
+	    cd victim &&
+	    ( git branch -D extra || : ) &&
+	    git config receive.denyDeletes true &&
+	    git branch extra master
+	) &&
+	test_must_fail git send-pack ./victim :extra master
 '
-rm -f victim/.git/refs/heads/extra
 
-test_expect_success \
-        'pushing with --force should be denied with denyNonFastforwards' '
-	cd victim &&
-	git config receive.denyNonFastforwards true &&
-	cd .. &&
-	git update-ref refs/heads/master master^ || return 1
-	git send-pack --force ./victim/.git/ master && return 1
-	! test_cmp .git/refs/heads/master victim/.git/refs/heads/master
+test_expect_success 'denyNonFastforwards trumps --force' '
+	(
+	    cd victim &&
+	    ( git branch -D extra || : ) &&
+	    git config receive.denyNonFastforwards true
+	) &&
+	victim_orig=$(cd victim && git rev-parse --verify master) &&
+	test_must_fail git send-pack --force ./victim master^:master &&
+	victim_head=$(cd victim && git rev-parse --verify master) &&
+	test "$victim_orig" = "$victim_head"
 '
 
-test_expect_success \
-	'pushing does not include non-head refs' '
-	mkdir parent && cd parent &&
-	git init && touch file && git add file && git commit -m add &&
-	cd .. &&
-	git clone parent child && cd child && git push --all &&
-	cd ../parent &&
-	git branch -a >branches && ! grep origin/master branches
+test_expect_success 'push --all excludes remote tracking hierarchy' '
+	mkdir parent &&
+	(
+	    cd parent &&
+	    git init && : >file && git add file && git commit -m add
+	) &&
+	git clone parent child &&
+	(
+	    cd child && git push --all
+	) &&
+	(
+	    cd parent &&
+	    test -z "$(git for-each-ref refs/remotes/origin)"
+	)
 '
 
 rewound_push_setup() {
 	rm -rf parent child &&
-	mkdir parent && cd parent &&
-	git init && echo one >file && git add file && git commit -m one &&
-	echo two >file && git commit -a -m two &&
-	cd .. &&
-	git clone parent child && cd child && git reset --hard HEAD^
+	mkdir parent &&
+	(
+	    cd parent &&
+	    git init &&
+	    echo one >file && git add file && git commit -m one &&
+	    echo two >file && git commit -a -m two
+	) &&
+	git clone parent child &&
+	(
+	    cd child && git reset --hard HEAD^
+	)
 }
 
 rewound_push_succeeded() {
@@ -156,30 +150,44 @@ rewound_push_failed() {
 	fi
 }
 
-test_expect_success \
-	'pushing explicit refspecs respects forcing' '
+test_expect_success 'pushing explicit refspecs respects forcing' '
 	rewound_push_setup &&
-	if git send-pack ../parent/.git refs/heads/master:refs/heads/master
-	then
-		false
-	else
-		true
-	fi && rewound_push_failed &&
-	git send-pack ../parent/.git +refs/heads/master:refs/heads/master &&
-	rewound_push_succeeded
+	parent_orig=$(cd parent && git rev-parse --verify master) &&
+	(
+	    cd child &&
+	    test_must_fail git send-pack ../parent \
+		refs/heads/master:refs/heads/master
+	) &&
+	parent_head=$(cd parent && git rev-parse --verify master) &&
+	test "$parent_orig" = "$parent_head"
+	(
+	    cd child &&
+	    git send-pack ../parent \
+	        +refs/heads/master:refs/heads/master
+	) &&
+	parent_head=$(cd parent && git rev-parse --verify master) &&
+	child_head=$(cd parent && git rev-parse --verify master) &&
+	test "$parent_head" = "$child_head"
 '
 
-test_expect_success \
-	'pushing wildcard refspecs respects forcing' '
+test_expect_success 'pushing wildcard refspecs respects forcing' '
 	rewound_push_setup &&
-	if git send-pack ../parent/.git refs/heads/*:refs/heads/*
-	then
-		false
-	else
-		true
-	fi && rewound_push_failed &&
-	git send-pack ../parent/.git +refs/heads/*:refs/heads/* &&
-	rewound_push_succeeded
+	parent_orig=$(cd parent && git rev-parse --verify master) &&
+	(
+	    cd child &&
+	    test_must_fail git send-pack ../parent \
+	        "refs/heads/*:refs/heads/*"
+	) &&
+	parent_head=$(cd parent && git rev-parse --verify master) &&
+	test "$parent_orig" = "$parent_head"
+	(
+	    cd child &&
+	    git send-pack ../parent \
+	        "+refs/heads/*:refs/heads/*"
+	) &&
+	parent_head=$(cd parent && git rev-parse --verify master) &&
+	child_head=$(cd parent && git rev-parse --verify master) &&
+	test "$parent_head" = "$child_head"
 '
 
 test_done
-- 
1.6.2.rc0.36.g8307
Previous: Junio C HamanoNext: Jeff King
Message 26 of 29 in “Deleting the "current" branch in remote bare repositories”
  1. Jan KrügerFeb 7, 2009
  2. Felipe ContrerasFeb 7, 2009
  3. Jan KrügerFeb 8, 2009
  4. Jeff KingFeb 8, 2009
  5. Junio C HamanoFeb 8, 2009
  6. Jeff KingFeb 8, 2009
  7. Junio C HamanoFeb 8, 2009
  8. 0/6 Deleting the "current" branch in a remote repositoryJunio C Hamano, Feb 9, 2009
  9. 1/6 builtin-receive-pack.c: do not initialize statics to 0Junio C Hamano, Feb 9, 2009
  10. 2/6 t5400: allow individual tests to failJunio C Hamano, Feb 9, 2009
  11. 3/6 receive-pack: receive.denyDeleteCurrentJunio C Hamano, Feb 9, 2009
  12. 4/6 remote prune: warn dangling symrefsJunio C Hamano, Feb 9, 2009
  13. 5/6 Warn use of "origin" when remotes/origin/HEAD is danglingJunio C Hamano, Feb 9, 2009
  14. 6/6 receive-pack: default receive.denyDeleteCurrent to refuseJunio C Hamano, Feb 9, 2009
  15. Jeff KingFeb 9, 2009
  16. Junio C HamanoFeb 11, 2009
  17. Jeff KingFeb 11, 2009
  18. Jeff KingFeb 11, 2009
  19. Jeff KingFeb 9, 2009
  20. Jeff KingFeb 9, 2009
  21. Junio C HamanoFeb 9, 2009
  22. Jeff KingFeb 10, 2009
  23. Junio C HamanoFeb 10, 2009
  24. Jeff KingFeb 9, 2009
  25. Junio C HamanoFeb 9, 2009
  26. Junio C HamanoFeb 9, 2009
  27. Jeff KingFeb 10, 2009
  28. Jeff KingFeb 9, 2009
  29. Jeff KingFeb 9, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.