{"thread":{"id":"64720","subject":"[GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging","startedAt":"2026-01-04T16:16:14Z","lastAt":"2026-01-08T01:31:09Z","messageCount":4,"participants":["Andrew Chitester","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"532994","messageId":"20260104161536.45384-1-andchi@fastmail.com","threadId":"64720","inReplyTo":null,"subject":"[GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging","fromName":"Andrew Chitester","fromEmail":"andchi@fastmail.com","sentAt":"2026-01-04T16:15:04Z","receivedAt":"2026-01-04T16:16:14Z","isPatch":true,"sender":{"key":"andchi@fastmail.com","avatar":null},"body":"This test will fail silently without giving any error message. Use\ntest_path_is_file in place of test -f to ensure this test errors with a\nmessage.\n\nSigned-off-by: Andrew Chitester <andchi@fastmail.com>\n---\n t/t1420-lost-found.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1420-lost-found.sh b/t/t1420-lost-found.sh\nindex 2fb2f44f02..5fbb1d10ed 100755\n--- a/t/t1420-lost-found.sh\n+++ b/t/t1420-lost-found.sh\n@@ -29,8 +29,8 @@ test_expect_success 'lost and found something' '\n \tgit reset --hard HEAD^ &&\n \tgit fsck --lost-found &&\n \ttest 2 = $(ls .git/lost-found/*/* | wc -l) &&\n-\ttest -f .git/lost-found/commit/$(cat lost-commit) &&\n-\ttest -f .git/lost-found/other/$(cat lost-other)\n+\ttest_path_is_file .git/lost-found/commit/$(cat lost-commit) &&\n+\ttest_path_is_file .git/lost-found/other/$(cat lost-other)\n '\n \n test_done\n-- \n2.52.0\n\n"},{"id":"533008","messageId":"xmqq4ip0n3mm.fsf@gitster.g","threadId":"64720","inReplyTo":"20260104161536.45384-1-andchi@fastmail.com","subject":"Re: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-05T03:24:17Z","receivedAt":"2026-01-05T03:24:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Chitester <andchi@fastmail.com> writes:\n\n> This test will fail silently without giving any error message. Use\n> test_path_is_file in place of test -f to ensure this test errors with a\n> message.\n>\n> Signed-off-by: Andrew Chitester <andchi@fastmail.com>\n> ---\n>  t/t1420-lost-found.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t1420-lost-found.sh b/t/t1420-lost-found.sh\n> index 2fb2f44f02..5fbb1d10ed 100755\n> --- a/t/t1420-lost-found.sh\n> +++ b/t/t1420-lost-found.sh\n> @@ -29,8 +29,8 @@ test_expect_success 'lost and found something' '\n>  \tgit reset --hard HEAD^ &&\n>  \tgit fsck --lost-found &&\n>  \ttest 2 = $(ls .git/lost-found/*/* | wc -l) &&\n> -\ttest -f .git/lost-found/commit/$(cat lost-commit) &&\n> -\ttest -f .git/lost-found/other/$(cat lost-other)\n> +\ttest_path_is_file .git/lost-found/commit/$(cat lost-commit) &&\n> +\ttest_path_is_file .git/lost-found/other/$(cat lost-other)\n>  '\n\nLooks correct, but given that what these tests want to ensure is\nthat underneath .git/lost-found there are only these two expected\nfiles, I have to wonder if the output of \"ls\" here is expected to be\nvery stable.  I.e. if we rewrote the whole thing to something like\n...\n\n\tls .git/lost-found/*/* >actual &&\n\tcat >expect <<-EOF &&\n\t.git/lost-found/commit/$(cat lost-commit)\n\t.git/lost-found/other/$(cat lost-other)\n\tEOF\n\ttest_cmp expect actual\n\n... would it be a more direct way to say that and is easier to\nunderstand to our readers.\n\n"},{"id":"533143","messageId":"20260106132658.798706-1-andchi@fastmail.com","threadId":"64720","inReplyTo":"20260104161536.45384-1-andchi@fastmail.com","subject":"[GSoC PATCH v2 1/1] t1420: modernize the lost-found test","fromName":"Andrew Chitester","fromEmail":"andchi@fastmail.com","sentAt":"2026-01-06T13:26:58Z","receivedAt":"2026-01-06T13:28:15Z","isPatch":true,"sender":{"key":"andchi@fastmail.com","avatar":null},"body":"This test indirectly checks that the lost-found folder has 2 files in it\nand then checks that the expected two files exist. Make this more\ndeliberate by removing the old test -f and compare the actual ls of the\nlost-found directory with the expected files.\n\nSigned-off-by: Andrew Chitester <andchi@fastmail.com>\n---\n t/t1420-lost-found.sh | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1420-lost-found.sh b/t/t1420-lost-found.sh\nindex 2fb2f44f02..926c6d63e3 100755\n--- a/t/t1420-lost-found.sh\n+++ b/t/t1420-lost-found.sh\n@@ -28,9 +28,12 @@ test_expect_success 'lost and found something' '\n \ttest_tick &&\n \tgit reset --hard HEAD^ &&\n \tgit fsck --lost-found &&\n-\ttest 2 = $(ls .git/lost-found/*/* | wc -l) &&\n-\ttest -f .git/lost-found/commit/$(cat lost-commit) &&\n-\ttest -f .git/lost-found/other/$(cat lost-other)\n+\tls .git/lost-found/*/* >actual &&\n+\tcat >expect <<-EOF &&\n+\t.git/lost-found/commit/$(cat lost-commit)\n+\t.git/lost-found/other/$(cat lost-other)\n+\tEOF\n+\ttest_cmp expect actual\n '\n \n test_done\n-- \n2.52.0\n\n"},{"id":"533259","messageId":"87v7hcvqk1.fsf@fastmail.com","threadId":"64720","inReplyTo":"xmqq4ip0n3mm.fsf@gitster.g","subject":"Re: [GSoC PATCH] t1420-lost-found.sh: use test_path_is_file for error logging","fromName":"Andrew Chitester","fromEmail":"andchi@fastmail.com","sentAt":"2026-01-08T01:30:54Z","receivedAt":"2026-01-08T01:31:09Z","isPatch":true,"sender":{"key":"andchi@fastmail.com","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Looks correct, but given that what these tests want to ensure is\n> that underneath .git/lost-found there are only these two expected\n> files, I have to wonder if the output of \"ls\" here is expected to be\n> very stable.  I.e. if we rewrote the whole thing to something like\n> ...\n>\n> \tls .git/lost-found/*/* >actual &&\n> \tcat >expect <<-EOF &&\n> \t.git/lost-found/commit/$(cat lost-commit)\n> \t.git/lost-found/other/$(cat lost-other)\n> \tEOF\n> \ttest_cmp expect actual\n>\n> ... would it be a more direct way to say that and is easier to\n> understand to our readers.\n\nThanks for the feedback. This is an elegant solution that I did not\nconsider. Looking through the other tests, I am seeing this similar\npattern of comparing an expected result with the actual result. It is\nmuch more deliberate and readable this way. I sent a v2, as a reply to\nmy original message, but I think I forgot to Cc you in that message. I'm\nstill figuring out the email workflow.\n"}]}