From 8c85f83e66f7371b5499f8f49bb82ea1f0bd32fd Mon Sep 17 00:00:00 2001 From: Jiang Xin Date: Thu, 19 Oct 2023 22:34:32 +0800 Subject: [PATCH 1/2] t5574: test porcelain output of atomic fetch The test case "fetch porcelain output" checks output of the fetch command. The error output must be empty with the follow assertion: test_must_be_empty stderr Refactor this test case to run it twice. The first time will be run using non-atomic fetch and the other time will be run using atomic fetch. We can see that the above assertion fails for atomic get, as shown below: ok 5 - fetch porcelain output # TODO known breakage vanished not ok 6 - fetch porcelain output (atomic) # TODO known breakage The failed test case had an error message with only the error prompt but no message body, as follows: 'stderr' is not empty, it contains: error: In a later commit, we will fix this issue. Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano --- t/t5574-fetch-output.sh | 96 ++++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 39 deletions(-) diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh index 90e6dcb9a7..1397101629 100755 --- a/t/t5574-fetch-output.sh +++ b/t/t5574-fetch-output.sh @@ -61,9 +61,7 @@ test_expect_success 'fetch compact output' ' test_cmp expect actual ' -test_expect_success 'fetch porcelain output' ' - test_when_finished "rm -rf porcelain" && - +test_expect_success 'setup for fetch porcelain output' ' # Set up a bunch of references that we can use to demonstrate different # kinds of flag symbols in the output format. MAIN_OLD=$(git rev-parse HEAD) && @@ -74,15 +72,10 @@ test_expect_success 'fetch porcelain output' ' FORCE_UPDATED_OLD=$(git rev-parse HEAD) && git checkout main && - # Clone and pre-seed the repositories. We fetch references into two - # namespaces so that we can test that rejected and force-updated - # references are reported properly. - refspecs="refs/heads/*:refs/unforced/* +refs/heads/*:refs/forced/*" && - git clone . porcelain && - git -C porcelain fetch origin $refspecs && + # Backup to preseed.git + git clone --mirror . preseed.git && - # Now that we have set up the client repositories we can change our - # local references. + # Continue changing our local references. git branch new-branch && git branch -d deleted-branch && git checkout fast-forward && @@ -91,36 +84,61 @@ test_expect_success 'fetch porcelain output' ' git checkout force-updated && git reset --hard HEAD~ && test_commit --no-tag force-update-new && - FORCE_UPDATED_NEW=$(git rev-parse HEAD) && - - cat >expect <<-EOF && - - $MAIN_OLD $ZERO_OID refs/forced/deleted-branch - - $MAIN_OLD $ZERO_OID refs/unforced/deleted-branch - $MAIN_OLD $FAST_FORWARD_NEW refs/unforced/fast-forward - ! $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/unforced/force-updated - * $ZERO_OID $MAIN_OLD refs/unforced/new-branch - $MAIN_OLD $FAST_FORWARD_NEW refs/forced/fast-forward - + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/forced/force-updated - * $ZERO_OID $MAIN_OLD refs/forced/new-branch - $MAIN_OLD $FAST_FORWARD_NEW refs/remotes/origin/fast-forward - + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/remotes/origin/force-updated - * $ZERO_OID $MAIN_OLD refs/remotes/origin/new-branch - EOF - - # Execute a dry-run fetch first. We do this to assert that the dry-run - # and non-dry-run fetches produces the same output. Execution of the - # fetch is expected to fail as we have a rejected reference update. - test_must_fail git -C porcelain fetch \ - --porcelain --dry-run --prune origin $refspecs >actual && - test_cmp expect actual && - - # And now we perform a non-dry-run fetch. - test_must_fail git -C porcelain fetch \ - --porcelain --prune origin $refspecs >actual 2>stderr && - test_cmp expect actual && - test_must_be_empty stderr + FORCE_UPDATED_NEW=$(git rev-parse HEAD) ' +for opt in off on +do + case $opt in + on) + opt=--atomic + ;; + off) + opt= + ;; + esac + test_expect_failure "fetch porcelain output ${opt:+(atomic)}" ' + test_when_finished "rm -rf porcelain" && + + # Clone and pre-seed the repositories. We fetch references into two + # namespaces so that we can test that rejected and force-updated + # references are reported properly. + refspecs="refs/heads/*:refs/unforced/* +refs/heads/*:refs/forced/*" && + git clone preseed.git porcelain && + git -C porcelain fetch origin $opt $refspecs && + + cat >expect <<-EOF && + - $MAIN_OLD $ZERO_OID refs/forced/deleted-branch + - $MAIN_OLD $ZERO_OID refs/unforced/deleted-branch + $MAIN_OLD $FAST_FORWARD_NEW refs/unforced/fast-forward + ! $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/unforced/force-updated + * $ZERO_OID $MAIN_OLD refs/unforced/new-branch + $MAIN_OLD $FAST_FORWARD_NEW refs/forced/fast-forward + + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/forced/force-updated + * $ZERO_OID $MAIN_OLD refs/forced/new-branch + $MAIN_OLD $FAST_FORWARD_NEW refs/remotes/origin/fast-forward + + $FORCE_UPDATED_OLD $FORCE_UPDATED_NEW refs/remotes/origin/force-updated + * $ZERO_OID $MAIN_OLD refs/remotes/origin/new-branch + EOF + + # Change the URL of the repository to fetch different references. + git -C porcelain remote set-url origin .. && + + # Execute a dry-run fetch first. We do this to assert that the dry-run + # and non-dry-run fetches produces the same output. Execution of the + # fetch is expected to fail as we have a rejected reference update. + test_must_fail git -C porcelain fetch $opt \ + --porcelain --dry-run --prune origin $refspecs >actual && + test_cmp expect actual && + + # And now we perform a non-dry-run fetch. + test_must_fail git -C porcelain fetch $opt \ + --porcelain --prune origin $refspecs >actual 2>stderr && + test_cmp expect actual && + test_must_be_empty stderr + ' +done + test_expect_success 'fetch porcelain with multiple remotes' ' test_when_finished "rm -rf porcelain" && From d3184a9d0fb538d80bd7a18a893a704edce3bf98 Mon Sep 17 00:00:00 2001 From: Jiang Xin Date: Thu, 19 Oct 2023 22:34:33 +0800 Subject: [PATCH 2/2] fetch: no redundant error message for atomic fetch If an error occurs during an atomic fetch, a redundant error message will appear at the end of do_fetch(). It was introduced in b3a804663c (fetch: make `--atomic` flag cover backfilling of tags, 2022-02-17). Instead of displaying the error message unconditionally, the final error output should follow the pattern in update-ref.c and files-backend.c as follows: if (ref_transaction_abort(transaction, &error)) error("abort: %s", error.buf); This will fix the test case "fetch porcelain output (atomic)" in t5574. Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano --- builtin/fetch.c | 4 +--- t/t5574-fetch-output.sh | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index fd134ba74d..01a573cf8d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1775,10 +1775,8 @@ static int do_fetch(struct transport *transport, } cleanup: - if (retcode && transaction) { - ref_transaction_abort(transaction, &err); + if (retcode && transaction && ref_transaction_abort(transaction, &err)) error("%s", err.buf); - } display_state_release(&display_state); close_fetch_head(&fetch_head); diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh index 1397101629..3c72fc693f 100755 --- a/t/t5574-fetch-output.sh +++ b/t/t5574-fetch-output.sh @@ -97,7 +97,7 @@ do opt= ;; esac - test_expect_failure "fetch porcelain output ${opt:+(atomic)}" ' + test_expect_success "fetch porcelain output ${opt:+(atomic)}" ' test_when_finished "rm -rf porcelain" && # Clone and pre-seed the repositories. We fetch references into two