All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] t: make many tests depend less on the refs being files
@ 2018-05-23  5:25 Christian Couder
  2018-05-23  6:08 ` Junio C Hamano
  2018-05-25  8:48 ` Michael Haggerty
  0 siblings, 2 replies; 7+ messages in thread
From: Christian Couder @ 2018-05-23  5:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Stefan Beller, Jonathan Nieder, Jonathan Tan, Duy Nguyen,
	Derrick Stolee, Carlos Martín Nieto, Michael Haggerty,
	David Turner, Johannes Schindelin, SZEDER Gábor,
	Christian Couder, David Turner

From: David Turner <dturner@twopensource.com>

Many tests are very focused on the file system representation of the
loose and packed refs code. As there are plans to implement other
ref storage systems, let's migrate these tests to a form that test
the intent of the refs storage system instead of it internals.

This will make clear to readers that these tests do not depend on
which ref backend is used.

The internals of the loose refs backend are still tested in
t1400-update-ref.sh, whereas the tests changed in this patch focus
on testing other aspects.

This patch just takes care of many low hanging fruits. It does not
try to completely solves the issue.

Helped-by: Stefan Beller <sbeller@google.com>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/lib-t6000.sh                   |  6 +++---
 t/t1401-symbolic-ref.sh          |  2 +-
 t/t3200-branch.sh                | 18 +++++++++---------
 t/t3903-stash.sh                 |  2 +-
 t/t5500-fetch-pack.sh            | 10 +++++-----
 t/t5510-fetch.sh                 |  6 +++---
 t/t6010-merge-base.sh            |  2 +-
 t/t7201-co.sh                    |  2 +-
 t/t9104-git-svn-follow-parent.sh |  3 ++-
 9 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/t/lib-t6000.sh b/t/lib-t6000.sh
index 3f2d873fec..b0ed4767e3 100644
--- a/t/lib-t6000.sh
+++ b/t/lib-t6000.sh
@@ -4,11 +4,11 @@ mkdir -p .git/refs/tags
 
 >sed.script
 
-# Answer the sha1 has associated with the tag. The tag must exist in .git/refs/tags
+# Answer the sha1 has associated with the tag. The tag must exist under refs/tags
 tag () {
 	_tag=$1
-	test -f ".git/refs/tags/$_tag" || error "tag: \"$_tag\" does not exist"
-	cat ".git/refs/tags/$_tag"
+	git rev-parse --verify "refs/tags/$_tag" ||
+	error "tag: \"$_tag\" does not exist"
 }
 
 # Generate a commit using the text specified to make it unique and the tree
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index 9e782a8122..a4ebb0b65f 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -65,7 +65,7 @@ reset_to_sane
 test_expect_success 'symbolic-ref fails to delete real ref' '
 	echo "fatal: Cannot delete refs/heads/foo, not a symbolic ref" >expect &&
 	test_must_fail git symbolic-ref -d refs/heads/foo >actual 2>&1 &&
-	test_path_is_file .git/refs/heads/foo &&
+	git rev-parse --verify refs/heads/foo &&
 	test_cmp expect actual
 '
 reset_to_sane
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index c0ef946811..222dc2c377 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -234,34 +234,34 @@ test_expect_success 'git branch -M master2 master2 should work when master is ch
 
 test_expect_success 'git branch -v -d t should work' '
 	git branch t &&
-	test_path_is_file .git/refs/heads/t &&
+	git rev-parse --verify refs/heads/t &&
 	git branch -v -d t &&
-	test_path_is_missing .git/refs/heads/t
+	test_must_fail git rev-parse --verify refs/heads/t
 '
 
 test_expect_success 'git branch -v -m t s should work' '
 	git branch t &&
-	test_path_is_file .git/refs/heads/t &&
+	git rev-parse --verify refs/heads/t &&
 	git branch -v -m t s &&
-	test_path_is_missing .git/refs/heads/t &&
-	test_path_is_file .git/refs/heads/s &&
+	test_must_fail git rev-parse --verify refs/heads/t &&
+	git rev-parse --verify refs/heads/s &&
 	git branch -d s
 '
 
 test_expect_success 'git branch -m -d t s should fail' '
 	git branch t &&
-	test_path_is_file .git/refs/heads/t &&
+	git rev-parse refs/heads/t &&
 	test_must_fail git branch -m -d t s &&
 	git branch -d t &&
-	test_path_is_missing .git/refs/heads/t
+	test_must_fail git rev-parse refs/heads/t
 '
 
 test_expect_success 'git branch --list -d t should fail' '
 	git branch t &&
-	test_path_is_file .git/refs/heads/t &&
+	git rev-parse refs/heads/t &&
 	test_must_fail git branch --list -d t &&
 	git branch -d t &&
-	test_path_is_missing .git/refs/heads/t
+	test_must_fail git rev-parse refs/heads/t
 '
 
 test_expect_success 'git branch --list -v with --abbrev' '
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index aefde7b172..1f871d3cca 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -726,7 +726,7 @@ test_expect_success 'store updates stash ref and reflog' '
 	git reset --hard &&
 	! grep quux bazzy &&
 	git stash store -m quuxery $STASH_ID &&
-	test $(cat .git/refs/stash) = $STASH_ID &&
+	test $(git rev-parse stash) = $STASH_ID &&
 	git reflog --format=%H stash| grep $STASH_ID &&
 	git stash pop &&
 	grep quux bazzy
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 0680dec808..d4f435155f 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -30,7 +30,7 @@ add () {
 	test_tick &&
 	commit=$(echo "$text" | git commit-tree $tree $parents) &&
 	eval "$name=$commit; export $name" &&
-	echo $commit > .git/refs/heads/$branch &&
+	git update-ref "refs/heads/$branch" "$commit" &&
 	eval ${branch}TIP=$commit
 }
 
@@ -45,10 +45,10 @@ pull_to_client () {
 
 			case "$heads" in
 			    *A*)
-				    echo $ATIP > .git/refs/heads/A;;
+				    git update-ref refs/heads/A "$ATIP";;
 			esac &&
 			case "$heads" in *B*)
-			    echo $BTIP > .git/refs/heads/B;;
+			    git update-ref refs/heads/B "$BTIP";;
 			esac &&
 			git symbolic-ref HEAD refs/heads/$(echo $heads \
 				| sed -e "s/^\(.\).*$/\1/") &&
@@ -92,8 +92,8 @@ test_expect_success 'setup' '
 		cur=$(($cur+1))
 	done &&
 	add B1 $A1 &&
-	echo $ATIP > .git/refs/heads/A &&
-	echo $BTIP > .git/refs/heads/B &&
+	git update-ref refs/heads/A "$ATIP" &&
+	git update-ref refs/heads/B "$BTIP" &&
 	git symbolic-ref HEAD refs/heads/B
 '
 
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index ae5a530a2d..e402aee6a2 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -63,7 +63,7 @@ test_expect_success "fetch test" '
 	git commit -a -m "updated by origin" &&
 	cd two &&
 	git fetch &&
-	test -f .git/refs/heads/one &&
+	git rev-parse --verify refs/heads/one &&
 	mine=$(git rev-parse refs/heads/one) &&
 	his=$(cd ../one && git rev-parse refs/heads/master) &&
 	test "z$mine" = "z$his"
@@ -73,8 +73,8 @@ test_expect_success "fetch test for-merge" '
 	cd "$D" &&
 	cd three &&
 	git fetch &&
-	test -f .git/refs/heads/two &&
-	test -f .git/refs/heads/one &&
+	git rev-parse --verify refs/heads/two &&
+	git rev-parse --verify refs/heads/one &&
 	master_in_two=$(cd ../two && git rev-parse master) &&
 	one_in_two=$(cd ../two && git rev-parse one) &&
 	{
diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
index 31db7b5f91..aa2d360ce3 100755
--- a/t/t6010-merge-base.sh
+++ b/t/t6010-merge-base.sh
@@ -34,7 +34,7 @@ doit () {
 
 	commit=$(echo $NAME | git commit-tree $T $PARENTS) &&
 
-	echo $commit >.git/refs/tags/$NAME &&
+	git update-ref "refs/tags/$NAME" "$commit" &&
 	echo $commit
 }
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 76c223c967..ab9da61da3 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -65,7 +65,7 @@ test_expect_success setup '
 test_expect_success "checkout from non-existing branch" '
 
 	git checkout -b delete-me master &&
-	rm .git/refs/heads/delete-me &&
+	git update-ref -d --no-deref refs/heads/delete-me &&
 	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
 	git checkout master &&
 	test refs/heads/master = "$(git symbolic-ref HEAD)"
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index a735fa3717..9c49b6c1fe 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -215,7 +215,8 @@ test_expect_success "multi-fetch continues to work" "
 	"
 
 test_expect_success "multi-fetch works off a 'clean' repository" '
-	rm -r "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" "$GIT_DIR/logs" &&
+	rm -rf "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" &&
+	git reflog expire --all --expire=all &&
 	mkdir "$GIT_DIR/svn" &&
 	git svn multi-fetch
 	'
-- 
2.17.0.764.gc4bd0c3328


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t: make many tests depend less on the refs being files
  2018-05-23  5:25 [PATCH v2] t: make many tests depend less on the refs being files Christian Couder
@ 2018-05-23  6:08 ` Junio C Hamano
  2018-05-25  8:48 ` Michael Haggerty
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2018-05-23  6:08 UTC (permalink / raw)
  To: Christian Couder
  Cc: git, Ævar Arnfjörð Bjarmason, Stefan Beller,
	Jonathan Nieder, Jonathan Tan, Duy Nguyen, Derrick Stolee,
	Carlos Martín Nieto, Michael Haggerty, David Turner,
	Johannes Schindelin, SZEDER Gábor, Christian Couder,
	David Turner

Christian Couder <christian.couder@gmail.com> writes:

> The internals of the loose refs backend are still tested in
> t1400-update-ref.sh, whereas the tests changed in this patch focus
> on testing other aspects.
>
> This patch just takes care of many low hanging fruits. It does not
> try to completely solves the issue.

Thanks.  All conversions in this patch look correct to me.

Will queue.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t: make many tests depend less on the refs being files
  2018-05-23  5:25 [PATCH v2] t: make many tests depend less on the refs being files Christian Couder
  2018-05-23  6:08 ` Junio C Hamano
@ 2018-05-25  8:48 ` Michael Haggerty
  2018-05-25  8:59   ` Jeff King
  2018-05-25 11:21   ` Christian Couder
  1 sibling, 2 replies; 7+ messages in thread
From: Michael Haggerty @ 2018-05-25  8:48 UTC (permalink / raw)
  To: Christian Couder, git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Stefan Beller, Jonathan Nieder, Jonathan Tan, Duy Nguyen,
	Derrick Stolee, Carlos Martín Nieto, David Turner,
	Johannes Schindelin, SZEDER Gábor, Christian Couder,
	David Turner

On 05/23/2018 07:25 AM, Christian Couder wrote:
> From: David Turner <dturner@twopensource.com>
> 
> Many tests are very focused on the file system representation of the
> loose and packed refs code. As there are plans to implement other
> ref storage systems, let's migrate these tests to a form that test
> the intent of the refs storage system instead of it internals.
> [...]
> 
> diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
> index 9e782a8122..a4ebb0b65f 100755
> --- a/t/t1401-symbolic-ref.sh
> +++ b/t/t1401-symbolic-ref.sh
> @@ -65,7 +65,7 @@ reset_to_sane
>  test_expect_success 'symbolic-ref fails to delete real ref' '
>  	echo "fatal: Cannot delete refs/heads/foo, not a symbolic ref" >expect &&
>  	test_must_fail git symbolic-ref -d refs/heads/foo >actual 2>&1 &&
> -	test_path_is_file .git/refs/heads/foo &&
> +	git rev-parse --verify refs/heads/foo &&
>  	test_cmp expect actual
>  '
>  reset_to_sane

Should t1401 be considered a backend-agnostic test, or is it needed to
ensure that symbolic refs are written correctly in the files backend?

> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index c0ef946811..222dc2c377 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -234,34 +234,34 @@ test_expect_success 'git branch -M master2 master2 should work when master is ch
>  
>  test_expect_success 'git branch -v -d t should work' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse --verify refs/heads/t &&
>  	git branch -v -d t &&
> -	test_path_is_missing .git/refs/heads/t
> +	test_must_fail git rev-parse --verify refs/heads/t
>  '
>  
>  test_expect_success 'git branch -v -m t s should work' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse --verify refs/heads/t &&
>  	git branch -v -m t s &&
> -	test_path_is_missing .git/refs/heads/t &&
> -	test_path_is_file .git/refs/heads/s &&
> +	test_must_fail git rev-parse --verify refs/heads/t &&
> +	git rev-parse --verify refs/heads/s &&
>  	git branch -d s
>  '
>  
>  test_expect_success 'git branch -m -d t s should fail' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse refs/heads/t &&
>  	test_must_fail git branch -m -d t s &&
>  	git branch -d t &&
> -	test_path_is_missing .git/refs/heads/t
> +	test_must_fail git rev-parse refs/heads/t
>  '
>  
>  test_expect_success 'git branch --list -d t should fail' '
>  	git branch t &&
> -	test_path_is_file .git/refs/heads/t &&
> +	git rev-parse refs/heads/t &&
>  	test_must_fail git branch --list -d t &&
>  	git branch -d t &&
> -	test_path_is_missing .git/refs/heads/t
> +	test_must_fail git rev-parse refs/heads/t
>  '
>  
>  test_expect_success 'git branch --list -v with --abbrev' '
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index aefde7b172..1f871d3cca 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -726,7 +726,7 @@ test_expect_success 'store updates stash ref and reflog' '
>  	git reset --hard &&
>  	! grep quux bazzy &&
>  	git stash store -m quuxery $STASH_ID &&
> -	test $(cat .git/refs/stash) = $STASH_ID &&
> +	test $(git rev-parse stash) = $STASH_ID &&
>  	git reflog --format=%H stash| grep $STASH_ID &&
>  	git stash pop &&
>  	grep quux bazzy
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 0680dec808..d4f435155f 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -30,7 +30,7 @@ add () {
>  	test_tick &&
>  	commit=$(echo "$text" | git commit-tree $tree $parents) &&
>  	eval "$name=$commit; export $name" &&
> -	echo $commit > .git/refs/heads/$branch &&
> +	git update-ref "refs/heads/$branch" "$commit" &&
>  	eval ${branch}TIP=$commit
>  }
>  
> @@ -45,10 +45,10 @@ pull_to_client () {
>  
>  			case "$heads" in
>  			    *A*)
> -				    echo $ATIP > .git/refs/heads/A;;
> +				    git update-ref refs/heads/A "$ATIP";;
>  			esac &&
>  			case "$heads" in *B*)
> -			    echo $BTIP > .git/refs/heads/B;;
> +			    git update-ref refs/heads/B "$BTIP";;
>  			esac &&
>  			git symbolic-ref HEAD refs/heads/$(echo $heads \
>  				| sed -e "s/^\(.\).*$/\1/") &&
> @@ -92,8 +92,8 @@ test_expect_success 'setup' '
>  		cur=$(($cur+1))
>  	done &&
>  	add B1 $A1 &&
> -	echo $ATIP > .git/refs/heads/A &&
> -	echo $BTIP > .git/refs/heads/B &&
> +	git update-ref refs/heads/A "$ATIP" &&
> +	git update-ref refs/heads/B "$BTIP" &&
>  	git symbolic-ref HEAD refs/heads/B
>  '
>  
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index ae5a530a2d..e402aee6a2 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -63,7 +63,7 @@ test_expect_success "fetch test" '
>  	git commit -a -m "updated by origin" &&
>  	cd two &&
>  	git fetch &&
> -	test -f .git/refs/heads/one &&
> +	git rev-parse --verify refs/heads/one &&
>  	mine=$(git rev-parse refs/heads/one) &&
>  	his=$(cd ../one && git rev-parse refs/heads/master) &&
>  	test "z$mine" = "z$his"
> @@ -73,8 +73,8 @@ test_expect_success "fetch test for-merge" '
>  	cd "$D" &&
>  	cd three &&
>  	git fetch &&
> -	test -f .git/refs/heads/two &&
> -	test -f .git/refs/heads/one &&
> +	git rev-parse --verify refs/heads/two &&
> +	git rev-parse --verify refs/heads/one &&
>  	master_in_two=$(cd ../two && git rev-parse master) &&
>  	one_in_two=$(cd ../two && git rev-parse one) &&
>  	{
> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
> index 31db7b5f91..aa2d360ce3 100755
> --- a/t/t6010-merge-base.sh
> +++ b/t/t6010-merge-base.sh
> @@ -34,7 +34,7 @@ doit () {
>  
>  	commit=$(echo $NAME | git commit-tree $T $PARENTS) &&
>  
> -	echo $commit >.git/refs/tags/$NAME &&
> +	git update-ref "refs/tags/$NAME" "$commit" &&
>  	echo $commit
>  }
>  
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 76c223c967..ab9da61da3 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -65,7 +65,7 @@ test_expect_success setup '
>  test_expect_success "checkout from non-existing branch" '
>  
>  	git checkout -b delete-me master &&
> -	rm .git/refs/heads/delete-me &&
> +	git update-ref -d --no-deref refs/heads/delete-me &&
>  	test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
>  	git checkout master &&
>  	test refs/heads/master = "$(git symbolic-ref HEAD)"
> diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
> index a735fa3717..9c49b6c1fe 100755
> --- a/t/t9104-git-svn-follow-parent.sh
> +++ b/t/t9104-git-svn-follow-parent.sh
> @@ -215,7 +215,8 @@ test_expect_success "multi-fetch continues to work" "
>  	"
>  
>  test_expect_success "multi-fetch works off a 'clean' repository" '
> -	rm -r "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" "$GIT_DIR/logs" &&
> +	rm -rf "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" &&
> +	git reflog expire --all --expire=all &&
>  	mkdir "$GIT_DIR/svn" &&
>  	git svn multi-fetch
>  	'
> 

`rm -rf "$GIT_DIR/refs/remotes"` is not kosher. I think it can be written

    printf 'option no-deref\ndelete %s\n' $(git for-each-ref
--format='%(refname)' refs/remotes) | git update-ref --stdin

as long as the number of references doesn't exceed command-line limits.
This will also take care of the reflogs. Another alternative would be to
write it as a loop.

Michael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t: make many tests depend less on the refs being files
  2018-05-25  8:48 ` Michael Haggerty
@ 2018-05-25  8:59   ` Jeff King
  2018-05-25  9:05     ` Michael Haggerty
  2018-05-25 11:21   ` Christian Couder
  1 sibling, 1 reply; 7+ messages in thread
From: Jeff King @ 2018-05-25  8:59 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Christian Couder, git, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Stefan Beller,
	Jonathan Nieder, Jonathan Tan, Duy Nguyen, Derrick Stolee,
	Carlos Martín Nieto, David Turner, Johannes Schindelin,
	SZEDER Gábor, Christian Couder, David Turner

On Fri, May 25, 2018 at 10:48:04AM +0200, Michael Haggerty wrote:

> >  test_expect_success "multi-fetch works off a 'clean' repository" '
> > -	rm -r "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" "$GIT_DIR/logs" &&
> > +	rm -rf "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" &&
> > +	git reflog expire --all --expire=all &&
> >  	mkdir "$GIT_DIR/svn" &&
> >  	git svn multi-fetch
> >  	'
> > 
> 
> `rm -rf "$GIT_DIR/refs/remotes"` is not kosher. I think it can be written
> 
>     printf 'option no-deref\ndelete %s\n' $(git for-each-ref
> --format='%(refname)' refs/remotes) | git update-ref --stdin
> 
> as long as the number of references doesn't exceed command-line limits.
> This will also take care of the reflogs. Another alternative would be to
> write it as a loop.

Perhaps:

  git for-each-ref --format="option no-deref%0adelete %(refname)" refs/remotes |
  git update-ref --stdin

-Peff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t: make many tests depend less on the refs being files
  2018-05-25  8:59   ` Jeff King
@ 2018-05-25  9:05     ` Michael Haggerty
  2018-05-25 11:22       ` Christian Couder
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Haggerty @ 2018-05-25  9:05 UTC (permalink / raw)
  To: Jeff King
  Cc: Christian Couder, Git Mailing List, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Stefan Beller,
	Jonathan Nieder, Jonathan Tan, Duy Nguyen, Derrick Stolee,
	Carlos Martín Nieto, David Turner, Johannes Schindelin,
	SZEDER Gábor, Christian Couder, David Turner

On Fri, May 25, 2018 at 10:59 AM, Jeff King <peff@peff.net> wrote:
> On Fri, May 25, 2018 at 10:48:04AM +0200, Michael Haggerty wrote:
>
>> >  test_expect_success "multi-fetch works off a 'clean' repository" '
>> > -   rm -r "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" "$GIT_DIR/logs" &&
>> > +   rm -rf "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" &&
>> > +   git reflog expire --all --expire=all &&
>> >     mkdir "$GIT_DIR/svn" &&
>> >     git svn multi-fetch
>> >     '
>> >
>>
>> `rm -rf "$GIT_DIR/refs/remotes"` is not kosher. I think it can be written
>>
>>     printf 'option no-deref\ndelete %s\n' $(git for-each-ref
>> --format='%(refname)' refs/remotes) | git update-ref --stdin
>>
>> as long as the number of references doesn't exceed command-line limits.
>> This will also take care of the reflogs. Another alternative would be to
>> write it as a loop.
>
> Perhaps:
>
>   git for-each-ref --format="option no-deref%0adelete %(refname)" refs/remotes |
>   git update-ref --stdin

Ah yes, that's nicer. I tried with `\n`, but that's not supported
(wouldn't it be nice if it were?). I didn't think to try `%0a` (let
alone look in the documentation!)

Michael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t: make many tests depend less on the refs being files
  2018-05-25  8:48 ` Michael Haggerty
  2018-05-25  8:59   ` Jeff King
@ 2018-05-25 11:21   ` Christian Couder
  1 sibling, 0 replies; 7+ messages in thread
From: Christian Couder @ 2018-05-25 11:21 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Stefan Beller, Jonathan Nieder, Jonathan Tan, Duy Nguyen,
	Derrick Stolee, Carlos Martín Nieto, David Turner,
	Johannes Schindelin, SZEDER Gábor, Christian Couder,
	David Turner

On Fri, May 25, 2018 at 10:48 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 05/23/2018 07:25 AM, Christian Couder wrote:
>>
>> diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
>> index 9e782a8122..a4ebb0b65f 100755
>> --- a/t/t1401-symbolic-ref.sh
>> +++ b/t/t1401-symbolic-ref.sh
>> @@ -65,7 +65,7 @@ reset_to_sane
>>  test_expect_success 'symbolic-ref fails to delete real ref' '
>>       echo "fatal: Cannot delete refs/heads/foo, not a symbolic ref" >expect &&
>>       test_must_fail git symbolic-ref -d refs/heads/foo >actual 2>&1 &&
>> -     test_path_is_file .git/refs/heads/foo &&
>> +     git rev-parse --verify refs/heads/foo &&
>>       test_cmp expect actual
>>  '
>>  reset_to_sane
>
> Should t1401 be considered a backend-agnostic test, or is it needed to
> ensure that symbolic refs are written correctly in the files backend?

I don't know. And I am ok to go either way. Another possibility would
be to split in two parts.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t: make many tests depend less on the refs being files
  2018-05-25  9:05     ` Michael Haggerty
@ 2018-05-25 11:22       ` Christian Couder
  0 siblings, 0 replies; 7+ messages in thread
From: Christian Couder @ 2018-05-25 11:22 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Jeff King, Git Mailing List, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Stefan Beller,
	Jonathan Nieder, Jonathan Tan, Duy Nguyen, Derrick Stolee,
	Carlos Martín Nieto, David Turner, Johannes Schindelin,
	SZEDER Gábor, Christian Couder, David Turner

On Fri, May 25, 2018 at 11:05 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On Fri, May 25, 2018 at 10:59 AM, Jeff King <peff@peff.net> wrote:
>> On Fri, May 25, 2018 at 10:48:04AM +0200, Michael Haggerty wrote:
>>
>>> >  test_expect_success "multi-fetch works off a 'clean' repository" '
>>> > -   rm -r "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" "$GIT_DIR/logs" &&
>>> > +   rm -rf "$GIT_DIR/svn" "$GIT_DIR/refs/remotes" &&
>>> > +   git reflog expire --all --expire=all &&
>>> >     mkdir "$GIT_DIR/svn" &&
>>> >     git svn multi-fetch
>>> >     '
>>> >
>>>
>>> `rm -rf "$GIT_DIR/refs/remotes"` is not kosher. I think it can be written
>>>
>>>     printf 'option no-deref\ndelete %s\n' $(git for-each-ref
>>> --format='%(refname)' refs/remotes) | git update-ref --stdin
>>>
>>> as long as the number of references doesn't exceed command-line limits.
>>> This will also take care of the reflogs. Another alternative would be to
>>> write it as a loop.
>>
>> Perhaps:
>>
>>   git for-each-ref --format="option no-deref%0adelete %(refname)" refs/remotes |
>>   git update-ref --stdin
>
> Ah yes, that's nicer. I tried with `\n`, but that's not supported
> (wouldn't it be nice if it were?). I didn't think to try `%0a` (let
> alone look in the documentation!)

Thanks both for this suggestion. I plan to use it in another patch.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-05-25 11:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23  5:25 [PATCH v2] t: make many tests depend less on the refs being files Christian Couder
2018-05-23  6:08 ` Junio C Hamano
2018-05-25  8:48 ` Michael Haggerty
2018-05-25  8:59   ` Jeff King
2018-05-25  9:05     ` Michael Haggerty
2018-05-25 11:22       ` Christian Couder
2018-05-25 11:21   ` Christian Couder

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.