[PATCH v2 0/2] Generalize reference locking in tests

Hello again,

This is the second version my patch series that refactors two tests to no longer be dependent on the files reference backend. Changes compared to v1:

 * Removed reliance on fifo magic to trigger error conditions.
 * d/f conflicts now used to create errors instead.

Thanks for taking a look!

Justin

Justin Tobler (2):
  t1401: remove lockfile creation
  t5541: remove lockfile creation

 t/t1401-symbolic-ref.sh    |  5 ++---
 t/t5541-http-push-smart.sh | 18 +++++-------------
 2 files changed, 7 insertions(+), 16 deletions(-)

base-commit: 624eb90fa8f65a79396615f3c2842ac5a3743350
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1634%2Fjltobler%2Fjt%2Fbackend-generic-ref-lock-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1634/jltobler/jt/backend-generic-ref-lock-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1634

Range-diff vs v1:

 1:  cb78b549e5e ! 1:  714f713ccec t1401: generalize reference locking
     @@ Metadata
      Author: Justin Tobler <jltobler@gmail.com>
      
       ## Commit message ##
     -    t1401: generalize reference locking
     +    t1401: remove lockfile creation
      
     -    Some tests set up reference locks by directly creating the lockfile.
     -    While this works for the files reference backend, reftable reference
     -    locks operate differently and are incompatible with this approach.
     -    Refactor the test to use git-update-ref(1) to lock refs instead so that
     -    the test does not need to be aware of how the ref backend locks refs.
     +    To create error conditions, some tests set up reference locks by
     +    directly creating its lockfile. While this works for the files reference
     +    backend, this approach is incompatible with the reftable backend.
     +    Refactor the test to create a d/f conflict via git-symbolic-ref(1)
     +    instead so that the test is reference backend agnostic.
      
          Signed-off-by: Justin Tobler <jltobler@gmail.com>
      
       ## t/t1401-symbolic-ref.sh ##
      @@ t/t1401-symbolic-ref.sh: test_expect_success LONG_REF 'we can parse long symbolic ref' '
     - 	test_cmp expect actual
       '
       
     --test_expect_success 'symbolic-ref reports failure in exit code' '
     + test_expect_success 'symbolic-ref reports failure in exit code' '
      -	test_when_finished "rm -f .git/HEAD.lock" &&
      -	>.git/HEAD.lock &&
      -	test_must_fail git symbolic-ref HEAD refs/heads/whatever
     -+test_expect_success PIPE 'symbolic-ref reports failure in exit code' '
     -+	mkfifo in out &&
     -+	(git update-ref --stdin <in >out &) &&
     -+
     -+	exec 9>in &&
     -+	exec 8<out &&
     -+	test_when_finished "exec 9>&-" &&
     -+	test_when_finished "exec 8<&-" &&
     -+
     -+	echo "start" >&9 &&
     -+	echo "start: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     -+
     -+	echo "update HEAD refs/heads/foo" >&9 &&
     -+
     -+	echo "prepare" >&9 &&
     -+	echo "prepare: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     -+
     -+	test_must_fail git symbolic-ref HEAD refs/heads/bar
     ++	# Create d/f conflict to simulate failure.
     ++	test_must_fail git symbolic-ref refs/heads refs/heads/foo
       '
       
       test_expect_success 'symbolic-ref writes reflog entry' '
 2:  11fd5091d61 ! 2:  f953a668c6a t5541: generalize reference locking
     @@ Metadata
      Author: Justin Tobler <jltobler@gmail.com>
      
       ## Commit message ##
     -    t5541: generalize reference locking
     +    t5541: remove lockfile creation
      
     -    Some tests set up reference locks by directly creating the lockfile.
     -    While this works for the files reference backend, reftable reference
     -    locks operate differently and are incompatible with this approach.
     -    Generalize reference locking by preparing a reference transaction.
     +    To create error conditions, some tests set up reference locks by
     +    directly creating its lockfile. While this works for the files reference
     +    backend, this approach is incompatible with the reftable backend.
     +    Refactor the test to create a d/f conflict via git-update-ref(1) instead
     +    so that the test is reference backend agnostic.
      
          Signed-off-by: Justin Tobler <jltobler@gmail.com>
      
     @@ t/t5541-http-push-smart.sh: test_expect_success 'push --atomic fails on server-s
       
      -	# break ref updates for other on the remote site
      -	mkdir "$d/refs/heads/other.lock" &&
     -+	mkfifo in out &&
     -+	(git -C "$d" update-ref --stdin <in >out &) &&
     -+
     -+	exec 9>in &&
     -+	exec 8<out &&
     -+	test_when_finished "exec 9>&-" &&
     -+	test_when_finished "exec 8<&-" &&
     -+
     -+	echo "start" >&9 &&
     -+	echo "start: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     -+
     -+	echo "update refs/heads/other refs/heads/other" >&9 &&
     -+
     -+	# Prepare reference transaction on `other` reference to lock it and thus
     -+	# break updates on the remote.
     -+	echo "prepare" >&9 &&
     -+	echo "prepare: ok" >expected &&
     -+	read line <&8 &&
     -+	echo "$line" >actual &&
     -+	test_cmp expected actual &&
     ++	# Create d/f conflict to break ref updates for other on the remote site.
     ++	git -C "$d" update-ref -d refs/heads/other &&
     ++	git -C "$d" update-ref refs/heads/other/conflict HEAD &&
       
       	# add the new commit to other
       	git branch -f other collateral &&
     +@@ t/t5541-http-push-smart.sh: test_expect_success 'push --atomic fails on server-side errors' '
     + 	# --atomic should cause entire push to be rejected
     + 	test_must_fail git push --atomic "$up" atomic other 2>output  &&
     + 
     +-	# the new branch should not have been created upstream
     +-	test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
     +-
     +-	# upstream should still reflect atomic2, the last thing we pushed
     +-	# successfully
     +-	git rev-parse atomic2 >expected &&
     +-	# ...to other.
     +-	git -C "$d" rev-parse refs/heads/other >actual &&
     +-	test_cmp expected actual &&
     +-
     +-	# the new branch should not have been created upstream
     ++	# The atomic and other branches should be created upstream.
     + 	test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
     ++	test_must_fail git -C "$d" show-ref --verify refs/heads/other &&
     + 
     + 	# the failed refs should be indicated to the user
     + 	grep "^ ! .*rejected.* other -> other .*atomic transaction failed" output &&