fetch+push tests: use "test_hook" and "test_when_finished" pattern

Change the "t5516-fetch-push.sh" test code to make use of the new "test_hook" helper, and to use "test_when_finished" to have tests clean up their own state, instead of relying on subsequent tests to clean the trash directory. Before this each test would have been responsible for cleaning up after a preceding test (which may or may not have run, e.g. if --run or "GIT_SKIP_TESTS" was used), now each test will instead clean up after itself. In order to use both "test_hook" and "test_when_finished" we need to move them out of sub-shells, which requires some refactoring. While we're at it split up the "push with negotiation" test, now the middle of the test doesn't need to "rm event", and since it delimited two halves that were testing two different things the end-state is easier to read and reason about. While changing these lines make the minor change from "-fr" to "-rf" as the "rm" argument, some of them used it already, it's more common in the test suite, and it leaves the end-state of the file with more consistency. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Mar 17, 2022 at 11:13 UTC f818f7f725e4a49296e9e28cf3bb26aa8a1bf3ba
1 file changed +88 -103
t/t5516-fetch-push.sh
+88 -103
@@ -23,13 +23,10 @@ D=$(pwd)
23
24 mk_empty () {
25 repo_name="$1"
26 - rm -fr "$repo_name" &&
27 - mkdir "$repo_name" &&
28 - (
29 - cd "$repo_name" &&
30 - git init &&
31 - git config receive.denyCurrentBranch warn
32 - )
26 + test_when_finished "rm -rf \"$repo_name\"" &&
27 + test_path_is_missing "$repo_name" &&
28 + git init "$repo_name" &&
29 + git -C "$repo_name" config receive.denyCurrentBranch warn
30 }
31
32 mk_test () {
@@ -58,40 +55,28 @@ mk_test () {
55 mk_test_with_hooks() {
56 repo_name=$1
57 mk_test "$@" &&
61 - (
62 - cd "$repo_name" &&
63 - mkdir .git/hooks &&
64 - cd .git/hooks &&
65 -
66 - cat >pre-receive <<-'EOF' &&
67 - #!/bin/sh
68 - cat - >>pre-receive.actual
69 - EOF
70 -
71 - cat >update <<-'EOF' &&
72 - #!/bin/sh
73 - printf "%s %s %s\n" "$@" >>update.actual
74 - EOF
75 -
76 - cat >post-receive <<-'EOF' &&
77 - #!/bin/sh
78 - cat - >>post-receive.actual
79 - EOF
80 -
81 - cat >post-update <<-'EOF' &&
82 - #!/bin/sh
83 - for ref in "$@"
84 - do
85 - printf "%s\n" "$ref" >>post-update.actual
86 - done
87 - EOF
88 -
89 - chmod +x pre-receive update post-receive post-update
90 - )
58 + test_hook -C "$repo_name" pre-receive <<-'EOF' &&
59 + cat - >>pre-receive.actual
60 + EOF
61 +
62 + test_hook -C "$repo_name" update <<-'EOF' &&
63 + printf "%s %s %s\n" "$@" >>update.actual
64 + EOF
65 +
66 + test_hook -C "$repo_name" post-receive <<-'EOF' &&
67 + cat - >>post-receive.actual
68 + EOF
69 +
70 + test_hook -C "$repo_name" post-update <<-'EOF'
71 + for ref in "$@"
72 + do
73 + printf "%s\n" "$ref" >>post-update.actual
74 + done
75 + EOF
76 }
77
78 mk_child() {
94 - rm -rf "$2" &&
79 + test_when_finished "rm -rf \"$2\"" &&
80 git clone "$1" "$2"
81 }
82
@@ -196,32 +181,32 @@ grep_wrote () {
181 grep 'write_pack_file/wrote.*"value":"'$1'"' $2
182 }
183
199 -test_expect_success 'push with negotiation' '
200 - # Without negotiation
184 +test_expect_success 'push without negotiation' '
185 mk_empty testrepo &&
186 git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
187 test_commit -C testrepo unrelated_commit &&
188 git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
205 - echo now pushing without negotiation &&
189 + test_when_finished "rm event" &&
190 GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 push testrepo refs/heads/main:refs/remotes/origin/main &&
207 - grep_wrote 5 event && # 2 commits, 2 trees, 1 blob
191 + grep_wrote 5 event # 2 commits, 2 trees, 1 blob
192 +'
193
209 - # Same commands, but with negotiation
210 - rm event &&
194 +test_expect_success 'push with negotiation' '
195 mk_empty testrepo &&
196 git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
197 test_commit -C testrepo unrelated_commit &&
198 git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
199 + test_when_finished "rm event" &&
200 GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main &&
201 grep_wrote 2 event # 1 commit, 1 tree
202 '
203
204 test_expect_success 'push with negotiation proceeds anyway even if negotiation fails' '
220 - rm event &&
205 mk_empty testrepo &&
206 git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
207 test_commit -C testrepo unrelated_commit &&
208 git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
209 + test_when_finished "rm event" &&
210 GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" \
211 git -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
212 grep_wrote 5 event && # 2 commits, 2 trees, 1 blob
@@ -667,7 +652,6 @@ test_expect_success 'push does not update local refs on failure' '
652
653 mk_test testrepo heads/main &&
654 mk_child testrepo child &&
670 - mkdir testrepo/.git/hooks &&
655 echo "#!/no/frobnication/today" >testrepo/.git/hooks/pre-receive &&
656 chmod +x testrepo/.git/hooks/pre-receive &&
657 (
@@ -1329,7 +1313,7 @@ done
1313
1314 test_expect_success 'fetch follows tags by default' '
1315 mk_test testrepo heads/main &&
1332 - rm -fr src dst &&
1316 + test_when_finished "rm -rf src" &&
1317 git init src &&
1318 (
1319 cd src &&
@@ -1339,6 +1323,7 @@ test_expect_success 'fetch follows tags by default' '
1323 sed -n "p; s|refs/heads/main$|refs/remotes/origin/main|p" tmp1 |
1324 sort -k 3 >../expect
1325 ) &&
1326 + test_when_finished "rm -rf dst" &&
1327 git init dst &&
1328 (
1329 cd dst &&
@@ -1364,8 +1349,9 @@ test_expect_success 'peeled advertisements are not considered ref tips' '
1349
1350 test_expect_success 'pushing a specific ref applies remote.$name.push as refmap' '
1351 mk_test testrepo heads/main &&
1367 - rm -fr src dst &&
1352 + test_when_finished "rm -rf src" &&
1353 git init src &&
1354 + test_when_finished "rm -rf dst" &&
1355 git init --bare dst &&
1356 (
1357 cd src &&
@@ -1388,8 +1374,9 @@ test_expect_success 'pushing a specific ref applies remote.$name.push as refmap'
1374
1375 test_expect_success 'with no remote.$name.push, it is not used as refmap' '
1376 mk_test testrepo heads/main &&
1391 - rm -fr src dst &&
1377 + test_when_finished "rm -rf src" &&
1378 git init src &&
1379 + test_when_finished "rm -rf dst" &&
1380 git init --bare dst &&
1381 (
1382 cd src &&
@@ -1410,8 +1397,9 @@ test_expect_success 'with no remote.$name.push, it is not used as refmap' '
1397
1398 test_expect_success 'with no remote.$name.push, upstream mapping is used' '
1399 mk_test testrepo heads/main &&
1413 - rm -fr src dst &&
1400 + test_when_finished "rm -rf src" &&
1401 git init src &&
1402 + test_when_finished "rm -rf dst" &&
1403 git init --bare dst &&
1404 (
1405 cd src &&
@@ -1439,8 +1427,9 @@ test_expect_success 'with no remote.$name.push, upstream mapping is used' '
1427
1428 test_expect_success 'push does not follow tags by default' '
1429 mk_test testrepo heads/main &&
1442 - rm -fr src dst &&
1430 + test_when_finished "rm -rf src" &&
1431 git init src &&
1432 + test_when_finished "rm -rf dst" &&
1433 git init --bare dst &&
1434 (
1435 cd src &&
@@ -1462,8 +1451,9 @@ test_expect_success 'push does not follow tags by default' '
1451
1452 test_expect_success 'push --follow-tags only pushes relevant tags' '
1453 mk_test testrepo heads/main &&
1465 - rm -fr src dst &&
1454 + test_when_finished "rm -rf src" &&
1455 git init src &&
1456 + test_when_finished "rm -rf dst" &&
1457 git init --bare dst &&
1458 (
1459 cd src &&
@@ -1501,9 +1491,9 @@ EOF
1491 '
1492
1493 test_expect_success 'pushing a tag pushes the tagged object' '
1504 - rm -rf dst.git &&
1494 blob=$(echo unreferenced | git hash-object -w --stdin) &&
1495 git tag -m foo tag-of-blob $blob &&
1496 + test_when_finished "rm -rf dst.git" &&
1497 git init --bare dst.git &&
1498 git push dst.git tag-of-blob &&
1499 # the receiving index-pack should have noticed
@@ -1514,7 +1504,7 @@ test_expect_success 'pushing a tag pushes the tagged object' '
1504 '
1505
1506 test_expect_success 'push into bare respects core.logallrefupdates' '
1517 - rm -rf dst.git &&
1507 + test_when_finished "rm -rf dst.git" &&
1508 git init --bare dst.git &&
1509 git -C dst.git config core.logallrefupdates true &&
1510
@@ -1532,7 +1522,7 @@ test_expect_success 'push into bare respects core.logallrefupdates' '
1522 '
1523
1524 test_expect_success 'fetch into bare respects core.logallrefupdates' '
1535 - rm -rf dst.git &&
1525 + test_when_finished "rm -rf dst.git" &&
1526 git init --bare dst.git &&
1527 (
1528 cd dst.git &&
@@ -1553,6 +1543,7 @@ test_expect_success 'fetch into bare respects core.logallrefupdates' '
1543 '
1544
1545 test_expect_success 'receive.denyCurrentBranch = updateInstead' '
1546 + mk_empty testrepo &&
1547 git push testrepo main &&
1548 (
1549 cd testrepo &&
@@ -1655,7 +1646,7 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' '
1646 ) &&
1647
1648 # (5) push into void
1658 - rm -fr void &&
1649 + test_when_finished "rm -rf void" &&
1650 git init void &&
1651 (
1652 cd void &&
@@ -1677,26 +1668,23 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' '
1668 '
1669
1670 test_expect_success 'updateInstead with push-to-checkout hook' '
1680 - rm -fr testrepo &&
1671 + test_when_finished "rm -rf testrepo" &&
1672 git init testrepo &&
1682 - (
1683 - cd testrepo &&
1684 - git pull .. main &&
1685 - git reset --hard HEAD^^ &&
1686 - git tag initial &&
1687 - git config receive.denyCurrentBranch updateInstead &&
1688 - write_script .git/hooks/push-to-checkout <<-\EOF
1689 - echo >&2 updating from $(git rev-parse HEAD)
1690 - echo >&2 updating to "$1"
1691 -
1692 - git update-index -q --refresh &&
1693 - git read-tree -u -m HEAD "$1" || {
1694 - status=$?
1695 - echo >&2 read-tree failed
1696 - exit $status
1697 - }
1698 - EOF
1699 - ) &&
1673 + git -C testrepo pull .. main &&
1674 + git -C testrepo reset --hard HEAD^^ &&
1675 + git -C testrepo tag initial &&
1676 + git -C testrepo config receive.denyCurrentBranch updateInstead &&
1677 + test_hook -C testrepo push-to-checkout <<-\EOF &&
1678 + echo >&2 updating from $(git rev-parse HEAD)
1679 + echo >&2 updating to "$1"
1680 +
1681 + git update-index -q --refresh &&
1682 + git read-tree -u -m HEAD "$1" || {
1683 + status=$?
1684 + echo >&2 read-tree failed
1685 + exit $status
1686 + }
1687 + EOF
1688
1689 # Try pushing into a pristine
1690 git push testrepo main &&
@@ -1739,35 +1727,32 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
1727 ) &&
1728
1729 # push into void
1742 - rm -fr void &&
1730 + test_when_finished "rm -rf void" &&
1731 git init void &&
1744 - (
1745 - cd void &&
1746 - git config receive.denyCurrentBranch updateInstead &&
1747 - write_script .git/hooks/push-to-checkout <<-\EOF
1748 - if git rev-parse --quiet --verify HEAD
1749 - then
1750 - has_head=yes
1751 - echo >&2 updating from $(git rev-parse HEAD)
1752 - else
1753 - has_head=no
1754 - echo >&2 pushing into void
1755 - fi
1756 - echo >&2 updating to "$1"
1757 -
1758 - git update-index -q --refresh &&
1759 - case "$has_head" in
1760 - yes)
1761 - git read-tree -u -m HEAD "$1" ;;
1762 - no)
1763 - git read-tree -u -m "$1" ;;
1764 - esac || {
1765 - status=$?
1766 - echo >&2 read-tree failed
1767 - exit $status
1768 - }
1769 - EOF
1770 - ) &&
1732 + git -C void config receive.denyCurrentBranch updateInstead &&
1733 + test_hook -C void push-to-checkout <<-\EOF &&
1734 + if git rev-parse --quiet --verify HEAD
1735 + then
1736 + has_head=yes
1737 + echo >&2 updating from $(git rev-parse HEAD)
1738 + else
1739 + has_head=no
1740 + echo >&2 pushing into void
1741 + fi
1742 + echo >&2 updating to "$1"
1743 +
1744 + git update-index -q --refresh &&
1745 + case "$has_head" in
1746 + yes)
1747 + git read-tree -u -m HEAD "$1" ;;
1748 + no)
1749 + git read-tree -u -m "$1" ;;
1750 + esac || {
1751 + status=$?
1752 + echo >&2 read-tree failed
1753 + exit $status
1754 + }
1755 + EOF
1756
1757 git push void main &&
1758 (