* [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging
@ 2026-01-04 16:15 Andrew Chitester
2026-01-05 3:24 ` Junio C Hamano
2026-01-06 13:26 ` [GSoC PATCH v2 1/1] t1420: modernize the lost-found test Andrew Chitester
0 siblings, 2 replies; 4+ messages in thread
From: Andrew Chitester @ 2026-01-04 16:15 UTC (permalink / raw)
To: git; +Cc: Andrew Chitester
This test will fail silently without giving any error message. Use
test_path_is_file in place of test -f to ensure this test errors with a
message.
Signed-off-by: Andrew Chitester <andchi@fastmail•com>
---
t/t1420-lost-found.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t1420-lost-found.sh b/t/t1420-lost-found.sh
index 2fb2f44f02..5fbb1d10ed 100755
--- a/t/t1420-lost-found.sh
+++ b/t/t1420-lost-found.sh
@@ -29,8 +29,8 @@ test_expect_success 'lost and found something' '
git reset --hard HEAD^ &&
git fsck --lost-found &&
test 2 = $(ls .git/lost-found/*/* | wc -l) &&
- test -f .git/lost-found/commit/$(cat lost-commit) &&
- test -f .git/lost-found/other/$(cat lost-other)
+ test_path_is_file .git/lost-found/commit/$(cat lost-commit) &&
+ test_path_is_file .git/lost-found/other/$(cat lost-other)
'
test_done
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging
2026-01-04 16:15 [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging Andrew Chitester
@ 2026-01-05 3:24 ` Junio C Hamano
2026-01-08 1:30 ` Andrew Chitester
2026-01-06 13:26 ` [GSoC PATCH v2 1/1] t1420: modernize the lost-found test Andrew Chitester
1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2026-01-05 3:24 UTC (permalink / raw)
To: Andrew Chitester; +Cc: git
Andrew Chitester <andchi@fastmail•com> writes:
> This test will fail silently without giving any error message. Use
> test_path_is_file in place of test -f to ensure this test errors with a
> message.
>
> Signed-off-by: Andrew Chitester <andchi@fastmail•com>
> ---
> t/t1420-lost-found.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1420-lost-found.sh b/t/t1420-lost-found.sh
> index 2fb2f44f02..5fbb1d10ed 100755
> --- a/t/t1420-lost-found.sh
> +++ b/t/t1420-lost-found.sh
> @@ -29,8 +29,8 @@ test_expect_success 'lost and found something' '
> git reset --hard HEAD^ &&
> git fsck --lost-found &&
> test 2 = $(ls .git/lost-found/*/* | wc -l) &&
> - test -f .git/lost-found/commit/$(cat lost-commit) &&
> - test -f .git/lost-found/other/$(cat lost-other)
> + test_path_is_file .git/lost-found/commit/$(cat lost-commit) &&
> + test_path_is_file .git/lost-found/other/$(cat lost-other)
> '
Looks correct, but given that what these tests want to ensure is
that underneath .git/lost-found there are only these two expected
files, I have to wonder if the output of "ls" here is expected to be
very stable. I.e. if we rewrote the whole thing to something like
...
ls .git/lost-found/*/* >actual &&
cat >expect <<-EOF &&
.git/lost-found/commit/$(cat lost-commit)
.git/lost-found/other/$(cat lost-other)
EOF
test_cmp expect actual
... would it be a more direct way to say that and is easier to
understand to our readers.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging
2026-01-05 3:24 ` Junio C Hamano
@ 2026-01-08 1:30 ` Andrew Chitester
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Chitester @ 2026-01-08 1:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox•com> writes:
> Looks correct, but given that what these tests want to ensure is
> that underneath .git/lost-found there are only these two expected
> files, I have to wonder if the output of "ls" here is expected to be
> very stable. I.e. if we rewrote the whole thing to something like
> ...
>
> ls .git/lost-found/*/* >actual &&
> cat >expect <<-EOF &&
> .git/lost-found/commit/$(cat lost-commit)
> .git/lost-found/other/$(cat lost-other)
> EOF
> test_cmp expect actual
>
> ... would it be a more direct way to say that and is easier to
> understand to our readers.
Thanks for the feedback. This is an elegant solution that I did not
consider. Looking through the other tests, I am seeing this similar
pattern of comparing an expected result with the actual result. It is
much more deliberate and readable this way. I sent a v2, as a reply to
my original message, but I think I forgot to Cc you in that message. I'm
still figuring out the email workflow.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [GSoC PATCH v2 1/1] t1420: modernize the lost-found test
2026-01-04 16:15 [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging Andrew Chitester
2026-01-05 3:24 ` Junio C Hamano
@ 2026-01-06 13:26 ` Andrew Chitester
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Chitester @ 2026-01-06 13:26 UTC (permalink / raw)
To: andchi; +Cc: git
This test indirectly checks that the lost-found folder has 2 files in it
and then checks that the expected two files exist. Make this more
deliberate by removing the old test -f and compare the actual ls of the
lost-found directory with the expected files.
Signed-off-by: Andrew Chitester <andchi@fastmail•com>
---
t/t1420-lost-found.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/t/t1420-lost-found.sh b/t/t1420-lost-found.sh
index 2fb2f44f02..926c6d63e3 100755
--- a/t/t1420-lost-found.sh
+++ b/t/t1420-lost-found.sh
@@ -28,9 +28,12 @@ test_expect_success 'lost and found something' '
test_tick &&
git reset --hard HEAD^ &&
git fsck --lost-found &&
- test 2 = $(ls .git/lost-found/*/* | wc -l) &&
- test -f .git/lost-found/commit/$(cat lost-commit) &&
- test -f .git/lost-found/other/$(cat lost-other)
+ ls .git/lost-found/*/* >actual &&
+ cat >expect <<-EOF &&
+ .git/lost-found/commit/$(cat lost-commit)
+ .git/lost-found/other/$(cat lost-other)
+ EOF
+ test_cmp expect actual
'
test_done
--
2.52.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-01-08 1:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-04 16:15 [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging Andrew Chitester
2026-01-05 3:24 ` Junio C Hamano
2026-01-08 1:30 ` Andrew Chitester
2026-01-06 13:26 ` [GSoC PATCH v2 1/1] t1420: modernize the lost-found test Andrew Chitester
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox