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@xxxxxxxxx> ## 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@xxxxxxxxx> ## 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@xxxxxxxxx> ## 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@xxxxxxxxx> @@ 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 && -- gitgitgadget
