From: Jiang Xin <worldhello.net@gmail.com>
To: Git List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Patrick Steinhardt <ps@pks.im>
Cc: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: [PATCH v2 0/2] jx/fetch-atomic-error-message-fix
Date: Thu, 14 Dec 2023 20:33:10 +0800 [thread overview]
Message-ID: <cover.1702556642.git.zhiyou.jx@alibaba-inc.com> (raw)
In-Reply-To: <38b0b22038399265407f7fc5f126f471dcc6f1a3.1697725898.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
# Changes since v1:
1. Add a "test_commit ..." command in test case of t5574, so we can run
test cases 4-6 individually.
2. Improve commit logs.
# range-diff v1...v2
1: 8c85f83e66 ! 1: 210191917b t5574: test porcelain output of atomic fetch
@@ Commit message
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:
+ But this assertion fails if using atomic fetch. Refactor this test case
+ to use different fetch options by splitting it into three test cases.
+ 1. "setup for fetch porcelain output".
+
+ 2. "fetch porcelain output": for non-atomic fetch.
+
+ 3. "fetch porcelain output (atomic)": for atomic fetch.
+
+ Add new command "test_commit ..." in the first test case, so that if we
+ run these test cases individually (--run=4-6), "git rev-parse HEAD~"
+ command will work properly. Run the above test cases, we can find that
+ one test case has a known breakage, as shown below:
+
+ ok 4 - setup for fetch porcelain output
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
+ The failed test case has an error message with only the error prompt but
no message body, as follows:
'stderr' is not empty, it contains:
@@ Commit message
In a later commit, we will fix this issue.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
## t/t5574-fetch-output.sh ##
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output' '
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output' '
+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.
++ test_commit commit-for-porcelain-output &&
MAIN_OLD=$(git rev-parse HEAD) &&
+ git branch "fast-forward" &&
+ git branch "deleted-branch" &&
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch porcelain output' '
FORCE_UPDATED_OLD=$(git rev-parse HEAD) &&
git checkout main &&
2: d3184a9d0f ! 2: 6fb83a0000 fetch: no redundant error message for atomic fetch
@@ Commit 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:
+ In function do_fetch(), a failure message is already shown before the
+ retcode is set, so we should not call additional error() at the end of
+ this function.
+
+ We can remove the redundant error() function, because we know that
+ the function ref_transaction_abort() never fails. While we can find a
+ common pattern for calling ref_transaction_abort() by running command
+ "git grep -A1 ref_transaction_abort", e.g.:
if (ref_transaction_abort(transaction, &error))
error("abort: %s", error.buf);
- This will fix the test case "fetch porcelain output (atomic)" in t5574.
+ We can fix this issue follow this pattern, and the test case "fetch
+ porcelain output (atomic)" in t5574 will also be fixed. If in the future
+ we decide that we don't need to check the return value of the function
+ ref_transaction_abort(), this change can be fixed along with it.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
## builtin/fetch.c ##
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
Jiang Xin (2):
t5574: test porcelain output of atomic fetch
fetch: no redundant error message for atomic fetch
builtin/fetch.c | 4 +-
t/t5574-fetch-output.sh | 97 ++++++++++++++++++++++++-----------------
2 files changed, 59 insertions(+), 42 deletions(-)
--
2.41.0.232.g2f6f0bca4f.agit.8.0.4.dev
next prev parent reply other threads:[~2023-12-14 12:33 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-19 14:34 [PATCH 1/2] t5574: test porcelain output of atomic fetch Jiang Xin
2023-10-19 14:34 ` [PATCH 2/2] fetch: no redundant error message for " Jiang Xin
2023-10-23 8:27 ` Patrick Steinhardt
2023-10-23 9:16 ` Jiang Xin
2023-10-23 10:07 ` Patrick Steinhardt
2023-10-23 23:20 ` Jiang Xin
2023-10-25 8:21 ` Patrick Steinhardt
2023-10-24 18:16 ` Junio C Hamano
2023-12-14 12:33 ` Jiang Xin [this message]
2023-12-14 12:33 ` [PATCH v2 1/2] t5574: test porcelain output of " Jiang Xin
2023-12-15 9:56 ` Patrick Steinhardt
2023-12-15 11:16 ` Jiang Xin
2023-12-15 16:47 ` Junio C Hamano
2023-12-14 12:33 ` [PATCH v2 2/2] fetch: no redundant error message for " Jiang Xin
2023-12-15 9:56 ` Patrick Steinhardt
2023-12-17 14:11 ` [PATCH v3 0/2] fix fetch atomic error message Jiang Xin
2023-12-17 14:11 ` [PATCH v3 1/2] t5574: test porcelain output of atomic fetch Jiang Xin
2023-12-17 14:11 ` [PATCH v3 2/2] fetch: no redundant error message for " Jiang Xin
2023-12-18 8:14 ` [PATCH v3 0/2] fix fetch atomic error message Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1702556642.git.zhiyou.jx@alibaba-inc.com \
--to=worldhello.net@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ps@pks.im \
--cc=zhiyou.jx@alibaba-inc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).