{"thread":{"id":"65238","subject":"[PATCH] Modernize pack-refs-tests.sh with git's standard command like test_path_is_file, etc","startedAt":"2026-03-13T16:19:11Z","lastAt":"2026-03-13T17:51:56Z","messageCount":2,"participants":["Ritesh Singh Jadoun","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"538911","messageId":"20260313161808.1242-1-riteshjd75@gmail.com","threadId":"65238","inReplyTo":null,"subject":"[PATCH] Modernize pack-refs-tests.sh with git's standard command like test_path_is_file, etc","fromName":"Ritesh Singh Jadoun","fromEmail":"riteshjd75@gmail.com","sentAt":"2026-03-13T16:18:08Z","receivedAt":"2026-03-13T16:19:11Z","isPatch":true,"sender":{"key":"riteshjd75@gmail.com","avatar":"https://avatars.githubusercontent.com/u/181055371?v=4"},"body":"---\n t/pack-refs-tests.sh | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh\nindex 2fdaccb6c7..dca0c77ca1 100644\n--- a/t/pack-refs-tests.sh\n+++ b/t/pack-refs-tests.sh\n@@ -61,13 +61,13 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune\n test_expect_success 'see if git ${pack_refs} --prune remove ref files' '\n \tgit branch f &&\n \tgit ${pack_refs} --all --prune &&\n-\t! test -f .git/refs/heads/f\n+\t! test_path_is_file .git/refs/heads/f\n '\n \n test_expect_success 'see if git ${pack_refs} --prune removes empty dirs' '\n \tgit branch r/s/t &&\n \tgit ${pack_refs} --all --prune &&\n-\t! test -e .git/refs/heads/r\n+\t! test_path_exists .git/refs/heads/r\n '\n \n test_expect_success 'git branch g should work when git branch g/h has been deleted' '\n@@ -111,43 +111,43 @@ test_expect_success 'test excluded refs are not packed' '\n \tgit branch dont_pack2 &&\n \tgit branch pack_this &&\n \tgit ${pack_refs} --all --exclude \"refs/heads/dont_pack*\" &&\n-\ttest -f .git/refs/heads/dont_pack1 &&\n-\ttest -f .git/refs/heads/dont_pack2 &&\n-\t! test -f .git/refs/heads/pack_this'\n+\ttest_path_is_file .git/refs/heads/dont_pack1 &&\n+\ttest_path_is_file .git/refs/heads/dont_pack2 &&\n+\t! test_path_is_file .git/refs/heads/pack_this'\n \n test_expect_success 'test --no-exclude refs clears excluded refs' '\n \tgit branch dont_pack3 &&\n \tgit branch dont_pack4 &&\n \tgit ${pack_refs} --all --exclude \"refs/heads/dont_pack*\" --no-exclude &&\n-\t! test -f .git/refs/heads/dont_pack3 &&\n-\t! test -f .git/refs/heads/dont_pack4'\n+\t! test_path_is_file .git/refs/heads/dont_pack3 &&\n+\t! test_path_is_file .git/refs/heads/dont_pack4'\n \n test_expect_success 'test only included refs are packed' '\n \tgit branch pack_this1 &&\n \tgit branch pack_this2 &&\n \tgit tag dont_pack5 &&\n \tgit ${pack_refs} --include \"refs/heads/pack_this*\" &&\n-\ttest -f .git/refs/tags/dont_pack5 &&\n-\t! test -f .git/refs/heads/pack_this1 &&\n-\t! test -f .git/refs/heads/pack_this2'\n+\ttest_path_is_file .git/refs/tags/dont_pack5 &&\n+\t! test_path_is_file .git/refs/heads/pack_this1 &&\n+\t! test_path_is_file .git/refs/heads/pack_this2'\n \n test_expect_success 'test --no-include refs clears included refs' '\n \tgit branch pack1 &&\n \tgit branch pack2 &&\n \tgit ${pack_refs} --include \"refs/heads/pack*\" --no-include &&\n-\ttest -f .git/refs/heads/pack1 &&\n-\ttest -f .git/refs/heads/pack2'\n+\ttest_path_is_file .git/refs/heads/pack1 &&\n+\ttest_path_is_file .git/refs/heads/pack2'\n \n test_expect_success 'test --exclude takes precedence over --include' '\n \tgit branch dont_pack5 &&\n \tgit ${pack_refs} --include \"refs/heads/pack*\" --exclude \"refs/heads/pack*\" &&\n-\ttest -f .git/refs/heads/dont_pack5'\n+\ttest_path_is_file .git/refs/heads/dont_pack5'\n \n test_expect_success 'see if up-to-date packed refs are preserved' '\n \tgit branch q &&\n \tgit ${pack_refs} --all --prune &&\n \tgit update-ref refs/heads/q refs/heads/q &&\n-\t! test -f .git/refs/heads/q\n+\t! test_path_is_file .git/refs/heads/q\n '\n \n test_expect_success 'pack, prune and repack' '\n-- \n2.46.0.windows.1\n\n"},{"id":"538919","messageId":"xmqq4imj62it.fsf@gitster.g","threadId":"65238","inReplyTo":"20260313161808.1242-1-riteshjd75@gmail.com","subject":"Re: [PATCH] Modernize pack-refs-tests.sh with git's standard command like test_path_is_file, etc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-13T17:51:54Z","receivedAt":"2026-03-13T17:51:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ritesh Singh Jadoun <riteshjd75@gmail.com> writes:\n\n> Subject: Re: [PATCH] Modernize pack-refs-tests.sh with git's standard command like test_path_is_file, etc\n\nUnusual patch title.\n\nNo justification given for these changes in the proposed log message.\n\nMissing sign-off.\n\n> ---\n>  t/pack-refs-tests.sh | 28 ++++++++++++++--------------\n>  1 file changed, 14 insertions(+), 14 deletions(-)\n\nCheck CodingGuidelines and SubmittingPatches (both found in\nthe Documentation/ directory).\n\n> diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh\n> index 2fdaccb6c7..dca0c77ca1 100644\n> --- a/t/pack-refs-tests.sh\n> +++ b/t/pack-refs-tests.sh\n> @@ -61,13 +61,13 @@ test_expect_success 'see if a branch still exists after git ${pack_refs} --prune\n>  test_expect_success 'see if git ${pack_refs} --prune remove ref files' '\n>  \tgit branch f &&\n>  \tgit ${pack_refs} --all --prune &&\n> -\t! test -f .git/refs/heads/f\n> +\t! test_path_is_file .git/refs/heads/f\n>  '\n\nThe point of \"test_path_is_file\" is \"we expect this path to be a\nfile and there is something wrong if it isn't and we should report\nto the person who is running the test loudly\".  That is why\n\n\ttest_path_is_file existing-file\n\nis silent, while\n\n\ttest_path_is_file missing-file\n\ttest_path_is_file existing-directory/\n\nboth loudly report the failure.\n\nBut in this test, that expects \"! test -f .git/refs/heads/f\" to be\ntrue, the story is the other way around.  The test expects that the\nloose ref file for the branch 'f' on the filesystem should be gone.\nIn other words, it is not a notable event if .git/refs/heads/f did\n*NOT* exist, and if .git/refs/heads/f existed, that is something you\nwant to report loudly, now you are using a better helper function.\n\nI think the update should use test_path_is_missing instead, without\nnegation.\n\nI did not look at the rest of the patch, but the above should be\na sufficient guideline to decide what replacement should be used.\nBe careful to the original that uses negation and you'd do fine.\n\nThanks.\n"}]}