public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] t/pack-refs-tests: drop '-f' from test_path_is_missing
@ 2026-03-22 10:56 jayesh0104
  0 siblings, 0 replies; 5+ messages in thread
From: jayesh0104 @ 2026-03-22 10:56 UTC (permalink / raw)
  To: git; +Cc: jayesh0104

test_path_is_missing expects exactly one argument: the path to
check for absence. Passing '-f' is incorrect and results in
"bug in the test script: 1 param" during test execution.

The '-f' flag appears to have been carried over from the
equivalent 'test -f' usage, but test_path_is_missing does not
accept such flags.

Remove the extraneous '-f' to use the helper correctly and
restore proper test behavior.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
 t/pack-refs-tests.sh | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)
 mode change 100755 => 100644 t/pack-refs-tests.sh

diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
old mode 100755
new mode 100644
index fa27d43a58..4a85d96c6b
--- a/t/pack-refs-tests.sh
+++ b/t/pack-refs-tests.sh
@@ -1,9 +1,3 @@
-#!/bin/sh
-
-test_description='test pack-refs'
-
-. ./test-lib.sh
-
 pack_refs=${pack_refs:-pack-refs}
 
 test_expect_success 'enable reflogs' '
@@ -119,16 +113,14 @@ test_expect_success 'test excluded refs are not packed' '
 	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 -f .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 -f .git/refs/heads/dont_pack4'
 
 test_expect_success 'test only included refs are packed' '
 	git branch pack_this1 &&
@@ -137,16 +129,14 @@ test_expect_success 'test only included refs are packed' '
 	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 -f .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 -f .git/refs/heads/pack2'
 
 test_expect_success 'test --exclude takes precedence over --include' '
 	git branch dont_pack5 &&
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] t/pack-refs-tests: drop '-f' from test_path_is_missing
@ 2026-03-22 13:50 Jayesh Daga via GitGitGadget
  2026-03-22 14:27 ` K Jayatheerth
  2026-03-22 16:37 ` Tian Yuchen
  0 siblings, 2 replies; 5+ messages in thread
From: Jayesh Daga via GitGitGadget @ 2026-03-22 13:50 UTC (permalink / raw)
  To: git; +Cc: K Jayatheerth, Jayesh Daga, jayesh0104

From: jayesh0104 <jayeshdaga99@gmail.com>

test_path_is_missing expects exactly one argument: the path to
check for absence. Passing '-f' is incorrect and results in
"bug in the test script: 1 param" during test execution.

The '-f' flag appears to have been carried over from the
equivalent 'test -f' usage, but test_path_is_missing does not
accept such flags.

Remove the extraneous '-f' to use the helper correctly and
restore proper test behavior.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
    t/pack-refs-tests: fix helper usage
    
    
    High-level (Intent & Context)
    =============================
    
    The test script t/pack-refs-tests.sh has two issues that prevent it from
    running correctly.
    
    It uses: ! test -f .git/refs/heads/f
    
    This is inconsistent with the Git test framework, where helper functions
    such as test_path_is_missing should be used instead of raw test checks.
    
    
    Low-level (Implementation & Justification)
    ==========================================
    
    Without sourcing test-lib.sh, the test framework is not initialized,
    leading to errors such as: test_expect_success: not found
    
    Replaced raw file check with the appropriate helper:
    
    - ! test -f .git/refs/heads/f
    + test_path_is_missing .git/refs/heads/f
    
    
    
    Summary
    =======
    
    Replace test -f with test_path_is_missing

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2248%2Fjayesh0104%2Ffix-pack-refs-test-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2248/jayesh0104/fix-pack-refs-test-v1
Pull-Request: https://github.com/git/git/pull/2248

 t/pack-refs-tests.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
index 2fdaccb6c7..4a85d96c6b 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 -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' '

base-commit: 6e8d538aab8fe4dd07ba9fb87b5c7edcfa5706ad
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] t/pack-refs-tests: drop '-f' from test_path_is_missing
  2026-03-22 13:50 Jayesh Daga via GitGitGadget
@ 2026-03-22 14:27 ` K Jayatheerth
  2026-03-22 16:37 ` Tian Yuchen
  1 sibling, 0 replies; 5+ messages in thread
From: K Jayatheerth @ 2026-03-22 14:27 UTC (permalink / raw)
  To: Jayesh Daga via GitGitGadget; +Cc: git, Jayesh Daga

On Sun, Mar 22, 2026 at 7:20 PM Jayesh Daga via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: jayesh0104 <jayeshdaga99@gmail.com>
>
> test_path_is_missing expects exactly one argument: the path to
> check for absence. Passing '-f' is incorrect and results in
> "bug in the test script: 1 param" during test execution.
>
> The '-f' flag appears to have been carried over from the
> equivalent 'test -f' usage, but test_path_is_missing does not
> accept such flags.
>
> Remove the extraneous '-f' to use the helper correctly and
> restore proper test behavior.
>
> Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>


While the code itself is now fine in my eyes, you aren't actually
removing a -f flag here as described in the commit message.
In the diff, you are entirely replacing the raw command with the
test_path_is_missing helper.

I did a similar microproject earlier this year,
and you can look at my commit message here for a reference [1]

Also, if this is for your GSoC microproject,
you should probably add a tag in your patch subject line (something
like [GSoC] ).

One other thing I should mention: you should make sure to CC the mentors
for the specific project you are applying to so they see your work!
or if you think the change is directly based on someone's work you can
CC them as well.

I am happy to review the code and help out,
but just letting you know I am a fellow GSoC applicant and not an
official mentor.

Regards,
- Jayatheerth

1 - https://lore.kernel.org/git/CALE2CrS0Q2NS1DbFv4pyRQsuypu=KH6Kurs=m4yWrFbR9QosoA@mail.gmail.com/T/#mbbd865b0c73a93096df476621d485f15674f475b

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] t/pack-refs-tests: drop '-f' from test_path_is_missing
  2026-03-22 13:50 Jayesh Daga via GitGitGadget
  2026-03-22 14:27 ` K Jayatheerth
@ 2026-03-22 16:37 ` Tian Yuchen
  2026-03-24  4:11   ` jayesh0104
  1 sibling, 1 reply; 5+ messages in thread
From: Tian Yuchen @ 2026-03-22 16:37 UTC (permalink / raw)
  To: Jayesh Daga via GitGitGadget, git; +Cc: K Jayatheerth, Jayesh Daga

Hi Jayesh,

> old mode 100755
> new mode 100644
> index fa27d43a58..4a85d96c6b
> --- a/t/pack-refs-tests.sh
> +++ b/t/pack-refs-tests.sh
> @@ -1,9 +1,3 @@
> -#!/bin/sh
> -
> -test_description='test pack-refs'
> -
> -. ./test-lib.sh
> -
>  pack_refs=${pack_refs:-pack-refs}

Above lines are included in the your 3/22/26 18:56 pm patch.

Here, you not only changed the file permission from 755 to 644, but also 
removed the shebang testing framework. That was clearly incorrect — 
fortunately, you seem to have realized this and sent another patch. ;)

> From: jayesh0104 <jayeshdaga99@gmail.com>
> 
> test_path_is_missing expects exactly one argument: the path to
> check for absence. Passing '-f' is incorrect and results in
> "bug in the test script: 1 param" during test execution.
> 
> The '-f' flag appears to have been carried over from the
> equivalent 'test -f' usage, but test_path_is_missing does not
> accept such flags.
> 
> Remove the extraneous '-f' to use the helper correctly and
> restore proper test behavior.
> 
> Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
> ---
>      t/pack-refs-tests: fix helper usage
>      
>      
>      High-level (Intent & Context)
>      =============================
>      
>      The test script t/pack-refs-tests.sh has two issues that prevent it from
>      running correctly.
>      
>      It uses: ! test -f .git/refs/heads/f
>      
>      This is inconsistent with the Git test framework, where helper functions
>      such as test_path_is_missing should be used instead of raw test checks.
>      
>      
>      Low-level (Implementation & Justification)
>      ==========================================
>      
>      Without sourcing test-lib.sh, the test framework is not initialized,
>      leading to errors such as: test_expect_success: not found
>      
>      Replaced raw file check with the appropriate helper:
>      
>      - ! test -f .git/refs/heads/f
>      + test_path_is_missing .git/refs/heads/f
>      
>      
>      
>      Summary
>      =======
>      
>      Replace test -f with test_path_is_missing
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2248%2Fjayesh0104%2Ffix-pack-refs-test-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2248/jayesh0104/fix-pack-refs-test-v1
> Pull-Request: https://github.com/git/git/pull/2248
> 
>   t/pack-refs-tests.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
> index 2fdaccb6c7..4a85d96c6b 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 -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' '
> 
> base-commit: 6e8d538aab8fe4dd07ba9fb87b5c7edcfa5706ad

...

I have no objections to the changes mentioned above, but I think you 
should name this patch V2, which is the community standard. Also, I 
think it would be great if you replied to the reviewers.

Thanks,

Yuchen

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] t/pack-refs-tests: drop '-f' from test_path_is_missing
  2026-03-22 16:37 ` Tian Yuchen
@ 2026-03-24  4:11   ` jayesh0104
  0 siblings, 0 replies; 5+ messages in thread
From: jayesh0104 @ 2026-03-24  4:11 UTC (permalink / raw)
  To: a3205153416; +Cc: git, gitgitgadget, jayeshdaga99

Hi Tian Yuchen,

Thanks for the review!

You're absolutely right, the earlier version accidentally removed the
shebang and test framework lines along with changing the file mode.
That was unintended, and I corrected it in the updated patch.

I'll make sure to properly version future updates as v2.

I appreciate the guidance.

Thanks,
Jayesh

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-24  4:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-22 10:56 [PATCH] t/pack-refs-tests: drop '-f' from test_path_is_missing jayesh0104
  -- strict thread matches above, loose matches on Subject: below --
2026-03-22 13:50 Jayesh Daga via GitGitGadget
2026-03-22 14:27 ` K Jayatheerth
2026-03-22 16:37 ` Tian Yuchen
2026-03-24  4:11   ` jayesh0104

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox