# [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging 4 messages from 2026-01-04 to 2026-01-08. Participants: Andrew Chitester, Junio C Hamano. Thread: https://gitlist.dev/t/64720 ## Andrew Chitester, 2026-01-04 16:15 Subject: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging Message-ID: <20260104161536.45384-1-andchi@fastmail.com> URL: https://gitlist.dev/e/20260104161536.45384-1-andchi%40fastmail.com ``` 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 --- 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 ``` ## Junio C Hamano, 2026-01-05 03:24 Subject: Re: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging Message-ID: URL: https://gitlist.dev/e/xmqq4ip0n3mm.fsf%40gitster.g In-Reply-To: <20260104161536.45384-1-andchi@fastmail.com> ``` Andrew Chitester 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 > --- > 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. ``` ## Andrew Chitester, 2026-01-06 13:26 Subject: [GSoC PATCH v2 1/1] t1420: modernize the lost-found test Message-ID: <20260106132658.798706-1-andchi@fastmail.com> URL: https://gitlist.dev/e/20260106132658.798706-1-andchi%40fastmail.com In-Reply-To: <20260104161536.45384-1-andchi@fastmail.com> ``` 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 --- 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 ``` ## Andrew Chitester, 2026-01-08 01:30 Subject: Re: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging Message-ID: <87v7hcvqk1.fsf@fastmail.com> URL: https://gitlist.dev/e/87v7hcvqk1.fsf%40fastmail.com In-Reply-To: ``` Junio C Hamano 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. ```