* [PATCH v2] t/pack-refs-tests: use test_path_is_missing
@ 2026-03-14 6:05 Ritesh Singh Jadoun
2026-03-14 16:39 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Ritesh Singh Jadoun @ 2026-03-14 6:05 UTC (permalink / raw)
To: git; +Cc: gitster, Ritesh Singh Jadoun
The pack-refs tests currently use raw 'test -f' checks with negation.
Update them to use Git's standard helper function test_path_is_missing
for consistency and clearer failure reporting. This aligns with
CodingGuidelines and makes test failures more obvious.
Signed-off-by: Ritesh Singh Jadoun <riteshjd75@gmail.com>
---
t/pack-refs-tests.sh | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
index 2fdaccb6c7..3cc4906f05 100644
--- a/t/pack-refs-tests.sh
+++ b/t/pack-refs-tests.sh
@@ -61,13 +61,13 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune
test_expect_success 'see if git ${pack_refs} --prune remove ref files' '
git branch f &&
git ${pack_refs} --all --prune &&
- ! test -f .git/refs/heads/f
+ test_path_is_missing .git/refs/heads/f
'
test_expect_success 'see if git ${pack_refs} --prune removes empty dirs' '
git branch r/s/t &&
git ${pack_refs} --all --prune &&
- ! test -e .git/refs/heads/r
+ ! test_path_exists .git/refs/heads/r
'
test_expect_success 'git branch g should work when git branch g/h has been deleted' '
@@ -111,43 +111,43 @@ test_expect_success 'test excluded refs are not packed' '
git branch dont_pack2 &&
git branch pack_this &&
git ${pack_refs} --all --exclude "refs/heads/dont_pack*" &&
- test -f .git/refs/heads/dont_pack1 &&
- test -f .git/refs/heads/dont_pack2 &&
- ! test -f .git/refs/heads/pack_this'
+ test_path_is_file .git/refs/heads/dont_pack1 &&
+ test_path_is_file .git/refs/heads/dont_pack2 &&
+ test_path_is_missing .git/refs/heads/pack_this'
test_expect_success 'test --no-exclude refs clears excluded refs' '
git branch dont_pack3 &&
git branch dont_pack4 &&
git ${pack_refs} --all --exclude "refs/heads/dont_pack*" --no-exclude &&
- ! test -f .git/refs/heads/dont_pack3 &&
- ! test -f .git/refs/heads/dont_pack4'
+ test_path_is_missing .git/refs/heads/dont_pack3 &&
+ test_path_is_missing .git/refs/heads/dont_pack4'
test_expect_success 'test only included refs are packed' '
git branch pack_this1 &&
git branch pack_this2 &&
git tag dont_pack5 &&
git ${pack_refs} --include "refs/heads/pack_this*" &&
- test -f .git/refs/tags/dont_pack5 &&
- ! test -f .git/refs/heads/pack_this1 &&
- ! test -f .git/refs/heads/pack_this2'
+ test_path_is_file .git/refs/tags/dont_pack5 &&
+ test_path_is_missing .git/refs/heads/pack_this1 &&
+ test_path_is_missing .git/refs/heads/pack_this2'
test_expect_success 'test --no-include refs clears included refs' '
git branch pack1 &&
git branch pack2 &&
git ${pack_refs} --include "refs/heads/pack*" --no-include &&
- test -f .git/refs/heads/pack1 &&
- test -f .git/refs/heads/pack2'
+ test_path_is_file .git/refs/heads/pack1 &&
+ test_path_is_file .git/refs/heads/pack2'
test_expect_success 'test --exclude takes precedence over --include' '
git branch dont_pack5 &&
git ${pack_refs} --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
- test -f .git/refs/heads/dont_pack5'
+ test_path_is_file .git/refs/heads/dont_pack5'
test_expect_success 'see if up-to-date packed refs are preserved' '
git branch q &&
git ${pack_refs} --all --prune &&
git update-ref refs/heads/q refs/heads/q &&
- ! test -f .git/refs/heads/q
+ test_path_is_missing .git/refs/heads/q
'
test_expect_success 'pack, prune and repack' '
--
2.46.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] t/pack-refs-tests: use test_path_is_missing
2026-03-14 6:05 [PATCH v2] t/pack-refs-tests: use test_path_is_missing Ritesh Singh Jadoun
@ 2026-03-14 16:39 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2026-03-14 16:39 UTC (permalink / raw)
To: Ritesh Singh Jadoun; +Cc: git
Ritesh Singh Jadoun <riteshjd75@gmail.com> writes:
> The pack-refs tests currently use raw 'test -f' checks with negation.
> Update them to use Git's standard helper function test_path_is_missing
> for consistency and clearer failure reporting. This aligns with
> CodingGuidelines and makes test failures more obvious.
>
> Signed-off-by: Ritesh Singh Jadoun <riteshjd75@gmail.com>
> ---
> t/pack-refs-tests.sh | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
> index 2fdaccb6c7..3cc4906f05 100644
> --- a/t/pack-refs-tests.sh
> +++ b/t/pack-refs-tests.sh
> @@ -61,13 +61,13 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune
> test_expect_success 'see if git ${pack_refs} --prune remove ref files' '
> git branch f &&
> git ${pack_refs} --all --prune &&
> - ! test -f .git/refs/heads/f
> + test_path_is_missing .git/refs/heads/f
> '
Good.
> test_expect_success 'see if git ${pack_refs} --prune removes empty dirs' '
> git branch r/s/t &&
> git ${pack_refs} --all --prune &&
> - ! test -e .git/refs/heads/r
> + ! test_path_exists .git/refs/heads/r
> '
Questionable. When do we want to loudly tell the human about an
unsatisfied expectation? We expect .git/refs/heads/r not to exist,
so we want to use "test_path_is_missing", no?
Please do double check the remainder of the patch, although from a
cursory look I think you got them all correctly.
THanks.
> test_expect_success 'git branch g should work when git branch g/h has been deleted' '
> @@ -111,43 +111,43 @@ test_expect_success 'test excluded refs are not packed' '
> git branch dont_pack2 &&
> git branch pack_this &&
> git ${pack_refs} --all --exclude "refs/heads/dont_pack*" &&
> - test -f .git/refs/heads/dont_pack1 &&
> - test -f .git/refs/heads/dont_pack2 &&
> - ! test -f .git/refs/heads/pack_this'
> + test_path_is_file .git/refs/heads/dont_pack1 &&
> + test_path_is_file .git/refs/heads/dont_pack2 &&
> + test_path_is_missing .git/refs/heads/pack_this'
>
> test_expect_success 'test --no-exclude refs clears excluded refs' '
> git branch dont_pack3 &&
> git branch dont_pack4 &&
> git ${pack_refs} --all --exclude "refs/heads/dont_pack*" --no-exclude &&
> - ! test -f .git/refs/heads/dont_pack3 &&
> - ! test -f .git/refs/heads/dont_pack4'
> + test_path_is_missing .git/refs/heads/dont_pack3 &&
> + test_path_is_missing .git/refs/heads/dont_pack4'
>
> test_expect_success 'test only included refs are packed' '
> git branch pack_this1 &&
> git branch pack_this2 &&
> git tag dont_pack5 &&
> git ${pack_refs} --include "refs/heads/pack_this*" &&
> - test -f .git/refs/tags/dont_pack5 &&
> - ! test -f .git/refs/heads/pack_this1 &&
> - ! test -f .git/refs/heads/pack_this2'
> + test_path_is_file .git/refs/tags/dont_pack5 &&
> + test_path_is_missing .git/refs/heads/pack_this1 &&
> + test_path_is_missing .git/refs/heads/pack_this2'
>
> test_expect_success 'test --no-include refs clears included refs' '
> git branch pack1 &&
> git branch pack2 &&
> git ${pack_refs} --include "refs/heads/pack*" --no-include &&
> - test -f .git/refs/heads/pack1 &&
> - test -f .git/refs/heads/pack2'
> + test_path_is_file .git/refs/heads/pack1 &&
> + test_path_is_file .git/refs/heads/pack2'
>
> test_expect_success 'test --exclude takes precedence over --include' '
> git branch dont_pack5 &&
> git ${pack_refs} --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
> - test -f .git/refs/heads/dont_pack5'
> + test_path_is_file .git/refs/heads/dont_pack5'
>
> test_expect_success 'see if up-to-date packed refs are preserved' '
> git branch q &&
> git ${pack_refs} --all --prune &&
> git update-ref refs/heads/q refs/heads/q &&
> - ! test -f .git/refs/heads/q
> + test_path_is_missing .git/refs/heads/q
> '
>
> test_expect_success 'pack, prune and repack' '
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] t/pack-refs-tests: use test_path_is_missing
@ 2026-03-15 8:10 Ritesh Singh Jadoun
2026-03-16 15:25 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Ritesh Singh Jadoun @ 2026-03-15 8:10 UTC (permalink / raw)
To: git; +Cc: gitster, Ritesh Singh Jadoun
The pack-refs tests previously used raw 'test -f' and 'test -e' checks
with negation. Update them to use Git's standard helper function
test_path_is_missing for consistency and clearer failure reporting.
As suggested in review, replaced the negated 'test_path_exists' with
test_path_is_missing to better reflect the expected absence of paths.
Signed-off-by: Ritesh Singh Jadoun <riteshjd75@gmail.com>
---
t/pack-refs-tests.sh | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
index 2fdaccb6c7..d76b087b09 100644
--- a/t/pack-refs-tests.sh
+++ b/t/pack-refs-tests.sh
@@ -61,13 +61,13 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune
test_expect_success 'see if git ${pack_refs} --prune remove ref files' '
git branch f &&
git ${pack_refs} --all --prune &&
- ! test -f .git/refs/heads/f
+ test_path_is_missing .git/refs/heads/f
'
test_expect_success 'see if git ${pack_refs} --prune removes empty dirs' '
git branch r/s/t &&
git ${pack_refs} --all --prune &&
- ! test -e .git/refs/heads/r
+ test_path_is_missing .git/refs/heads/r
'
test_expect_success 'git branch g should work when git branch g/h has been deleted' '
@@ -111,43 +111,43 @@ test_expect_success 'test excluded refs are not packed' '
git branch dont_pack2 &&
git branch pack_this &&
git ${pack_refs} --all --exclude "refs/heads/dont_pack*" &&
- test -f .git/refs/heads/dont_pack1 &&
- test -f .git/refs/heads/dont_pack2 &&
- ! test -f .git/refs/heads/pack_this'
+ test_path_is_file .git/refs/heads/dont_pack1 &&
+ test_path_is_file .git/refs/heads/dont_pack2 &&
+ test_path_is_missing .git/refs/heads/pack_this'
test_expect_success 'test --no-exclude refs clears excluded refs' '
git branch dont_pack3 &&
git branch dont_pack4 &&
git ${pack_refs} --all --exclude "refs/heads/dont_pack*" --no-exclude &&
- ! test -f .git/refs/heads/dont_pack3 &&
- ! test -f .git/refs/heads/dont_pack4'
+ test_path_is_missing .git/refs/heads/dont_pack3 &&
+ test_path_is_missing .git/refs/heads/dont_pack4'
test_expect_success 'test only included refs are packed' '
git branch pack_this1 &&
git branch pack_this2 &&
git tag dont_pack5 &&
git ${pack_refs} --include "refs/heads/pack_this*" &&
- test -f .git/refs/tags/dont_pack5 &&
- ! test -f .git/refs/heads/pack_this1 &&
- ! test -f .git/refs/heads/pack_this2'
+ test_path_is_file .git/refs/tags/dont_pack5 &&
+ test_path_is_missing .git/refs/heads/pack_this1 &&
+ test_path_is_missing .git/refs/heads/pack_this2'
test_expect_success 'test --no-include refs clears included refs' '
git branch pack1 &&
git branch pack2 &&
git ${pack_refs} --include "refs/heads/pack*" --no-include &&
- test -f .git/refs/heads/pack1 &&
- test -f .git/refs/heads/pack2'
+ test_path_is_file .git/refs/heads/pack1 &&
+ test_path_is_file .git/refs/heads/pack2'
test_expect_success 'test --exclude takes precedence over --include' '
git branch dont_pack5 &&
git ${pack_refs} --include "refs/heads/pack*" --exclude "refs/heads/pack*" &&
- test -f .git/refs/heads/dont_pack5'
+ test_path_is_file .git/refs/heads/dont_pack5'
test_expect_success 'see if up-to-date packed refs are preserved' '
git branch q &&
git ${pack_refs} --all --prune &&
git update-ref refs/heads/q refs/heads/q &&
- ! test -f .git/refs/heads/q
+ test_path_is_missing .git/refs/heads/q
'
test_expect_success 'pack, prune and repack' '
--
2.46.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] t/pack-refs-tests: use test_path_is_missing
2026-03-15 8:10 Ritesh Singh Jadoun
@ 2026-03-16 15:25 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2026-03-16 15:25 UTC (permalink / raw)
To: Ritesh Singh Jadoun; +Cc: git
Ritesh Singh Jadoun <riteshjd75@gmail.com> writes:
> The pack-refs tests previously used raw 'test -f' and 'test -e' checks
> with negation. Update them to use Git's standard helper function
> test_path_is_missing for consistency and clearer failure reporting.
>
> As suggested in review, replaced the negated 'test_path_exists' with
> test_path_is_missing to better reflect the expected absence of paths.
>
> Signed-off-by: Ritesh Singh Jadoun <riteshjd75@gmail.com>
> ---
> t/pack-refs-tests.sh | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
This round looks very good. Thanks. Will queue.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] t/pack-refs-tests: use test_path_is_missing
@ 2026-03-14 3:46 Ritesh Singh Jadoun
2026-03-14 5:00 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Ritesh Singh Jadoun @ 2026-03-14 3:46 UTC (permalink / raw)
To: git; +Cc: gitster, Ritesh Singh Jadoun
The pack-refs tests currently use raw 'test -f' checks with negation.
Update them to use Git's standard helper function test_path_is_missing
for clearer failure reporting.
Signed-off-by: Ritesh Singh Jadoun <riteshjd75@gmail.com>
---
t/pack-refs-tests.sh | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
index dca0c77ca1..3cc4906f05 100644
--- a/t/pack-refs-tests.sh
+++ b/t/pack-refs-tests.sh
@@ -61,7 +61,7 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune
test_expect_success 'see if git ${pack_refs} --prune remove ref files' '
git branch f &&
git ${pack_refs} --all --prune &&
- ! test_path_is_file .git/refs/heads/f
+ test_path_is_missing .git/refs/heads/f
'
test_expect_success 'see if git ${pack_refs} --prune removes empty dirs' '
@@ -113,14 +113,14 @@ test_expect_success 'test excluded refs are not packed' '
git ${pack_refs} --all --exclude "refs/heads/dont_pack*" &&
test_path_is_file .git/refs/heads/dont_pack1 &&
test_path_is_file .git/refs/heads/dont_pack2 &&
- ! test_path_is_file .git/refs/heads/pack_this'
+ test_path_is_missing .git/refs/heads/pack_this'
test_expect_success 'test --no-exclude refs clears excluded refs' '
git branch dont_pack3 &&
git branch dont_pack4 &&
git ${pack_refs} --all --exclude "refs/heads/dont_pack*" --no-exclude &&
- ! test_path_is_file .git/refs/heads/dont_pack3 &&
- ! test_path_is_file .git/refs/heads/dont_pack4'
+ test_path_is_missing .git/refs/heads/dont_pack3 &&
+ test_path_is_missing .git/refs/heads/dont_pack4'
test_expect_success 'test only included refs are packed' '
git branch pack_this1 &&
@@ -128,8 +128,8 @@ test_expect_success 'test only included refs are packed' '
git tag dont_pack5 &&
git ${pack_refs} --include "refs/heads/pack_this*" &&
test_path_is_file .git/refs/tags/dont_pack5 &&
- ! test_path_is_file .git/refs/heads/pack_this1 &&
- ! test_path_is_file .git/refs/heads/pack_this2'
+ test_path_is_missing .git/refs/heads/pack_this1 &&
+ test_path_is_missing .git/refs/heads/pack_this2'
test_expect_success 'test --no-include refs clears included refs' '
git branch pack1 &&
@@ -147,7 +147,7 @@ test_expect_success 'see if up-to-date packed refs are preserved' '
git branch q &&
git ${pack_refs} --all --prune &&
git update-ref refs/heads/q refs/heads/q &&
- ! test_path_is_file .git/refs/heads/q
+ test_path_is_missing .git/refs/heads/q
'
test_expect_success 'pack, prune and repack' '
--
2.46.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] t/pack-refs-tests: use test_path_is_missing
2026-03-14 3:46 Ritesh Singh Jadoun
@ 2026-03-14 5:00 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2026-03-14 5:00 UTC (permalink / raw)
To: Ritesh Singh Jadoun; +Cc: git
Ritesh Singh Jadoun <riteshjd75@gmail.com> writes:
> The pack-refs tests currently use raw 'test -f' checks with negation.
> Update them to use Git's standard helper function test_path_is_missing
> for clearer failure reporting.
>
> Signed-off-by: Ritesh Singh Jadoun <riteshjd75@gmail.com>
> ---
> t/pack-refs-tests.sh | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
> diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
> index dca0c77ca1..3cc4906f05 100644
> --- a/t/pack-refs-tests.sh
> +++ b/t/pack-refs-tests.sh
> @@ -61,7 +61,7 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune
> test_expect_success 'see if git ${pack_refs} --prune remove ref files' '
> git branch f &&
> git ${pack_refs} --all --prune &&
> - ! test_path_is_file .git/refs/heads/f
> + test_path_is_missing .git/refs/heads/f
> '
This test in my tree looks like this:
test_expect_success 'see if git ${pack_refs} --prune remove ref files' '
git branch f &&
git ${pack_refs} --all --prune &&
! test -f .git/refs/heads/f
'
No patch that turns this instance of "! test -f" into an incorrect
use of test_path_is_file helper, i.e., "! test_path_is_file", has
ever been accepted to my tree. Which means that this [v2] will not
apply to my tree.
I suspect that you have two patches on top of my tree, one is a
botched attempt that turns "! test -f" into "! test_path_is_file",
and the other one is an "oops, the previous one was a bad change, so
fix it on top with another commit" that further changes it to
"test_path_is_missing", and we are looking only at the latter patch.
We however do not work that way around here. Until a patch is
accepted and merged to the 'next' branch, updates are expected to
come as wholesale replacements. See Documentation/SubmittingPatches.
Even though no developer is perfect, when you are presenting your
updated work, armed with wisdom borrowed from your reviewers'
comments on your earlier attempts, you are expected to take the
opportunity to pretend to have written a series of patches that are
perfect logical progression towards the final shape of the code
without detours, change of plans, and fixing earlier mistakes made
in the series.
The final series accepted by the project will have to stay in our
history for later developers to see in "git log" output to learn
from, and a series being clean logical progression is a must for
that to happen.
In this case, if what you have is indeed a pair of patches "one went
into a wrong direction, the next corrects the course", then you
would want to squash them into a single patch that turns "! test -f
.git/refs/heads/f" that is in my tree into an improved form that is
"test_path_is_missing". And send that as the second attempt (v2).
See also
https://lore.kernel.org/git/xmqq34283b12.fsf@gitster.g/
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-16 15:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-14 6:05 [PATCH v2] t/pack-refs-tests: use test_path_is_missing Ritesh Singh Jadoun
2026-03-14 16:39 ` Junio C Hamano
-- strict thread matches above, loose matches on Subject: below --
2026-03-15 8:10 Ritesh Singh Jadoun
2026-03-16 15:25 ` Junio C Hamano
2026-03-14 3:46 Ritesh Singh Jadoun
2026-03-14 5:00 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox