* [PATCH] t5510-fetch: upgrade to a more modern style
@ 2022-03-31 17:54 Elia Pinto
2022-04-01 20:11 ` Junio C Hamano
2022-04-02 1:14 ` Ævar Arnfjörð Bjarmason
0 siblings, 2 replies; 4+ messages in thread
From: Elia Pinto @ 2022-03-31 17:54 UTC (permalink / raw)
To: git; +Cc: Elia Pinto
Clean up the code style so all the tests, and not just a few,
that chdir around isolate themselves in a subshell.
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
this patch was inspired by a Junio #leftoverbit
https://lore.kernel.org/git/xmqqmtjh0x5f.fsf@gitster.g/
t/t5510-fetch.sh | 927 ++++++++++++++++++++++++-----------------------
1 file changed, 477 insertions(+), 450 deletions(-)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 6f38a69fbb..d0b249d276 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -48,342 +48,349 @@ test_expect_success "clone and setup child repos" '
'
test_expect_success "fetch test" '
- cd "$D" &&
- echo >file updated by origin &&
- git commit -a -m "updated by origin" &&
- cd two &&
- git fetch &&
- git rev-parse --verify refs/heads/one &&
- mine=$(git rev-parse refs/heads/one) &&
- his=$(cd ../one && git rev-parse refs/heads/main) &&
- test "z$mine" = "z$his"
+ (
+ cd "$D" &&
+ echo >file updated by origin &&
+ git commit -a -m "updated by origin" &&
+ (
+ cd two &&
+ git fetch &&
+ git rev-parse --verify refs/heads/one &&
+ mine=$(git rev-parse refs/heads/one) &&
+ his=$(cd ../one && git rev-parse refs/heads/main) &&
+ test "z$mine" = "z$his"
+ )
+ )
'
test_expect_success "fetch test for-merge" '
- cd "$D" &&
- cd three &&
- git fetch &&
- git rev-parse --verify refs/heads/two &&
- git rev-parse --verify refs/heads/one &&
- main_in_two=$(cd ../two && git rev-parse main) &&
- one_in_two=$(cd ../two && git rev-parse one) &&
- {
- echo "$one_in_two " &&
- echo "$main_in_two not-for-merge"
- } >expected &&
- cut -f -2 .git/FETCH_HEAD >actual &&
- test_cmp expected actual'
+ (
+ cd "$D" &&
+ (
+ cd three &&
+ git fetch &&
+ git rev-parse --verify refs/heads/two &&
+ git rev-parse --verify refs/heads/one &&
+ main_in_two=$(cd ../two && git rev-parse main) &&
+ one_in_two=$(cd ../two && git rev-parse one) &&
+ {
+ echo "$one_in_two " &&
+ echo "$main_in_two not-for-merge"
+ } >expected &&
+ cut -f -2 .git/FETCH_HEAD >actual &&
+ test_cmp expected actual
+ )
+ )
+'
test_expect_success 'fetch --prune on its own works as expected' '
- cd "$D" &&
- git clone . prune &&
- cd prune &&
- git update-ref refs/remotes/origin/extrabranch main &&
-
- git fetch --prune origin &&
- test_must_fail git rev-parse origin/extrabranch
+ (
+ cd "$D" &&
+ git clone . prune &&
+ cd prune &&
+ git update-ref refs/remotes/origin/extrabranch main &&
+ git fetch --prune origin &&
+ test_must_fail git rev-parse origin/extrabranch
+ )
'
test_expect_success 'fetch --prune with a branch name keeps branches' '
- cd "$D" &&
- git clone . prune-branch &&
- cd prune-branch &&
- git update-ref refs/remotes/origin/extrabranch main &&
-
- git fetch --prune origin main &&
- git rev-parse origin/extrabranch
+ (
+ cd "$D" &&
+ git clone . prune-branch &&
+ cd prune-branch &&
+ git update-ref refs/remotes/origin/extrabranch main &&
+ git fetch --prune origin main &&
+ git rev-parse origin/extrabranch
+ )
'
test_expect_success 'fetch --prune with a namespace keeps other namespaces' '
- cd "$D" &&
- git clone . prune-namespace &&
- cd prune-namespace &&
-
- git fetch --prune origin refs/heads/a/*:refs/remotes/origin/a/* &&
- git rev-parse origin/main
+ (
+ cd "$D" &&
+ git clone . prune-namespace &&
+ cd prune-namespace &&
+ git fetch --prune origin refs/heads/a/*:refs/remotes/origin/a/* &&
+ git rev-parse origin/main
+ )
'
test_expect_success 'fetch --prune handles overlapping refspecs' '
- cd "$D" &&
- git update-ref refs/pull/42/head main &&
- git clone . prune-overlapping &&
- cd prune-overlapping &&
- git config --add remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
-
- git fetch --prune origin &&
- git rev-parse origin/main &&
- git rev-parse origin/pr/42 &&
-
- git config --unset-all remote.origin.fetch &&
- git config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
- git config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* &&
-
- git fetch --prune origin &&
- git rev-parse origin/main &&
- git rev-parse origin/pr/42
+ (
+ cd "$D" &&
+ git update-ref refs/pull/42/head main &&
+ git clone . prune-overlapping &&
+ cd prune-overlapping &&
+ git config --add remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
+ git fetch --prune origin &&
+ git rev-parse origin/main &&
+ git rev-parse origin/pr/42 &&
+ git config --unset-all remote.origin.fetch &&
+ git config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
+ git config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* &&
+ git fetch --prune origin &&
+ git rev-parse origin/main &&
+ git rev-parse origin/pr/42
+ )
'
test_expect_success 'fetch --prune --tags prunes branches but not tags' '
- cd "$D" &&
- git clone . prune-tags &&
- cd prune-tags &&
- git tag sometag main &&
- # Create what looks like a remote-tracking branch from an earlier
- # fetch that has since been deleted from the remote:
- git update-ref refs/remotes/origin/fake-remote main &&
-
- git fetch --prune --tags origin &&
- git rev-parse origin/main &&
- test_must_fail git rev-parse origin/fake-remote &&
- git rev-parse sometag
+ (
+ cd "$D" &&
+ git clone . prune-tags &&
+ cd prune-tags &&
+ git tag sometag main &&
+ # Create what looks like a remote-tracking branch from an earlier
+ # fetch that has since been deleted from the remote:
+ git update-ref refs/remotes/origin/fake-remote main &&
+ git fetch --prune --tags origin &&
+ git rev-parse origin/main &&
+ test_must_fail git rev-parse origin/fake-remote &&
+ git rev-parse sometag
+ )
'
test_expect_success 'fetch --prune --tags with branch does not prune other things' '
- cd "$D" &&
- git clone . prune-tags-branch &&
- cd prune-tags-branch &&
- git tag sometag main &&
- git update-ref refs/remotes/origin/extrabranch main &&
-
- git fetch --prune --tags origin main &&
- git rev-parse origin/extrabranch &&
- git rev-parse sometag
+ (
+ cd "$D" &&
+ git clone . prune-tags-branch &&
+ cd prune-tags-branch &&
+ git tag sometag main &&
+ git update-ref refs/remotes/origin/extrabranch main &&
+ git fetch --prune --tags origin main &&
+ git rev-parse origin/extrabranch &&
+ git rev-parse sometag
+ )
'
test_expect_success 'fetch --prune --tags with refspec prunes based on refspec' '
- cd "$D" &&
- git clone . prune-tags-refspec &&
- cd prune-tags-refspec &&
- git tag sometag main &&
- git update-ref refs/remotes/origin/foo/otherbranch main &&
- git update-ref refs/remotes/origin/extrabranch main &&
-
- git fetch --prune --tags origin refs/heads/foo/*:refs/remotes/origin/foo/* &&
- test_must_fail git rev-parse refs/remotes/origin/foo/otherbranch &&
- git rev-parse origin/extrabranch &&
- git rev-parse sometag
+ (
+ cd "$D" &&
+ git clone . prune-tags-refspec &&
+ cd prune-tags-refspec &&
+ git tag sometag main &&
+ git update-ref refs/remotes/origin/foo/otherbranch main &&
+ git update-ref refs/remotes/origin/extrabranch main &&
+ git fetch --prune --tags origin refs/heads/foo/*:refs/remotes/origin/foo/* &&
+ test_must_fail git rev-parse refs/remotes/origin/foo/otherbranch &&
+ git rev-parse origin/extrabranch &&
+ git rev-parse sometag
+ )
'
test_expect_success REFFILES 'fetch --prune fails to delete branches' '
- cd "$D" &&
- git clone . prune-fail &&
- cd prune-fail &&
- git update-ref refs/remotes/origin/extrabranch main &&
- : this will prevent --prune from locking packed-refs for deleting refs, but adding loose refs still succeeds &&
- >.git/packed-refs.new &&
-
- test_must_fail git fetch --prune origin
+ (
+ cd "$D" &&
+ git clone . prune-fail &&
+ cd prune-fail &&
+ git update-ref refs/remotes/origin/extrabranch main &&
+ : this will prevent --prune from locking packed-refs for deleting refs, but adding loose refs still succeeds &&
+ >.git/packed-refs.new &&
+ test_must_fail git fetch --prune origin
+ )
'
test_expect_success 'fetch --atomic works with a single branch' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git clone . atomic &&
- git branch atomic-branch &&
- oid=$(git rev-parse atomic-branch) &&
- echo "$oid" >expected &&
-
- git -C atomic fetch --atomic origin &&
- git -C atomic rev-parse origin/atomic-branch >actual &&
- test_cmp expected actual &&
- test $oid = "$(git -C atomic rev-parse --verify FETCH_HEAD)"
+ (
+ cd "$D" &&
+ git clone . atomic &&
+ git branch atomic-branch &&
+ oid=$(git rev-parse atomic-branch) &&
+ echo "$oid" >expected &&
+ git -C atomic fetch --atomic origin &&
+ git -C atomic rev-parse origin/atomic-branch >actual &&
+ test_cmp expected actual &&
+ test $oid = "$(git -C atomic rev-parse --verify FETCH_HEAD)"
+ )
'
test_expect_success 'fetch --atomic works with multiple branches' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git clone . atomic &&
- git branch atomic-branch-1 &&
- git branch atomic-branch-2 &&
- git branch atomic-branch-3 &&
- git rev-parse refs/heads/atomic-branch-1 refs/heads/atomic-branch-2 refs/heads/atomic-branch-3 >actual &&
-
- git -C atomic fetch --atomic origin &&
- git -C atomic rev-parse refs/remotes/origin/atomic-branch-1 refs/remotes/origin/atomic-branch-2 refs/remotes/origin/atomic-branch-3 >expected &&
- test_cmp expected actual
+ (
+ cd "$D" &&
+ git clone . atomic &&
+ git branch atomic-branch-1 &&
+ git branch atomic-branch-2 &&
+ git branch atomic-branch-3 &&
+ git rev-parse refs/heads/atomic-branch-1 refs/heads/atomic-branch-2 refs/heads/atomic-branch-3 >actual &&
+ git -C atomic fetch --atomic origin &&
+ git -C atomic rev-parse refs/remotes/origin/atomic-branch-1 refs/remotes/origin/atomic-branch-2 refs/remotes/origin/atomic-branch-3 >expected &&
+ test_cmp expected actual
+ )
'
test_expect_success 'fetch --atomic works with mixed branches and tags' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git clone . atomic &&
- git branch atomic-mixed-branch &&
- git tag atomic-mixed-tag &&
- git rev-parse refs/heads/atomic-mixed-branch refs/tags/atomic-mixed-tag >actual &&
-
- git -C atomic fetch --tags --atomic origin &&
- git -C atomic rev-parse refs/remotes/origin/atomic-mixed-branch refs/tags/atomic-mixed-tag >expected &&
- test_cmp expected actual
+ (
+ cd "$D" &&
+ git clone . atomic &&
+ git branch atomic-mixed-branch &&
+ git tag atomic-mixed-tag &&
+ git rev-parse refs/heads/atomic-mixed-branch refs/tags/atomic-mixed-tag >actual &&
+ git -C atomic fetch --tags --atomic origin &&
+ git -C atomic rev-parse refs/remotes/origin/atomic-mixed-branch refs/tags/atomic-mixed-tag >expected &&
+ test_cmp expected actual
+ )
'
test_expect_success 'fetch --atomic prunes references' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git branch atomic-prune-delete &&
- git clone . atomic &&
- git branch --delete atomic-prune-delete &&
- git branch atomic-prune-create &&
- git rev-parse refs/heads/atomic-prune-create >actual &&
-
- git -C atomic fetch --prune --atomic origin &&
- test_must_fail git -C atomic rev-parse refs/remotes/origin/atomic-prune-delete &&
- git -C atomic rev-parse refs/remotes/origin/atomic-prune-create >expected &&
- test_cmp expected actual
+ (
+ cd "$D" &&
+ git branch atomic-prune-delete &&
+ git clone . atomic &&
+ git branch --delete atomic-prune-delete &&
+ git branch atomic-prune-create &&
+ git rev-parse refs/heads/atomic-prune-create >actual &&
+ git -C atomic fetch --prune --atomic origin &&
+ test_must_fail git -C atomic rev-parse refs/remotes/origin/atomic-prune-delete &&
+ git -C atomic rev-parse refs/remotes/origin/atomic-prune-create >expected &&
+ test_cmp expected actual
+ )
'
test_expect_success 'fetch --atomic aborts with non-fast-forward update' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git branch atomic-non-ff &&
- git clone . atomic &&
- git rev-parse HEAD >actual &&
-
- git branch atomic-new-branch &&
- parent_commit=$(git rev-parse atomic-non-ff~) &&
- git update-ref refs/heads/atomic-non-ff $parent_commit &&
-
- test_must_fail git -C atomic fetch --atomic origin refs/heads/*:refs/remotes/origin/* &&
- test_must_fail git -C atomic rev-parse refs/remotes/origin/atomic-new-branch &&
- git -C atomic rev-parse refs/remotes/origin/atomic-non-ff >expected &&
- test_cmp expected actual &&
- test_must_be_empty atomic/.git/FETCH_HEAD
+ (
+ cd "$D" &&
+ git branch atomic-non-ff &&
+ git clone . atomic &&
+ git rev-parse HEAD >actual &&
+ git branch atomic-new-branch &&
+ parent_commit=$(git rev-parse atomic-non-ff~) &&
+ git update-ref refs/heads/atomic-non-ff $parent_commit &&
+ test_must_fail git -C atomic fetch --atomic origin refs/heads/*:refs/remotes/origin/* &&
+ test_must_fail git -C atomic rev-parse refs/remotes/origin/atomic-new-branch &&
+ git -C atomic rev-parse refs/remotes/origin/atomic-non-ff >expected &&
+ test_cmp expected actual &&
+ test_must_be_empty atomic/.git/FETCH_HEAD
+ )
'
test_expect_success 'fetch --atomic executes a single reference transaction only' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git clone . atomic &&
- git branch atomic-hooks-1 &&
- git branch atomic-hooks-2 &&
- head_oid=$(git rev-parse HEAD) &&
-
- cat >expected <<-EOF &&
- prepared
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
- committed
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
- EOF
-
- rm -f atomic/actual &&
- test_hook -C atomic reference-transaction <<-\EOF &&
- ( echo "$*" && cat ) >>actual
- EOF
-
- git -C atomic fetch --atomic origin &&
- test_cmp expected atomic/actual
+ (
+ cd "$D" &&
+ git clone . atomic &&
+ git branch atomic-hooks-1 &&
+ git branch atomic-hooks-2 &&
+ head_oid=$(git rev-parse HEAD) &&
+ cat >expected <<-EOF &&
+ prepared
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
+ committed
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
+ EOF
+ rm -f atomic/actual &&
+ write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+ ( echo "$*" && cat ) >>actual
+ EOF
+ git -C atomic fetch --atomic origin &&
+ test_cmp expected atomic/actual
+ )
'
test_expect_success 'fetch --atomic aborts all reference updates if hook aborts' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git clone . atomic &&
- git branch atomic-hooks-abort-1 &&
- git branch atomic-hooks-abort-2 &&
- git branch atomic-hooks-abort-3 &&
- git tag atomic-hooks-abort &&
- head_oid=$(git rev-parse HEAD) &&
-
- cat >expected <<-EOF &&
- prepared
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3
- $ZERO_OID $head_oid refs/tags/atomic-hooks-abort
- aborted
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2
- $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3
- $ZERO_OID $head_oid refs/tags/atomic-hooks-abort
- EOF
-
- rm -f atomic/actual &&
- test_hook -C atomic/.git reference-transaction <<-\EOF &&
- ( echo "$*" && cat ) >>actual
- exit 1
- EOF
-
- git -C atomic for-each-ref >expected-refs &&
- test_must_fail git -C atomic fetch --tags --atomic origin &&
- git -C atomic for-each-ref >actual-refs &&
- test_cmp expected-refs actual-refs &&
- test_must_be_empty atomic/.git/FETCH_HEAD
+ (
+ cd "$D" &&
+ git clone . atomic &&
+ git branch atomic-hooks-abort-1 &&
+ git branch atomic-hooks-abort-2 &&
+ git branch atomic-hooks-abort-3 &&
+ git tag atomic-hooks-abort &&
+ head_oid=$(git rev-parse HEAD) &&
+ cat >expected <<-EOF &&
+ prepared
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3
+ $ZERO_OID $head_oid refs/tags/atomic-hooks-abort
+ aborted
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-1
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-2
+ $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-abort-3
+ $ZERO_OID $head_oid refs/tags/atomic-hooks-abort
+ EOF
+ rm -f atomic/actual &&
+ write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+ ( echo "$*" && cat ) >>actual
+ exit 1
+ EOF
+ git -C atomic for-each-ref >expected-refs &&
+ test_must_fail git -C atomic fetch --tags --atomic origin &&
+ git -C atomic for-each-ref >actual-refs &&
+ test_cmp expected-refs actual-refs &&
+ test_must_be_empty atomic/.git/FETCH_HEAD
+ )
'
test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git clone . atomic &&
- oid=$(git rev-parse HEAD) &&
-
- git branch atomic-fetch-head-1 &&
- git -C atomic fetch --atomic origin atomic-fetch-head-1 &&
- test_line_count = 1 atomic/.git/FETCH_HEAD &&
-
- git branch atomic-fetch-head-2 &&
- git -C atomic fetch --atomic --append origin atomic-fetch-head-2 &&
- test_line_count = 2 atomic/.git/FETCH_HEAD &&
- cp atomic/.git/FETCH_HEAD expected &&
-
- test_hook -C atomic reference-transaction <<-\EOF &&
- exit 1
- EOF
-
- git branch atomic-fetch-head-3 &&
- test_must_fail git -C atomic fetch --atomic --append origin atomic-fetch-head-3 &&
- test_cmp expected atomic/.git/FETCH_HEAD
+ (
+ cd "$D" &&
+ git clone . atomic &&
+ oid=$(git rev-parse HEAD) &&
+ git branch atomic-fetch-head-1 &&
+ git -C atomic fetch --atomic origin atomic-fetch-head-1 &&
+ test_line_count = 1 atomic/.git/FETCH_HEAD &&
+ git branch atomic-fetch-head-2 &&
+ git -C atomic fetch --atomic --append origin atomic-fetch-head-2 &&
+ test_line_count = 2 atomic/.git/FETCH_HEAD &&
+ cp atomic/.git/FETCH_HEAD expected &&
+ write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+ exit 1
+ EOF
+ git branch atomic-fetch-head-3 &&
+ test_must_fail git -C atomic fetch --atomic --append origin atomic-fetch-head-3 &&
+ test_cmp expected atomic/.git/FETCH_HEAD
+ )
'
test_expect_success 'fetch --atomic --prune executes a single reference transaction only' '
test_when_finished "rm -rf \"$D\"/atomic" &&
-
- cd "$D" &&
- git branch scheduled-for-deletion &&
- git clone . atomic &&
- git branch -D scheduled-for-deletion &&
- git branch new-branch &&
- head_oid=$(git rev-parse HEAD) &&
-
- # Fetching with the `--atomic` flag should update all references in a
- # single transaction.
- cat >expected <<-EOF &&
- prepared
- $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
- $ZERO_OID $head_oid refs/remotes/origin/new-branch
- committed
- $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
- $ZERO_OID $head_oid refs/remotes/origin/new-branch
- EOF
-
- test_hook -C atomic reference-transaction <<-\EOF &&
- ( echo "$*" && cat ) >>actual
- EOF
-
- git -C atomic fetch --atomic --prune origin &&
- test_cmp expected atomic/actual
+ (
+ cd "$D" &&
+ git branch scheduled-for-deletion &&
+ git clone . atomic &&
+ git branch -D scheduled-for-deletion &&
+ git branch new-branch &&
+ head_oid=$(git rev-parse HEAD) &&
+ # Fetching with the `--atomic` flag should update all references in a
+ # single transaction.
+ cat >expected <<-EOF &&
+ prepared
+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
+ $ZERO_OID $head_oid refs/remotes/origin/new-branch
+ committed
+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
+ $ZERO_OID $head_oid refs/remotes/origin/new-branch
+ EOF
+ write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
+ ( echo "$*" && cat ) >>actual
+ EOF
+ git -C atomic fetch --atomic --prune origin &&
+ test_cmp expected atomic/actual
+ )
'
test_expect_success '--refmap="" ignores configured refspec' '
- cd "$TRASH_DIRECTORY" &&
- git clone "$D" remote-refs &&
- git -C remote-refs rev-parse remotes/origin/main >old &&
- git -C remote-refs update-ref refs/remotes/origin/main main~1 &&
- git -C remote-refs rev-parse remotes/origin/main >new &&
- git -C remote-refs fetch --refmap= origin "+refs/heads/*:refs/hidden/origin/*" &&
- git -C remote-refs rev-parse remotes/origin/main >actual &&
- test_cmp new actual &&
- git -C remote-refs fetch origin &&
- git -C remote-refs rev-parse remotes/origin/main >actual &&
- test_cmp old actual
+ (
+ cd "$TRASH_DIRECTORY" &&
+ git clone "$D" remote-refs &&
+ git -C remote-refs rev-parse remotes/origin/main >old &&
+ git -C remote-refs update-ref refs/remotes/origin/main main~1 &&
+ git -C remote-refs rev-parse remotes/origin/main >new &&
+ git -C remote-refs fetch --refmap= origin "+refs/heads/*:refs/hidden/origin/*" &&
+ git -C remote-refs rev-parse remotes/origin/main >actual &&
+ test_cmp new actual &&
+ git -C remote-refs fetch origin &&
+ git -C remote-refs rev-parse remotes/origin/main >actual &&
+ test_cmp old actual
+ )
'
test_expect_success '--refmap="" and --prune' '
@@ -397,89 +404,86 @@ test_expect_success '--refmap="" and --prune' '
'
test_expect_success 'fetch tags when there is no tags' '
-
- cd "$D" &&
-
- mkdir notags &&
- cd notags &&
- git init &&
-
- git fetch -t ..
-
+ (
+ cd "$D" &&
+ mkdir notags &&
+ cd notags &&
+ git init &&
+ git fetch -t ..
+ )
'
test_expect_success 'fetch following tags' '
-
- cd "$D" &&
- git tag -a -m "annotated" anno HEAD &&
- git tag light HEAD &&
-
- mkdir four &&
- cd four &&
- git init &&
-
- git fetch .. :track &&
- git show-ref --verify refs/tags/anno &&
- git show-ref --verify refs/tags/light
-
+ (
+ cd "$D" &&
+ git tag -a -m "annotated" anno HEAD &&
+ git tag light HEAD &&
+ mkdir four &&
+ cd four &&
+ git init &&
+ git fetch .. :track &&
+ git show-ref --verify refs/tags/anno &&
+ git show-ref --verify refs/tags/light
+ )
'
test_expect_success 'fetch uses remote ref names to describe new refs' '
- cd "$D" &&
- git init descriptive &&
- (
- cd descriptive &&
- git config remote.o.url .. &&
- git config remote.o.fetch "refs/heads/*:refs/crazyheads/*" &&
- git config --add remote.o.fetch "refs/others/*:refs/heads/*" &&
- git fetch o
- ) &&
- git tag -a -m "Descriptive tag" descriptive-tag &&
- git branch descriptive-branch &&
- git checkout descriptive-branch &&
- echo "Nuts" >crazy &&
- git add crazy &&
- git commit -a -m "descriptive commit" &&
- git update-ref refs/others/crazy HEAD &&
(
- cd descriptive &&
- git fetch o 2>actual &&
- test_i18ngrep "new branch.* -> refs/crazyheads/descriptive-branch$" actual &&
- test_i18ngrep "new tag.* -> descriptive-tag$" actual &&
- test_i18ngrep "new ref.* -> crazy$" actual
- ) &&
- git checkout main
+ cd "$D" &&
+ git init descriptive &&
+ (
+ cd descriptive &&
+ git config remote.o.url .. &&
+ git config remote.o.fetch "refs/heads/*:refs/crazyheads/*" &&
+ git config --add remote.o.fetch "refs/others/*:refs/heads/*" &&
+ git fetch o
+ ) &&
+ git tag -a -m "Descriptive tag" descriptive-tag &&
+ git branch descriptive-branch &&
+ git checkout descriptive-branch &&
+ echo "Nuts" >crazy &&
+ git add crazy &&
+ git commit -a -m "descriptive commit" &&
+ git update-ref refs/others/crazy HEAD &&
+ (
+ cd descriptive &&
+ git fetch o 2>actual &&
+ test_i18ngrep "new branch.* -> refs/crazyheads/descriptive-branch$" actual &&
+ test_i18ngrep "new tag.* -> descriptive-tag$" actual &&
+ test_i18ngrep "new ref.* -> crazy$" actual
+ ) &&
+ git checkout main
+ )
'
test_expect_success 'fetch must not resolve short tag name' '
-
- cd "$D" &&
-
- mkdir five &&
- cd five &&
- git init &&
-
- test_must_fail git fetch .. anno:five
-
+ (
+ cd "$D" &&
+ mkdir five &&
+ cd five &&
+ git init &&
+ test_must_fail git fetch .. anno:five
+ )
'
test_expect_success 'fetch can now resolve short remote name' '
-
- cd "$D" &&
- git update-ref refs/remotes/six/HEAD HEAD &&
-
- mkdir six &&
- cd six &&
- git init &&
-
- git fetch .. six:six
+ (
+ cd "$D" &&
+ git update-ref refs/remotes/six/HEAD HEAD &&
+ mkdir six &&
+ cd six &&
+ git init &&
+ git fetch .. six:six
+ )
'
test_expect_success 'create bundle 1' '
- cd "$D" &&
- echo >file updated again by origin &&
- git commit -a -m "tip" &&
- git bundle create --version=3 bundle1 main^..main
+ (
+ cd "$D" &&
+ echo >file updated again by origin &&
+ git commit -a -m "tip" &&
+ git bundle create --version=3 bundle1 main^..main
+ )
'
test_expect_success 'header of bundle looks right' '
@@ -495,43 +499,53 @@ test_expect_success 'header of bundle looks right' '
'
test_expect_success 'create bundle 2' '
- cd "$D" &&
- git bundle create bundle2 main~2..main
+ (
+ cd "$D" &&
+ git bundle create bundle2 main~2..main
+ )
'
test_expect_success 'unbundle 1' '
- cd "$D/bundle" &&
- git checkout -b some-branch &&
- test_must_fail git fetch "$D/bundle1" main:main
+ (
+ cd "$D/bundle" &&
+ git checkout -b some-branch &&
+ test_must_fail git fetch "$D/bundle1" main:main
+ )
'
test_expect_success 'bundle 1 has only 3 files ' '
- cd "$D" &&
- test_bundle_object_count bundle1 3
+ (
+ cd "$D" &&
+ test_bundle_object_count bundle1 3
+ )
'
test_expect_success 'unbundle 2' '
- cd "$D/bundle" &&
- git fetch ../bundle2 main:main &&
- test "tip" = "$(git log -1 --pretty=oneline main | cut -d" " -f2)"
+ (
+ cd "$D/bundle" &&
+ git fetch ../bundle2 main:main &&
+ test "tip" = "$(git log -1 --pretty=oneline main | cut -d" " -f2)"
+ )
'
test_expect_success 'bundle does not prerequisite objects' '
- cd "$D" &&
- touch file2 &&
- git add file2 &&
- git commit -m add.file2 file2 &&
- git bundle create bundle3 -1 HEAD &&
- test_bundle_object_count bundle3 3
+ (
+ cd "$D" &&
+ touch file2 &&
+ git add file2 &&
+ git commit -m add.file2 file2 &&
+ git bundle create bundle3 -1 HEAD &&
+ test_bundle_object_count bundle3 3
+ )
'
test_expect_success 'bundle should be able to create a full history' '
-
- cd "$D" &&
- git tag -a -m "1.0" v1.0 main &&
- git bundle create bundle4 v1.0
-
+ (
+ cd "$D" &&
+ git tag -a -m "1.0" v1.0 main &&
+ git bundle create bundle4 v1.0
+ )
'
test_expect_success 'fetch with a non-applying branch.<name>.merge' '
@@ -581,15 +595,16 @@ test_expect_success 'quoting of a strangely named repo' '
'
test_expect_success 'bundle should record HEAD correctly' '
-
- cd "$D" &&
- git bundle create bundle5 HEAD main &&
- git bundle list-heads bundle5 >actual &&
- for h in HEAD refs/heads/main
- do
- echo "$(git rev-parse --verify $h) $h" || return 1
- done >expect &&
- test_cmp expect actual
+ (
+ cd "$D" &&
+ git bundle create bundle5 HEAD main &&
+ git bundle list-heads bundle5 >actual &&
+ for h in HEAD refs/heads/main
+ do
+ echo "$(git rev-parse --verify $h) $h" || return 1
+ done >expect &&
+ test_cmp expect actual
+ )
'
@@ -601,127 +616,139 @@ test_expect_success 'mark initial state of origin/main' '
'
test_expect_success 'explicit fetch should update tracking' '
-
- cd "$D" &&
- git branch -f side &&
(
- cd three &&
- git update-ref refs/remotes/origin/main base-origin-main &&
- o=$(git rev-parse --verify refs/remotes/origin/main) &&
- git fetch origin main &&
- n=$(git rev-parse --verify refs/remotes/origin/main) &&
- test "$o" != "$n" &&
- test_must_fail git rev-parse --verify refs/remotes/origin/side
+ cd "$D" &&
+ git branch -f side &&
+ (
+ cd three &&
+ git update-ref refs/remotes/origin/main base-origin-main &&
+ o=$(git rev-parse --verify refs/remotes/origin/main) &&
+ git fetch origin main &&
+ n=$(git rev-parse --verify refs/remotes/origin/main) &&
+ test "$o" != "$n" &&
+ test_must_fail git rev-parse --verify refs/remotes/origin/side
+ )
)
'
test_expect_success 'explicit pull should update tracking' '
-
- cd "$D" &&
- git branch -f side &&
(
- cd three &&
- git update-ref refs/remotes/origin/main base-origin-main &&
- o=$(git rev-parse --verify refs/remotes/origin/main) &&
- git pull origin main &&
- n=$(git rev-parse --verify refs/remotes/origin/main) &&
- test "$o" != "$n" &&
- test_must_fail git rev-parse --verify refs/remotes/origin/side
+ cd "$D" &&
+ git branch -f side &&
+ (
+ cd three &&
+ git update-ref refs/remotes/origin/main base-origin-main &&
+ o=$(git rev-parse --verify refs/remotes/origin/main) &&
+ git pull origin main &&
+ n=$(git rev-parse --verify refs/remotes/origin/main) &&
+ test "$o" != "$n" &&
+ test_must_fail git rev-parse --verify refs/remotes/origin/side
+ )
)
'
test_expect_success 'explicit --refmap is allowed only with command-line refspec' '
- cd "$D" &&
(
- cd three &&
- test_must_fail git fetch --refmap="*:refs/remotes/none/*"
+ cd "$D" &&
+ (
+ cd three &&
+ test_must_fail git fetch --refmap="*:refs/remotes/none/*"
+ )
)
'
test_expect_success 'explicit --refmap option overrides remote.*.fetch' '
- cd "$D" &&
- git branch -f side &&
(
- cd three &&
- git update-ref refs/remotes/origin/main base-origin-main &&
- o=$(git rev-parse --verify refs/remotes/origin/main) &&
- git fetch --refmap="refs/heads/*:refs/remotes/other/*" origin main &&
- n=$(git rev-parse --verify refs/remotes/origin/main) &&
- test "$o" = "$n" &&
- test_must_fail git rev-parse --verify refs/remotes/origin/side &&
- git rev-parse --verify refs/remotes/other/main
+ cd "$D" &&
+ git branch -f side &&
+ (
+ cd three &&
+ git update-ref refs/remotes/origin/main base-origin-main &&
+ o=$(git rev-parse --verify refs/remotes/origin/main) &&
+ git fetch --refmap="refs/heads/*:refs/remotes/other/*" origin main &&
+ n=$(git rev-parse --verify refs/remotes/origin/main) &&
+ test "$o" = "$n" &&
+ test_must_fail git rev-parse --verify refs/remotes/origin/side &&
+ git rev-parse --verify refs/remotes/other/main
+ )
)
'
test_expect_success 'explicitly empty --refmap option disables remote.*.fetch' '
- cd "$D" &&
- git branch -f side &&
(
- cd three &&
- git update-ref refs/remotes/origin/main base-origin-main &&
- o=$(git rev-parse --verify refs/remotes/origin/main) &&
- git fetch --refmap="" origin main &&
- n=$(git rev-parse --verify refs/remotes/origin/main) &&
- test "$o" = "$n" &&
- test_must_fail git rev-parse --verify refs/remotes/origin/side
+ cd "$D" &&
+ git branch -f side &&
+ (
+ cd three &&
+ git update-ref refs/remotes/origin/main base-origin-main &&
+ o=$(git rev-parse --verify refs/remotes/origin/main) &&
+ git fetch --refmap="" origin main &&
+ n=$(git rev-parse --verify refs/remotes/origin/main) &&
+ test "$o" = "$n" &&
+ test_must_fail git rev-parse --verify refs/remotes/origin/side
+ )
)
'
test_expect_success 'configured fetch updates tracking' '
-
- cd "$D" &&
- git branch -f side &&
(
- cd three &&
- git update-ref refs/remotes/origin/main base-origin-main &&
- o=$(git rev-parse --verify refs/remotes/origin/main) &&
- git fetch origin &&
- n=$(git rev-parse --verify refs/remotes/origin/main) &&
- test "$o" != "$n" &&
- git rev-parse --verify refs/remotes/origin/side
+ cd "$D" &&
+ git branch -f side &&
+ (
+ cd three &&
+ git update-ref refs/remotes/origin/main base-origin-main &&
+ o=$(git rev-parse --verify refs/remotes/origin/main) &&
+ git fetch origin &&
+ n=$(git rev-parse --verify refs/remotes/origin/main) &&
+ test "$o" != "$n" &&
+ git rev-parse --verify refs/remotes/origin/side
+ )
)
'
test_expect_success 'non-matching refspecs do not confuse tracking update' '
- cd "$D" &&
- git update-ref refs/odd/location HEAD &&
(
- cd three &&
- git update-ref refs/remotes/origin/main base-origin-main &&
- git config --add remote.origin.fetch \
- refs/odd/location:refs/remotes/origin/odd &&
- o=$(git rev-parse --verify refs/remotes/origin/main) &&
- git fetch origin main &&
- n=$(git rev-parse --verify refs/remotes/origin/main) &&
- test "$o" != "$n" &&
- test_must_fail git rev-parse --verify refs/remotes/origin/odd
+ cd "$D" &&
+ git update-ref refs/odd/location HEAD &&
+ (
+ cd three &&
+ git update-ref refs/remotes/origin/main base-origin-main &&
+ git config --add remote.origin.fetch \
+ refs/odd/location:refs/remotes/origin/odd &&
+ o=$(git rev-parse --verify refs/remotes/origin/main) &&
+ git fetch origin main &&
+ n=$(git rev-parse --verify refs/remotes/origin/main) &&
+ test "$o" != "$n" &&
+ test_must_fail git rev-parse --verify refs/remotes/origin/odd
+ )
)
'
test_expect_success 'pushing nonexistent branch by mistake should not segv' '
-
- cd "$D" &&
- test_must_fail git push seven no:no
-
+ (
+ cd "$D" &&
+ test_must_fail git push seven no:no
+ )
'
test_expect_success 'auto tag following fetches minimum' '
-
- cd "$D" &&
- git clone .git follow &&
- git checkout HEAD^0 &&
(
- for i in 1 2 3 4 5 6 7
- do
- echo $i >>file &&
- git commit -m $i -a &&
- git tag -a -m $i excess-$i || exit 1
- done
- ) &&
- git checkout main &&
- (
- cd follow &&
- git fetch
+ cd "$D" &&
+ git clone .git follow &&
+ git checkout HEAD^0 &&
+ (
+ for i in 1 2 3 4 5 6 7
+ do
+ echo $i >>file &&
+ git commit -m $i -a &&
+ git tag -a -m $i excess-$i || exit 1
+ done
+ ) &&
+ git checkout main &&
+ (
+ cd follow &&
+ git fetch
+ )
)
'
--
2.35.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] t5510-fetch: upgrade to a more modern style
2022-03-31 17:54 [PATCH] t5510-fetch: upgrade to a more modern style Elia Pinto
@ 2022-04-01 20:11 ` Junio C Hamano
2022-04-02 7:32 ` Elia Pinto
2022-04-02 1:14 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2022-04-01 20:11 UTC (permalink / raw)
To: Elia Pinto; +Cc: git
Elia Pinto <gitter.spiros@gmail.com> writes:
> Clean up the code style so all the tests, and not just a few,
> that chdir around isolate themselves in a subshell.
>
> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
> this patch was inspired by a Junio #leftoverbit
> https://lore.kernel.org/git/xmqqmtjh0x5f.fsf@gitster.g/
> t/t5510-fetch.sh | 927 ++++++++++++++++++++++++-----------------------
> 1 file changed, 477 insertions(+), 450 deletions(-)
>
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index 6f38a69fbb..d0b249d276 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -48,342 +48,349 @@ test_expect_success "clone and setup child repos" '
> '
>
> test_expect_success "fetch test" '
> - cd "$D" &&
> - echo >file updated by origin &&
> - git commit -a -m "updated by origin" &&
> - cd two &&
> - git fetch &&
> - git rev-parse --verify refs/heads/one &&
> - mine=$(git rev-parse refs/heads/one) &&
> - his=$(cd ../one && git rev-parse refs/heads/main) &&
> - test "z$mine" = "z$his"
> + (
> + cd "$D" &&
> + echo >file updated by origin &&
> + git commit -a -m "updated by origin" &&
> + (
> + cd two &&
> + git fetch &&
> + git rev-parse --verify refs/heads/one &&
> + mine=$(git rev-parse refs/heads/one) &&
> + his=$(cd ../one && git rev-parse refs/heads/main) &&
> + test "z$mine" = "z$his"
> + )
> + )
> '
I think the idea of the "first unconditionally go to $D and then do
these things" pattern was that these tests anticipate that the step
before them will leave the process in an unexpected directory when
they begin. If the original version of this test fails when we
created the first commit "updated by origin", the next test piece
will start in "$D" directory, and if we successfully run it to the
end, the next test piece will start in "$D/ two".
Now, the point of this patch is to make sure each test piece will
not chdir around by isolating the parts that run in different
directories inside subshells. The purpose of doing so is? It is to
relieve later tests from having to worry about "going back to the
known starting place".
So, it is dubious that we want the subshell around the whole thing,
whose first command is to go to "$D", after we apply this patch.
Removal of that part was the primary reason why we are writing this
patch.
So I'd expect that the above test piece would become more like
in "git diff -w" output.
It is important to notice that the reason why we had 'cd "$D"' in
this test is *not* because the previous test has chdir'ed around,
but to look similar to later tests in the series.
Thanks.
diff --git c/t/t5510-fetch.sh w/t/t5510-fetch.sh
index 6f38a69fbb..1ed27607e2 100755
--- c/t/t5510-fetch.sh
+++ w/t/t5510-fetch.sh
@@ -48,15 +48,16 @@ test_expect_success "clone and setup child repos" '
'
test_expect_success "fetch test" '
- cd "$D" &&
echo >file updated by origin &&
git commit -a -m "updated by origin" &&
+ (
cd two &&
git fetch &&
git rev-parse --verify refs/heads/one &&
mine=$(git rev-parse refs/heads/one) &&
his=$(cd ../one && git rev-parse refs/heads/main) &&
test "z$mine" = "z$his"
+ )
'
test_expect_success "fetch test for-merge" '
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] t5510-fetch: upgrade to a more modern style
2022-04-01 20:11 ` Junio C Hamano
@ 2022-04-02 7:32 ` Elia Pinto
0 siblings, 0 replies; 4+ messages in thread
From: Elia Pinto @ 2022-04-02 7:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Il giorno ven 1 apr 2022 alle ore 22:11 Junio C Hamano
<gitster@pobox.com> ha scritto:
>
> Elia Pinto <gitter.spiros@gmail.com> writes:
>
> > Clean up the code style so all the tests, and not just a few,
> > that chdir around isolate themselves in a subshell.
> >
> > Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> > ---
> > this patch was inspired by a Junio #leftoverbit
> > https://lore.kernel.org/git/xmqqmtjh0x5f.fsf@gitster.g/
> > t/t5510-fetch.sh | 927 ++++++++++++++++++++++++-----------------------
> > 1 file changed, 477 insertions(+), 450 deletions(-)
> >
> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> > index 6f38a69fbb..d0b249d276 100755
> > --- a/t/t5510-fetch.sh
> > +++ b/t/t5510-fetch.sh
> > @@ -48,342 +48,349 @@ test_expect_success "clone and setup child repos" '
> > '
> >
> > test_expect_success "fetch test" '
> > - cd "$D" &&
> > - echo >file updated by origin &&
> > - git commit -a -m "updated by origin" &&
> > - cd two &&
> > - git fetch &&
> > - git rev-parse --verify refs/heads/one &&
> > - mine=$(git rev-parse refs/heads/one) &&
> > - his=$(cd ../one && git rev-parse refs/heads/main) &&
> > - test "z$mine" = "z$his"
> > + (
> > + cd "$D" &&
> > + echo >file updated by origin &&
> > + git commit -a -m "updated by origin" &&
> > + (
> > + cd two &&
> > + git fetch &&
> > + git rev-parse --verify refs/heads/one &&
> > + mine=$(git rev-parse refs/heads/one) &&
> > + his=$(cd ../one && git rev-parse refs/heads/main) &&
> > + test "z$mine" = "z$his"
> > + )
> > + )
> > '
>
> I think the idea of the "first unconditionally go to $D and then do
> these things" pattern was that these tests anticipate that the step
> before them will leave the process in an unexpected directory when
> they begin. If the original version of this test fails when we
> created the first commit "updated by origin", the next test piece
> will start in "$D" directory, and if we successfully run it to the
> end, the next test piece will start in "$D/ two".
>
> Now, the point of this patch is to make sure each test piece will
> not chdir around by isolating the parts that run in different
> directories inside subshells. The purpose of doing so is? It is to
> relieve later tests from having to worry about "going back to the
> known starting place".
>
> So, it is dubious that we want the subshell around the whole thing,
> whose first command is to go to "$D", after we apply this patch.
> Removal of that part was the primary reason why we are writing this
> patch.
That is fine. I had misinterpreted your answer here
https://lore.kernel.org/git/xmqqmtjh0x5f.fsf@gitster.g/
So the right thing to do is to leave "cd $ D" not in a subshell while
the actual test yes.
But at this point "cd $D" should be removed from subsequent tests, as
it is redundant.
In fact, if the first test of the chain fails, all the others would
always execute
with the current directory equal to $D for sure.
>
> So I'd expect that the above test piece would become more like
> in "git diff -w" output.
>
> It is important to notice that the reason why we had 'cd "$D"' in
> this test is *not* because the previous test has chdir'ed around,
> but to look similar to later tests in the series.
>
> Thanks.
>
>
> diff --git c/t/t5510-fetch.sh w/t/t5510-fetch.sh
> index 6f38a69fbb..1ed27607e2 100755
> --- c/t/t5510-fetch.sh
> +++ w/t/t5510-fetch.sh
> @@ -48,15 +48,16 @@ test_expect_success "clone and setup child repos" '
> '
>
> test_expect_success "fetch test" '
> - cd "$D" &&
> echo >file updated by origin &&
> git commit -a -m "updated by origin" &&
> + (
> cd two &&
> git fetch &&
> git rev-parse --verify refs/heads/one &&
> mine=$(git rev-parse refs/heads/one) &&
> his=$(cd ../one && git rev-parse refs/heads/main) &&
> test "z$mine" = "z$his"
> + )
> '
>
> test_expect_success "fetch test for-merge" '
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] t5510-fetch: upgrade to a more modern style
2022-03-31 17:54 [PATCH] t5510-fetch: upgrade to a more modern style Elia Pinto
2022-04-01 20:11 ` Junio C Hamano
@ 2022-04-02 1:14 ` Ævar Arnfjörð Bjarmason
1 sibling, 0 replies; 4+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-04-02 1:14 UTC (permalink / raw)
To: Elia Pinto; +Cc: git
On Thu, Mar 31 2022, Elia Pinto wrote:
> Clean up the code style so all the tests, and not just a few,
> that chdir around isolate themselves in a subshell.
Sounds sensible.
> test_expect_success "fetch test" '
> - cd "$D" &&
> - echo >file updated by origin &&
> - git commit -a -m "updated by origin" &&
> - cd two &&
> - git fetch &&
> - git rev-parse --verify refs/heads/one &&
> - mine=$(git rev-parse refs/heads/one) &&
> - his=$(cd ../one && git rev-parse refs/heads/main) &&
> - test "z$mine" = "z$his"
> + (
> + cd "$D" &&
> + echo >file updated by origin &&
> + git commit -a -m "updated by origin" &&
> + (
> + cd two &&
Why the two levels of subshelling though? We don't need a new one every
time we change directories, or do we?
The point is usually to avoid cd-ing in our main shell, not that each
level needs a new shell & indentation...
> - test_cmp expected actual'
> + (
> + cd "$D" &&
> + (
> + cd three &&
ditto..
> + git fetch &&
> + git rev-parse --verify refs/heads/two &&
> + git rev-parse --verify refs/heads/one &&
FWIW an alternative here is to use git -C "$D/three", but that may end
up being too verbose..
> test_expect_success 'fetch --prune handles overlapping refspecs' '
> - cd "$D" &&
> - git update-ref refs/pull/42/head main &&
> - git clone . prune-overlapping &&
> - cd prune-overlapping &&
> - git config --add remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
> -
> - git fetch --prune origin &&
> - git rev-parse origin/main &&
> - git rev-parse origin/pr/42 &&
> -
> - git config --unset-all remote.origin.fetch &&
> - git config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
> - git config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* &&
> -
> - git fetch --prune origin &&
> - git rev-parse origin/main &&
> - git rev-parse origin/pr/42
> + (
> + cd "$D" &&
> + git update-ref refs/pull/42/head main &&
> + git clone . prune-overlapping &&
> + cd prune-overlapping &&
> + git config --add remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
> + git fetch --prune origin &&
> + git rev-parse origin/main &&
> + git rev-parse origin/pr/42 &&
> + git config --unset-all remote.origin.fetch &&
> + git config remote.origin.fetch refs/pull/*/head:refs/remotes/origin/pr/* &&
> + git config --add remote.origin.fetch refs/heads/*:refs/remotes/origin/* &&
> + git fetch --prune origin &&
> + git rev-parse origin/main &&
> + git rev-parse origin/pr/42
> + )
> '
Please don't lose grouping whitespace while at it. I.e. the pre-image
intentionally splits "steps" by \n\n.
>
> test_expect_success 'fetch --prune --tags prunes branches but not tags' '
> - cd "$D" &&
> - git clone . prune-tags &&
> - cd prune-tags &&
> - git tag sometag main &&
> - # Create what looks like a remote-tracking branch from an earlier
> - # fetch that has since been deleted from the remote:
> - git update-ref refs/remotes/origin/fake-remote main &&
> -
> - git fetch --prune --tags origin &&
> - git rev-parse origin/main &&
> - test_must_fail git rev-parse origin/fake-remote &&
> - git rev-parse sometag
> + (
> + cd "$D" &&
> + git clone . prune-tags &&
> + cd prune-tags &&
> + git tag sometag main &&
> + # Create what looks like a remote-tracking branch from an earlier
> + # fetch that has since been deleted from the remote:
> + git update-ref refs/remotes/origin/fake-remote main &&
> + git fetch --prune --tags origin &&
> + git rev-parse origin/main &&
> + test_must_fail git rev-parse origin/fake-remote &&
> + git rev-parse sometag
> + )
> '
>
> test_expect_success 'fetch --prune --tags with branch does not prune other things' '
> - cd "$D" &&
> - git clone . prune-tags-branch &&
> - cd prune-tags-branch &&
> - git tag sometag main &&
> - git update-ref refs/remotes/origin/extrabranch main &&
> -
> - git fetch --prune --tags origin main &&
> - git rev-parse origin/extrabranch &&
> - git rev-parse sometag
> + (
> + cd "$D" &&
> + git clone . prune-tags-branch &&
> + cd prune-tags-branch &&
> + git tag sometag main &&
> + git update-ref refs/remotes/origin/extrabranch main &&
> + git fetch --prune --tags origin main &&
> + git rev-parse origin/extrabranch &&
> + git rev-parse sometag
> + )
> '
Skimming these these seem like much of the same code over & over again
with tiny variations. Perhaps even better would be splitting much of
this into a helper function(s)?
> - git -C atomic fetch --atomic origin &&
> - git -C atomic rev-parse origin/atomic-branch >actual &&
> - test_cmp expected actual &&
> - test $oid = "$(git -C atomic rev-parse --verify FETCH_HEAD)"
> + (
> + cd "$D" &&
> + git clone . atomic &&
> + git branch atomic-branch &&
> + oid=$(git rev-parse atomic-branch) &&
> + echo "$oid" >expected &&
> + git -C atomic fetch --atomic origin &&
> + git -C atomic rev-parse origin/atomic-branch >actual &&
> + test_cmp expected actual &&
> + test $oid = "$(git -C atomic rev-parse --verify FETCH_HEAD)"
speaking of modern style, perhaps it's worth it to fix these exit code
hiding issues? I.e. use test_cmp, test_cmp_rev etc.
> + head_oid=$(git rev-parse HEAD) &&
> + cat >expected <<-EOF &&
> + prepared
> + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
> + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
> + committed
> + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-1
> + $ZERO_OID $head_oid refs/remotes/origin/atomic-hooks-2
> + EOF
There was a discussion on-list the other day about how this particular
here-doc style is the odd one out, and we'd prefer the content aligned
with the "cat".
So if we're re-indenting all of these that would be a nice change while
we're at it, particularly as it would make the diff smaller, they'd
already be at the "right" indent level.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-04-02 7:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 17:54 [PATCH] t5510-fetch: upgrade to a more modern style Elia Pinto
2022-04-01 20:11 ` Junio C Hamano
2022-04-02 7:32 ` Elia Pinto
2022-04-02 1:14 ` Ævar Arnfjörð Bjarmason
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.