[PATCH v2 0/2] jx/fetch-atomic-error-message-fix
From: Jiang Xin <hidden>
Date: 2023-12-14 12:33:19
From: Jiang Xin <redacted>
# 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 [off-list ref]
## 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 [off-list ref]
## 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