Github Patch

Github Patch

[Zakariyah Ali]

[-- Attachment #1.1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #1.2: Type: text/html, Size: 26 bytes --]

[-- Attachment #2: 0001-t-t2000-modernize-path-checks-to-use-test_path-helpe.patch --]
[-- Type: text/x-patch, Size: 2203 bytes --]

From 91a3ccf496cdb61149e3c031265fe252c6c8ef3c Mon Sep 17 00:00:00 2001
From: alibaba0010 <zakariyahali100@gmail.com>
Date: Tue, 24 Mar 2026 21:04:38 +0100
Subject: [PATCH] t/t2000: modernize path checks to use test_path helpers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace old-style path checks using `test -f`, `test -d`,
and `test ! -h` with dedicated test helper functions for
improved test clarity and consistency.

This modernization improves test script readability by using
Git's dedicated test helpers:
- `test -f` → `test_path_is_file`
- `test -d` → `test_path_is_dir`
- `test ! -h && test -f` →
  `test_path_is_file_not_symlink`
- `test ! -h && test -d` →
  `test_path_is_dir_not_symlink`

Found instances using:
  git grep 'test -[efd]' t/ | grep 'test -[efd].*&&'

Converted 5 instances in
t/t2000-conflict-when-checking-files-out.sh

Signed-off-by: alibaba0010 <zakariyahali100@gmail.com>
---
 t/t2000-conflict-when-checking-files-out.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index f18616ad2b..b535bb002a 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -58,7 +58,7 @@ test_expect_success \
 
 test_expect_success \
     'git checkout-index conflicting paths.' \
-    'test -f path0 && test -d path1 && test -f path1/file1'
+    'test_path_is_file path0 && test_path_is_dir path1 && test_path_is_file path1/file1'
 
 test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 	mkdir -p tar/get &&
@@ -127,9 +127,9 @@ test_debug 'show_files $tree2'
 
 test_expect_success \
     'checking out conflicting path with -f' \
-    'test ! -h path2 && test -d path2 &&
-     test ! -h path3 && test -d path3 &&
-     test ! -h path2/file0 && test -f path2/file0 &&
-     test ! -h path3/file1 && test -f path3/file1'
+    'test_path_is_dir_not_symlink path2 &&
+     test_path_is_dir_not_symlink path3 &&
+     test_path_is_file_not_symlink path2/file0 &&
+     test_path_is_file_not_symlink path3/file1'
 
 test_done
-- 
2.43.0

Re: Github Patch

[Pablo]

Zakariyah Ali (<zakariyahali100@gmail.com>) escribió:

The mail is empty but for the attached patch.

The patch should be sent inline in the email body, not as an attachment.
Please read Documentation/SubmittingPatches [1] and
Documentation/MyFirstContribution [2] about how to send patches.

Now, about the patch:

> From: alibaba0010 <zakariyahali100@gmail.com>
> Signed-off-by: alibaba0010 <zakariyahali100@gmail.com>

Your email says "Zakariyah Ali" but the From and Signed-off-by says
"alibaba0010. These should be consistent.
See Documentation/SubmittingPatches [1].

 "It is common, but not required, to use some form of your real name.
We realize that some contributors are not comfortable doing so or
prefer to contribute under a pseudonym or preferred name and we can
accept your patch either way, as long as the name and email you use
are distinctive, identifying, and not misleading."

> Replace old-style path checks using `test -f`, `test -d`,
> and `test ! -h` with dedicated test helper functions for
> improved test clarity and consistency.
>
> This modernization improves test script readability by using
> Git's dedicated test helpers: > - `test -f` → `test_path_is_file`
> - `test -d` → `test_path_is_dir`
> - `test ! -h && test -f` →
> `test_path_is_file_not_symlink`
> - `test ! -h && test -d` →
> `test_path_is_dir_not_symlink`

Try explaining why have you done this, why are these helpers better
than what was before rather than clarity and consistency, what do
these new helpers do to be considered valuable to refactor it.

> Found instances using: > git grep 'test -[efd]' t/ | grep 'test -[efd].*&&'

Even though it is mentioned on the microprojects to mention what have
you used to find the file, because it is only one file, including the
search command is not very useful. Drop it.

Given the date and that this seems a microproject I guess this is for
GSoC, you should add to the subject [GSoC PATCH] and CC your possible
co mentors.

I encourage you the same that it's being encourage for newcomers, read:

      https://lore.kernel.org/git/

There you'll find other microprojects similar to yours where you can learn from.

Code seems OK.

[1]: https://github.com/git/git/blob/master/Documentation/SubmittingPatches
[2]: https://github.com/git/git/blob/master/Documentation/MyFirstContribution.adoc

[GSoC PATCH v2] t2000: modernize path checks with test_path_is_* helpers

[Zakariyah Ali]

Replace bare 'test -f/-d' and 'test ! -h' assertions with dedicated
helpers. These helpers report loudly what expectation wasn't met,
therefore making debugging easier.

Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
 t/t2000-conflict-when-checking-files-out.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index f18616ad2b..96bae6c53d 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -58,7 +58,9 @@ test_expect_success \
 
 test_expect_success \
     'git checkout-index conflicting paths.' \
-    'test -f path0 && test -d path1 && test -f path1/file1'
+    'test_path_is_file path0 &&
+     test_path_is_dir path1 &&
+     test_path_is_file path1/file1'
 
 test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 	mkdir -p tar/get &&
@@ -127,9 +129,9 @@ test_debug 'show_files $tree2'
 
 test_expect_success \
     'checking out conflicting path with -f' \
-    'test ! -h path2 && test -d path2 &&
-     test ! -h path3 && test -d path3 &&
-     test ! -h path2/file0 && test -f path2/file0 &&
-     test ! -h path3/file1 && test -f path3/file1'
+    'test_path_is_dir_not_symlink path2 &&
+     test_path_is_dir_not_symlink path3 &&
+     test_path_is_file_not_symlink path2/file0 &&
+     test_path_is_file_not_symlink path3/file1'
 
 test_done
-- 
2.43.0

Re: [GSoC PATCH v2] t2000: modernize path checks with test_path_is_* helpers

[Junio C Hamano]

Zakariyah Ali <zakariyahali100@gmail.com> writes:

> Replace bare 'test -f/-d' and 'test ! -h' assertions with dedicated
> helpers. These helpers report loudly what expectation wasn't met,
> therefore making debugging easier.
>
> Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
> ---
>  t/t2000-conflict-when-checking-files-out.sh | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

There is nothing in the patch text or in the proposed log message
that is wrong per-se, but looking at the entire test script, it
needs a major clean-up to match the modern testing standard.

On top of the patch we see here, we may want to do a follow-up patch
series to clean them.

Here is how such a patch may start out.  I think the remainder of
the file needs to be cleaned up similarly with about the same amount
of work.

---- >8 ----
Subject: t2000: modernise overall structure

This test script that dates back to 2005 certainly shows its age and
both its style and the way the tests are laid out do not match the
modern standard.

 * Executables that prepare the data used to test the command
   should be inside the test_expect_success block in modern tests.

 * In modern tests, running a command that is being tested, making
   sure it succeeds, and inspecting other side effects that are
   expected, are all done in a single test_expect_success block.

 * A test_expect_success block in modern tests are laid out as

	test_expect_success 'title of the test' '
		body of the test &&
		...
		body of the test
	'

   not as

	test_expect_success \
		'title of the test' \
                'body of the test &&
		...
		body of the test'

   which is in a prehistoric style.

 * In modern tests, each &&-chained statement in the body of the
   test_expect_success block are indented with a horizontal tab,
   unlike prehistoric style that used 4-space indent.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t2000-conflict-when-checking-files-out.sh | 43 ++++++++++++++---------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git c/t/t2000-conflict-when-checking-files-out.sh w/t/t2000-conflict-when-checking-files-out.sh
index 96bae6c53d..39c80e80ea 100755
--- c/t/t2000-conflict-when-checking-files-out.sh
+++ w/t/t2000-conflict-when-checking-files-out.sh
@@ -35,32 +35,31 @@ show_files() {
 	sed -e 's/^\([0-9]*\)	[^ ]*	[0-9a-f]*	/tr: \1 /'
 }
 
-date >path0
-mkdir path1
-date >path1/file1
+test_expect_success 'prepare files path0 and path1/file1' '
+	date >path0 &&
+	mkdir path1 &&
+	date >path1/file1 &&
 
-test_expect_success \
-    'git update-index --add various paths.' \
-    'git update-index --add path0 path1/file1'
-
-rm -fr path0 path1
-mkdir path0
-date >path0/file0
-date >path1
+	git update-index --add path0 path1/file1
+'
 
-test_expect_success \
-    'git checkout-index without -f should fail on conflicting work tree.' \
-    'test_must_fail git checkout-index -a'
+test_expect_success 'prepare working tree files with D/F conflicts' '
+	rm -fr path0 path1 &&
+	mkdir path0 &&
+	date >path0/file0 &&
+	date >path1
+'
 
-test_expect_success \
-    'git checkout-index with -f should succeed.' \
-    'git checkout-index -f -a'
+test_expect_success 'git checkout-index without -f should fail on conflicting work tree.' '
+	test_must_fail git checkout-index -a
+'
 
-test_expect_success \
-    'git checkout-index conflicting paths.' \
-    'test_path_is_file path0 &&
-     test_path_is_dir path1 &&
-     test_path_is_file path1/file1'
+test_expect_success 'git checkout-index with -f should succeed.' '
+	git checkout-index -f -a &&
+	test_path_is_file path0 &&
+	test_path_is_dir path1 &&
+	test_path_is_file path1/file1
+'
 
 test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 	mkdir -p tar/get &&

[GSoC][PATCH v3] t2000: modernise overall structure

[Zakariyah Ali]

This test script that dates back to 2005 certainly shows its age and
both its style and the way the tests are laid out do not match the
modern standard.

 * Executables that prepare the data used to test the command should
   be inside the test_expect_success block in modern tests.

 * In modern tests, running a command that is being tested, making
   sure it succeeds, and inspecting other side effects that are
   expected, are all done in a single test_expect_success block.

 * A test_expect_success block in modern tests are laid out as

        test_expect_success 'title of the test' '
                body of the test &&
                ...
                body of the test
        '

   not as

        test_expect_success \
                'title of the test' \
                'body of the test &&
                ...
                body of the test'

   which is in a prehistoric style.

 * In modern tests, each &&-chained statement in the body of the
   test_expect_success block are indented with a horizontal tab,
   unlike prehistoric style that used 4-space indent.

Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
 t/t2000-conflict-when-checking-files-out.sh | 122 +++++++++++---------
 1 file changed, 66 insertions(+), 56 deletions(-)

diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index f18616ad2b..af199d8191 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -35,30 +35,30 @@ show_files() {
 	sed -e 's/^\([0-9]*\)	[^ ]*	[0-9a-f]*	/tr: \1 /'
 }
 
-date >path0
-mkdir path1
-date >path1/file1
-
-test_expect_success \
-    'git update-index --add various paths.' \
-    'git update-index --add path0 path1/file1'
-
-rm -fr path0 path1
-mkdir path0
-date >path0/file0
-date >path1
+test_expect_success 'prepare files path0 and path1/file1' '
+	date >path0 &&
+	mkdir path1 &&
+	date >path1/file1 &&
+	git update-index --add path0 path1/file1
+'
 
-test_expect_success \
-    'git checkout-index without -f should fail on conflicting work tree.' \
-    'test_must_fail git checkout-index -a'
+test_expect_success 'prepare working tree files with D/F conflicts' '
+	rm -fr path0 path1 &&
+	mkdir path0 &&
+	date >path0/file0 &&
+	date >path1
+'
 
-test_expect_success \
-    'git checkout-index with -f should succeed.' \
-    'git checkout-index -f -a'
+test_expect_success 'git checkout-index without -f should fail on conflicting work tree.' '
+	test_must_fail git checkout-index -a
+'
 
-test_expect_success \
-    'git checkout-index conflicting paths.' \
-    'test -f path0 && test -d path1 && test -f path1/file1'
+test_expect_success 'git checkout-index with -f should succeed.' '
+	git checkout-index -f -a &&
+	test_path_is_file path0 &&
+	test_path_is_dir path1 &&
+	test_path_is_file path1/file1
+'
 
 test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 	mkdir -p tar/get &&
@@ -83,53 +83,63 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 # path path3 is occupied by a non-directory.  With "-f" it should remove
 # the symlink path3 and create directory path3 and file path3/file1.
 
-mkdir path2
-date >path2/file0
-test_expect_success \
-    'git update-index --add path2/file0' \
-    'git update-index --add path2/file0'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree1=$(git write-tree)'
+test_expect_success 'prepare path2/file0 and index' '
+	mkdir path2 &&
+	date >path2/file0 &&
+	git update-index --add path2/file0
+'
+
+test_expect_success 'write tree with path2/file0' '
+	tree1=$(git write-tree)
+'
+
 test_debug 'show_files $tree1'
 
-mkdir path3
-date >path3/file1
-test_expect_success \
-    'git update-index --add path3/file1' \
-    'git update-index --add path3/file1'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree2=$(git write-tree)'
+test_expect_success 'prepare path3/file1 and index' '
+	mkdir path3 &&
+	date >path3/file1 &&
+	git update-index --add path3/file1
+'
+
+test_expect_success 'write tree with path3/file1' '
+	tree2=$(git write-tree)
+'
+
 test_debug 'show_files $tree2'
 
-rm -fr path3
-test_expect_success \
-    'read previously written tree and checkout.' \
-    'git read-tree -m $tree1 && git checkout-index -f -a'
+test_expect_success 'read previously written tree and checkout.' '
+	rm -fr path3 &&
+	git read-tree -m $tree1 &&
+	git checkout-index -f -a
+'
+
 test_debug 'show_files $tree1'
 
-test_expect_success \
-    'add a symlink' \
-    'test_ln_s_add path2 path3'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree3=$(git write-tree)'
+test_expect_success 'add a symlink' '
+	test_ln_s_add path2 path3
+'
+
+test_expect_success 'write tree with symlink path3' '
+	tree3=$(git write-tree)
+'
+
 test_debug 'show_files $tree3'
 
 # Morten says "Got that?" here.
 # Test begins.
 
-test_expect_success \
-    'read previously written tree and checkout.' \
-    'git read-tree $tree2 && git checkout-index -f -a'
+test_expect_success 'read previously written tree and checkout.' '
+	git read-tree $tree2 &&
+	git checkout-index -f -a
+'
+
 test_debug 'show_files $tree2'
 
-test_expect_success \
-    'checking out conflicting path with -f' \
-    'test ! -h path2 && test -d path2 &&
-     test ! -h path3 && test -d path3 &&
-     test ! -h path2/file0 && test -f path2/file0 &&
-     test ! -h path3/file1 && test -f path3/file1'
+test_expect_success 'checking out conflicting path with -f' '
+	test_path_is_dir_not_symlink path2 &&
+	test_path_is_dir_not_symlink path3 &&
+	test_path_is_file_not_symlink path2/file0 &&
+	test_path_is_file_not_symlink path3/file1
+'
 
 test_done
-- 
2.43.0

Re: [GSoC][PATCH v3] t2000: modernise overall structure

[Zakariyah Ali]

Hi everyone,

Just a gentle reminder on this v3 patch. I would be looking forward for your review.

Thanks,
Zakariyah

Re: [GSoC][PATCH v3] t2000: modernise overall structure

[Tian Yuchen]

On 3/28/26 07:40, Zakariyah Ali wrote:
> This test script that dates back to 2005 certainly shows its age and
> both its style and the way the tests are laid out do not match the
> modern standard.
> 
>   * Executables that prepare the data used to test the command should
>     be inside the test_expect_success block in modern tests.
> 
>   * In modern tests, running a command that is being tested, making
>     sure it succeeds, and inspecting other side effects that are
>     expected, are all done in a single test_expect_success block.
> 
>   * A test_expect_success block in modern tests are laid out as
> 
>          test_expect_success 'title of the test' '
>                  body of the test &&
>                  ...
>                  body of the test
>          '
> 
>     not as
> 
>          test_expect_success \
>                  'title of the test' \
>                  'body of the test &&
>                  ...
>                  body of the test'
> 
>     which is in a prehistoric style.
> 
>   * In modern tests, each &&-chained statement in the body of the
>     test_expect_success block are indented with a horizontal tab,
>     unlike prehistoric style that used 4-space indent.

I like this commit message. Nice.

Would it be better to add a 'Helped-by'?

> Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
> ---
>   t/t2000-conflict-when-checking-files-out.sh | 122 +++++++++++---------
>   1 file changed, 66 insertions(+), 56 deletions(-)
> 
> diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
> index f18616ad2b..af199d8191 100755
> --- a/t/t2000-conflict-when-checking-files-out.sh
> +++ b/t/t2000-conflict-when-checking-files-out.sh
> @@ -35,30 +35,30 @@ show_files() {
>   	sed -e 's/^\([0-9]*\)	[^ ]*	[0-9a-f]*	/tr: \1 /'
>   }
>   
> -date >path0
> -mkdir path1
> -date >path1/file1
> -
> -test_expect_success \
> -    'git update-index --add various paths.' \
> -    'git update-index --add path0 path1/file1'
> -
> -rm -fr path0 path1
> -mkdir path0
> -date >path0/file0
> -date >path1
> +test_expect_success 'prepare files path0 and path1/file1' '
> +	date >path0 &&
> +	mkdir path1 &&
> +	date >path1/file1 &&
> +	git update-index --add path0 path1/file1
> +'
>   
> -test_expect_success \
> -    'git checkout-index without -f should fail on conflicting work tree.' \
> -    'test_must_fail git checkout-index -a'
> +test_expect_success 'prepare working tree files with D/F conflicts' '
> +	rm -fr path0 path1 &&
> +	mkdir path0 &&
> +	date >path0/file0 &&
> +	date >path1
> +'
>   
> -test_expect_success \
> -    'git checkout-index with -f should succeed.' \
> -    'git checkout-index -f -a'
> +test_expect_success 'git checkout-index without -f should fail on conflicting work tree.' '
> +	test_must_fail git checkout-index -a
> +'
>   
> -test_expect_success \
> -    'git checkout-index conflicting paths.' \
> -    'test -f path0 && test -d path1 && test -f path1/file1'
> +test_expect_success 'git checkout-index with -f should succeed.' '
> +	git checkout-index -f -a &&
> +	test_path_is_file path0 &&
> +	test_path_is_dir path1 &&
> +	test_path_is_file path1/file1
> +'
>   
>   test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
>   	mkdir -p tar/get &&
> @@ -83,53 +83,63 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
>   # path path3 is occupied by a non-directory.  With "-f" it should remove
>   # the symlink path3 and create directory path3 and file path3/file1.
>   
> -mkdir path2
> -date >path2/file0
> -test_expect_success \
> -    'git update-index --add path2/file0' \
> -    'git update-index --add path2/file0'
> -test_expect_success \
> -    'writing tree out with git write-tree' \
> -    'tree1=$(git write-tree)'
> +test_expect_success 'prepare path2/file0 and index' '
> +	mkdir path2 &&
> +	date >path2/file0 &&
> +	git update-index --add path2/file0
> +'
> +
> +test_expect_success 'write tree with path2/file0' '
> +	tree1=$(git write-tree)
> +'
> +
>   test_debug 'show_files $tree1'
>   
> -mkdir path3
> -date >path3/file1
> -test_expect_success \
> -    'git update-index --add path3/file1' \
> -    'git update-index --add path3/file1'
> -test_expect_success \
> -    'writing tree out with git write-tree' \
> -    'tree2=$(git write-tree)'
> +test_expect_success 'prepare path3/file1 and index' '
> +	mkdir path3 &&
> +	date >path3/file1 &&
> +	git update-index --add path3/file1
> +'
> +
> +test_expect_success 'write tree with path3/file1' '
> +	tree2=$(git write-tree)
> +'
> +
>   test_debug 'show_files $tree2'
>   
> -rm -fr path3
> -test_expect_success \
> -    'read previously written tree and checkout.' \
> -    'git read-tree -m $tree1 && git checkout-index -f -a'
> +test_expect_success 'read previously written tree and checkout.' '
> +	rm -fr path3 &&
> +	git read-tree -m $tree1 &&
> +	git checkout-index -f -a
> +'
> +
>   test_debug 'show_files $tree1'
>   
> -test_expect_success \
> -    'add a symlink' \
> -    'test_ln_s_add path2 path3'
> -test_expect_success \
> -    'writing tree out with git write-tree' \
> -    'tree3=$(git write-tree)'
> +test_expect_success 'add a symlink' '
> +	test_ln_s_add path2 path3
> +'
> +
> +test_expect_success 'write tree with symlink path3' '
> +	tree3=$(git write-tree)
> +'
> +
>   test_debug 'show_files $tree3'
>   
>   # Morten says "Got that?" here.
>   # Test begins.
>   
> -test_expect_success \
> -    'read previously written tree and checkout.' \
> -    'git read-tree $tree2 && git checkout-index -f -a'
> +test_expect_success 'read previously written tree and checkout.' '
> +	git read-tree $tree2 &&
> +	git checkout-index -f -a
> +'
> +
>   test_debug 'show_files $tree2'
>   
> -test_expect_success \
> -    'checking out conflicting path with -f' \
> -    'test ! -h path2 && test -d path2 &&
> -     test ! -h path3 && test -d path3 &&
> -     test ! -h path2/file0 && test -f path2/file0 &&
> -     test ! -h path3/file1 && test -f path3/file1'
> +test_expect_success 'checking out conflicting path with -f' '
> +	test_path_is_dir_not_symlink path2 &&
> +	test_path_is_dir_not_symlink path3 &&
> +	test_path_is_file_not_symlink path2/file0 &&
> +	test_path_is_file_not_symlink path3/file1
> +'

test_path_is_file_not_symlink and test_path_is_dir_not_symlink look 
rather unfamiliar, and sure enough, only four test scripts in the Git 
test suite make use of them.

These two functions were implemented four years ago; the commit hash is 
456296b5d1f05ca16949e7d37ae87f5750118564. Although the names of these 
two functions are particularly simple and straightforward, I’m not sure 
if it would be better to mention them in the commit message.

>   
>   test_done

Thanks, Yuchen

[PATCH v4 1/1] t2000: modernize overall structure and path checks

[Zakariyah Ali]

This test script that dates back to 2005 certainly shows its age and
both its style and the way the tests are laid out do not match the
modern standard. Modernize it to match the current testing standards:

  * Executables that prepare the data used to test the command should
    be inside the test_expect_success block in modern tests.

  * In modern tests, running a command that is being tested, making
    sure it succeeds, and inspecting other side effects that are
    expected, are all done in a single test_expect_success block.

  * A test_expect_success block in modern tests are laid out as

         test_expect_success 'title of the test' '
                 body of the test &&
                 ...
                 body of the test
         '

    not as

         test_expect_success \
                 'title of the test' \
                 'body of the test &&
                 ...
                 body of the test'

    which is in a prehistoric style.

  * In modern tests, each &&-chained statement in the body of the
    test_expect_success block are indented with a horizontal tab,
    unlike prehistoric style that used 4-space indent.

  * Replace bare 'test -f/-d' and 'test ! -h' assertions with dedicated
    test_path_is_* helpers (specifically test_path_is_file_not_symlink and
    test_path_is_dir_not_symlink). While less commonly used in the test
    suite than test_path_is_file/dir, they act as direct replacements
    for the specific checks being performed and provide clearer
    diagnostics on failure.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
Hi Yuchen,

Thanks for the review. Regarding test_path_is_file_not_symlink and test_path_is_dir_not_symlink, I used them as direct replacements for the existing pattern: `test ! -h <path> && test -f/-d <path>`. As you noted, they are simple and straightforward. Following your suggestion, I've added a note about them to the commit message in this v4 patch.

I have also added the 'Helped-by' trailer as suggested, since the commit message structure was provided by Junio C Hamano.

Also, a quick question please: since the GSoC proposal period for 2026 has closed, could you guide me on the next steps for applying to subsequent related internships (like Outreachy, if applicable)? I would love to know how I can best continue contributing to Git in the meantime.

Regards,
Zakariyah Ali.

 t/t2000-conflict-when-checking-files-out.sh | 99 +++++++++++----------
 1 file changed, 54 insertions(+), 45 deletions(-)

diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index f18616ad2b..a8a49df93e 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -48,17 +48,16 @@ mkdir path0
 date >path0/file0
 date >path1
 
-test_expect_success \
-    'git checkout-index without -f should fail on conflicting work tree.' \
-    'test_must_fail git checkout-index -a'
-
-test_expect_success \
-    'git checkout-index with -f should succeed.' \
-    'git checkout-index -f -a'
+test_expect_success 'git checkout-index without -f should fail on conflicting work tree.' '
+	test_must_fail git checkout-index -a
+'
 
-test_expect_success \
-    'git checkout-index conflicting paths.' \
-    'test -f path0 && test -d path1 && test -f path1/file1'
+test_expect_success 'git checkout-index with -f should succeed.' '
+	git checkout-index -f -a &&
+	test_path_is_file path0 &&
+	test_path_is_dir path1 &&
+	test_path_is_file path1/file1
+'
 
 test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 	mkdir -p tar/get &&
@@ -83,53 +82,63 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 # path path3 is occupied by a non-directory.  With "-f" it should remove
 # the symlink path3 and create directory path3 and file path3/file1.
 
-mkdir path2
-date >path2/file0
-test_expect_success \
-    'git update-index --add path2/file0' \
-    'git update-index --add path2/file0'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree1=$(git write-tree)'
+test_expect_success 'prepare path2/file0 and index' '
+	mkdir path2 &&
+	date >path2/file0 &&
+	git update-index --add path2/file0
+'
+
+test_expect_success 'write tree with path2/file0' '
+	tree1=$(git write-tree)
+'
+
 test_debug 'show_files $tree1'
 
-mkdir path3
-date >path3/file1
-test_expect_success \
-    'git update-index --add path3/file1' \
-    'git update-index --add path3/file1'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree2=$(git write-tree)'
+test_expect_success 'prepare path3/file1 and index' '
+	mkdir path3 &&
+	date >path3/file1 &&
+	git update-index --add path3/file1
+'
+
+test_expect_success 'write tree with path3/file1' '
+	tree2=$(git write-tree)
+'
+
 test_debug 'show_files $tree2'
 
-rm -fr path3
-test_expect_success \
-    'read previously written tree and checkout.' \
-    'git read-tree -m $tree1 && git checkout-index -f -a'
+test_expect_success 'read previously written tree and checkout.' '
+	rm -fr path3 &&
+	git read-tree -m $tree1 &&
+	git checkout-index -f -a
+'
+
 test_debug 'show_files $tree1'
 
-test_expect_success \
-    'add a symlink' \
-    'test_ln_s_add path2 path3'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree3=$(git write-tree)'
+test_expect_success 'add a symlink' '
+	test_ln_s_add path2 path3
+'
+
+test_expect_success 'write tree with symlink path3' '
+	tree3=$(git write-tree)
+'
+
 test_debug 'show_files $tree3'
 
 # Morten says "Got that?" here.
 # Test begins.
 
-test_expect_success \
-    'read previously written tree and checkout.' \
-    'git read-tree $tree2 && git checkout-index -f -a'
+test_expect_success 'read previously written tree and checkout.' '
+	git read-tree $tree2 &&
+	git checkout-index -f -a
+'
+
 test_debug 'show_files $tree2'
 
-test_expect_success \
-    'checking out conflicting path with -f' \
-    'test ! -h path2 && test -d path2 &&
-     test ! -h path3 && test -d path3 &&
-     test ! -h path2/file0 && test -f path2/file0 &&
-     test ! -h path3/file1 && test -f path3/file1'
+test_expect_success 'checking out conflicting path with -f' '
+	test_path_is_dir_not_symlink path2 &&
+	test_path_is_dir_not_symlink path3 &&
+	test_path_is_file_not_symlink path2/file0 &&
+	test_path_is_file_not_symlink path3/file1
+'
 
 test_done
-- 
2.43.0

Re: [PATCH v4 1/1] t2000: modernize overall structure and path checks

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 7623 bytes --]

Zakariyah Ali <zakariyahali100@gmail.com> writes:

> This test script that dates back to 2005 certainly shows its age and
> both its style and the way the tests are laid out do not match the
> modern standard. Modernize it to match the current testing standards:
>
>   * Executables that prepare the data used to test the command should
>     be inside the test_expect_success block in modern tests.
>
>   * In modern tests, running a command that is being tested, making
>     sure it succeeds, and inspecting other side effects that are
>     expected, are all done in a single test_expect_success block.
>
>   * A test_expect_success block in modern tests are laid out as
>
>          test_expect_success 'title of the test' '
>                  body of the test &&
>                  ...
>                  body of the test
>          '
>
>     not as
>
>          test_expect_success \
>                  'title of the test' \
>                  'body of the test &&
>                  ...
>                  body of the test'
>
>     which is in a prehistoric style.
>
>   * In modern tests, each &&-chained statement in the body of the
>     test_expect_success block are indented with a horizontal tab,
>     unlike prehistoric style that used 4-space indent.
>
>   * Replace bare 'test -f/-d' and 'test ! -h' assertions with dedicated
>     test_path_is_* helpers (specifically test_path_is_file_not_symlink and
>     test_path_is_dir_not_symlink). While less commonly used in the test
>     suite than test_path_is_file/dir, they act as direct replacements
>     for the specific checks being performed and provide clearer
>     diagnostics on failure.
>

> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
> ---
> Hi Yuchen,
>
> Thanks for the review. Regarding test_path_is_file_not_symlink and test_path_is_dir_not_symlink, I used them as direct replacements for the existing pattern: `test ! -h <path> && test -f/-d <path>`. As you noted, they are simple and straightforward. Following your suggestion, I've added a note about them to the commit message in this v4 patch.
>
> I have also added the 'Helped-by' trailer as suggested, since the commit message structure was provided by Junio C Hamano.
>
> Also, a quick question please: since the GSoC proposal period for 2026 has closed, could you guide me on the next steps for applying to subsequent related internships (like Outreachy, if applicable)? I would love to know how I can best continue contributing to Git in the meantime.
>
> Regards,
> Zakariyah Ali.
>
>  t/t2000-conflict-when-checking-files-out.sh | 99 +++++++++++----------
>  1 file changed, 54 insertions(+), 45 deletions(-)
>
> diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
> index f18616ad2b..a8a49df93e 100755
> --- a/t/t2000-conflict-when-checking-files-out.sh
> +++ b/t/t2000-conflict-when-checking-files-out.sh
> @@ -48,17 +48,16 @@ mkdir path0
>  date >path0/file0
>  date >path1
>
> -test_expect_success \
> -    'git checkout-index without -f should fail on conflicting work tree.' \
> -    'test_must_fail git checkout-index -a'
> -
> -test_expect_success \
> -    'git checkout-index with -f should succeed.' \
> -    'git checkout-index -f -a'
> +test_expect_success 'git checkout-index without -f should fail on conflicting work tree.' '
> +	test_must_fail git checkout-index -a
> +'
>

Not on you, but generally the data setup happens in the same block as
the test, in this case we're relying on previously setup data. But this
is already better than before.

> -test_expect_success \
> -    'git checkout-index conflicting paths.' \
> -    'test -f path0 && test -d path1 && test -f path1/file1'
> +test_expect_success 'git checkout-index with -f should succeed.' '
> +	git checkout-index -f -a &&
> +	test_path_is_file path0 &&
> +	test_path_is_dir path1 &&
> +	test_path_is_file path1/file1
> +'
>

Okay we combine the two tests which were doing the execution and
validation independently.

>  test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
>  	mkdir -p tar/get &&
> @@ -83,53 +82,63 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
>  # path path3 is occupied by a non-directory.  With "-f" it should remove
>  # the symlink path3 and create directory path3 and file path3/file1.
>
> -mkdir path2
> -date >path2/file0
> -test_expect_success \
> -    'git update-index --add path2/file0' \
> -    'git update-index --add path2/file0'
> -test_expect_success \
> -    'writing tree out with git write-tree' \
> -    'tree1=$(git write-tree)'
> +test_expect_success 'prepare path2/file0 and index' '
> +	mkdir path2 &&
> +	date >path2/file0 &&
> +	git update-index --add path2/file0
> +'
> +
> +test_expect_success 'write tree with path2/file0' '
> +	tree1=$(git write-tree)
> +'
> +
>  test_debug 'show_files $tree1'
>
> -mkdir path3
> -date >path3/file1
> -test_expect_success \
> -    'git update-index --add path3/file1' \
> -    'git update-index --add path3/file1'
> -test_expect_success \
> -    'writing tree out with git write-tree' \
> -    'tree2=$(git write-tree)'
> +test_expect_success 'prepare path3/file1 and index' '
> +	mkdir path3 &&
> +	date >path3/file1 &&
> +	git update-index --add path3/file1
> +'
> +
> +test_expect_success 'write tree with path3/file1' '
> +	tree2=$(git write-tree)
> +'
> +
>  test_debug 'show_files $tree2'
>
> -rm -fr path3
> -test_expect_success \
> -    'read previously written tree and checkout.' \
> -    'git read-tree -m $tree1 && git checkout-index -f -a'
> +test_expect_success 'read previously written tree and checkout.' '
> +	rm -fr path3 &&
> +	git read-tree -m $tree1 &&
> +	git checkout-index -f -a
> +'
> +
>  test_debug 'show_files $tree1'
>
> -test_expect_success \
> -    'add a symlink' \
> -    'test_ln_s_add path2 path3'
> -test_expect_success \
> -    'writing tree out with git write-tree' \
> -    'tree3=$(git write-tree)'
> +test_expect_success 'add a symlink' '
> +	test_ln_s_add path2 path3
> +'
> +
> +test_expect_success 'write tree with symlink path3' '
> +	tree3=$(git write-tree)
> +'
> +
>  test_debug 'show_files $tree3'
>
>  # Morten says "Got that?" here.
>  # Test begins.
>
> -test_expect_success \
> -    'read previously written tree and checkout.' \
> -    'git read-tree $tree2 && git checkout-index -f -a'
> +test_expect_success 'read previously written tree and checkout.' '
> +	git read-tree $tree2 &&
> +	git checkout-index -f -a
> +'
> +
>  test_debug 'show_files $tree2'
>
> -test_expect_success \
> -    'checking out conflicting path with -f' \
> -    'test ! -h path2 && test -d path2 &&
> -     test ! -h path3 && test -d path3 &&
> -     test ! -h path2/file0 && test -f path2/file0 &&
> -     test ! -h path3/file1 && test -f path3/file1'
> +test_expect_success 'checking out conflicting path with -f' '
> +	test_path_is_dir_not_symlink path2 &&
> +	test_path_is_dir_not_symlink path3 &&
> +	test_path_is_file_not_symlink path2/file0 &&
> +	test_path_is_file_not_symlink path3/file1
> +'
>

Shouldn't all the tests above (since 'mkdir path2') be a single test
block? First we setup the data, validate the data, the previous test
runs 'git-checkout-index' and finally here we're verifying the endstate
here. I think this should all fit into one test block.

Thanks,
Karthik

>  test_done
> --
> 2.43.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v4 1/1] t2000: modernize overall structure and path checks

[Tian Yuchen]

On 4/6/26 06:04, Karthik Nayak wrote:
> 
> Shouldn't all the tests above (since 'mkdir path2') be a single test
> block? First we setup the data, validate the data, the previous test
> runs 'git-checkout-index' and finally here we're verifying the endstate
> here. I think this should all fit into one test block.

That makes sense. I didn’t mention this earlier because I hadn’t noticed 
that the patch title had changed from 't2000: modernise path checks with 
test_path_is_* helpers' to 't2000: modernise overall structure'. If this 
is a overall refactor, then it would be worth revising this outdated 
test structure as well ;)

> 
> Thanks,
> Karthik
> 
>>   test_done
>> --
>> 2.43.0

Regards, Yuchen

[PATCH v5] t2000: modernize overall structure and path checks

[Zakariyah Ali]

This test script that dates back to 2005 certainly shows its age and
both its style and the way the tests are laid out do not match the
modern standard. Modernize it to match the current testing standards:

  * Executables that prepare the data used to test the command should
    be inside the test_expect_success block in modern tests.

  * In modern tests, running a command that is being tested, making
    sure it succeeds, and inspecting other side effects that are
    expected, are all done in a single test_expect_success block.

  * A test_expect_success block in modern tests are laid out as

         test_expect_success 'title of the test' '
                 body of the test &&
                 ...
                 body of the test
         '

    not as

         test_expect_success \
                 'title of the test' \
                 'body of the test &&
                 ...
                 body of the test'

    which is in a prehistoric style.

  * In modern tests, each &&-chained statement in the body of the
    test_expect_success block are indented with a horizontal tab,
    unlike prehistoric style that used 4-space indent.

  * Replace bare 'test -f/-d' and 'test ! -h' assertions with dedicated
    test_path_is_* helpers (specifically test_path_is_file_not_symlink and
    test_path_is_dir_not_symlink). While less commonly used in the test
    suite than test_path_is_file/dir, they act as direct replacements
    for the specific checks being performed and provide clearer
    diagnostics on failure.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Tian Yuchen <cat@malon.dev>
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
 t/t2000-conflict-when-checking-files-out.sh | 88 +++++++--------------
 1 file changed, 30 insertions(+), 58 deletions(-)

diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index f18616ad2b..44728329f3 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -48,17 +48,16 @@ mkdir path0
 date >path0/file0
 date >path1
 
-test_expect_success \
-    'git checkout-index without -f should fail on conflicting work tree.' \
-    'test_must_fail git checkout-index -a'
-
-test_expect_success \
-    'git checkout-index with -f should succeed.' \
-    'git checkout-index -f -a'
+test_expect_success 'git checkout-index without -f should fail on conflicting work tree.' '
+	test_must_fail git checkout-index -a
+'
 
-test_expect_success \
-    'git checkout-index conflicting paths.' \
-    'test -f path0 && test -d path1 && test -f path1/file1'
+test_expect_success 'git checkout-index with -f should succeed.' '
+	git checkout-index -f -a &&
+	test_path_is_file path0 &&
+	test_path_is_dir path1 &&
+	test_path_is_file path1/file1
+'
 
 test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 	mkdir -p tar/get &&
@@ -83,53 +82,26 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 # path path3 is occupied by a non-directory.  With "-f" it should remove
 # the symlink path3 and create directory path3 and file path3/file1.
 
-mkdir path2
-date >path2/file0
-test_expect_success \
-    'git update-index --add path2/file0' \
-    'git update-index --add path2/file0'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree1=$(git write-tree)'
-test_debug 'show_files $tree1'
-
-mkdir path3
-date >path3/file1
-test_expect_success \
-    'git update-index --add path3/file1' \
-    'git update-index --add path3/file1'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree2=$(git write-tree)'
-test_debug 'show_files $tree2'
-
-rm -fr path3
-test_expect_success \
-    'read previously written tree and checkout.' \
-    'git read-tree -m $tree1 && git checkout-index -f -a'
-test_debug 'show_files $tree1'
-
-test_expect_success \
-    'add a symlink' \
-    'test_ln_s_add path2 path3'
-test_expect_success \
-    'writing tree out with git write-tree' \
-    'tree3=$(git write-tree)'
-test_debug 'show_files $tree3'
-
-# Morten says "Got that?" here.
-# Test begins.
-
-test_expect_success \
-    'read previously written tree and checkout.' \
-    'git read-tree $tree2 && git checkout-index -f -a'
-test_debug 'show_files $tree2'
-
-test_expect_success \
-    'checking out conflicting path with -f' \
-    'test ! -h path2 && test -d path2 &&
-     test ! -h path3 && test -d path3 &&
-     test ! -h path2/file0 && test -f path2/file0 &&
-     test ! -h path3/file1 && test -f path3/file1'
+test_expect_success 'checkout-index -f resolves symlink conflict on leading path' '
+	mkdir path2 &&
+	date >path2/file0 &&
+	git update-index --add path2/file0 &&
+	tree1=$(git write-tree) &&
+	mkdir path3 &&
+	date >path3/file1 &&
+	git update-index --add path3/file1 &&
+	tree2=$(git write-tree) &&
+	rm -fr path3 &&
+	git read-tree -m $tree1 &&
+	git checkout-index -f -a &&
+	test_ln_s_add path2 path3 &&
+	tree3=$(git write-tree) &&
+	git read-tree $tree2 &&
+	git checkout-index -f -a &&
+	test_path_is_dir_not_symlink path2 &&
+	test_path_is_dir_not_symlink path3 &&
+	test_path_is_file_not_symlink path2/file0 &&
+	test_path_is_file_not_symlink path3/file1
+'
 
 test_done
-- 
2.43.0

Re: [PATCH v5] t2000: modernize overall structure and path checks

[Junio C Hamano]

Zakariyah Ali <zakariyahali100@gmail.com> writes:

> This test script that dates back to 2005 certainly shows its age and
> both its style and the way the tests are laid out do not match the
> modern standard. Modernize it to match the current testing standards:
> ...

How does this relate to d8e34f97 (t2000: modernise overall
structure, 2026-03-28) that was merged to 'next' at 279c41a3 (Merge
branch 'za/t2000-modernise' into next, 2026-03-31) and is now in
'master' at 0713d3b7 (Merge branch 'za/t2000-modernise', 2026-04-06)?

The topioc appeared first in the issue 2026/03 #12 of the "What's
cooking" report (Mar 30th), marked for 'next'.  Then the issue
2026/04 #01 of the report (Apr 1st) listed the topic in 'next'
slated for 'master'.  The issue 2026/03 #02 (Apr 6th) reports it is
now in 'master'.

The description of this v5 patch looks suspiciously similar, as its
patch text, so I suspect it won't apply to my tree.

Re: [PATCH v5] t2000: modernize overall structure and path checks

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> The description of this v5 patch looks suspiciously similar, as its
> patch text, so I suspect it won't apply to my tree.

So, I applied this "v5" to a slightly older 'master', immediately
before the za/t2000-modernise topic was merged, and compared the
result with what we already have in 'master'.

    $ git log -1 --oneline 0713d3b7f6
    0713d3b7f6 Merge branch 'za/t2000-modernise'
    $ git checkout 0713d3b7f6~1
    $ git am patch-v5.mbox
    $ git diff 0713d3b7f6 HEAD

There are some things that are better, and some that do not look
improvements.

> diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
> index af199d8191..44728329f3 100755
> --- a/t/t2000-conflict-when-checking-files-out.sh
> +++ b/t/t2000-conflict-when-checking-files-out.sh
> @@ -35,19 +35,18 @@ show_files() {
>  	sed -e 's/^\([0-9]*\)	[^ ]*	[0-9a-f]*	/tr: \1 /'
>  }
>  
> -test_expect_success 'prepare files path0 and path1/file1' '
> -	date >path0 &&
> -	mkdir path1 &&
> -	date >path1/file1 &&
> -	git update-index --add path0 path1/file1
> -'
> +date >path0
> +mkdir path1
> +date >path1/file1
>  
> -test_expect_success 'prepare working tree files with D/F conflicts' '
> -	rm -fr path0 path1 &&
> -	mkdir path0 &&
> -	date >path0/file0 &&
> -	date >path1
> -'
> +test_expect_success \
> +    'git update-index --add various paths.' \
> +    'git update-index --add path0 path1/file1'
> +
> +rm -fr path0 path1
> +mkdir path0
> +date >path0/file0
> +date >path1

All of the above look regression to me, for the purpose of
"modernization" effort.  We want the steps to prepare for tests
(e.g., creation of test files and directories and preparation of
their contents), the steps of actual tests (e.g., running git
commands and ensuring that they succeed or fail as expected), and
the steps to verify the results, all contained inside a single
test_expect_success for each step of the test. 

On the other hand, the below looks like moving things in a better
direction.  The original has a logically single test split into
multiple pieces and code to debug tests sprinkled all over,
like ...

> @@ -83,59 +82,22 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
>  # path path3 is occupied by a non-directory.  With "-f" it should remove
>  # the symlink path3 and create directory path3 and file path3/file1.
>  
> -test_expect_success 'prepare path2/file0 and index' '
> +test_expect_success 'checkout-index -f resolves symlink conflict on leading path' '
>  	mkdir path2 &&
>  	date >path2/file0 &&
> -	git update-index --add path2/file0
> -'
> -
> -test_expect_success 'write tree with path2/file0' '
> -	tree1=$(git write-tree)
> -'
> -
> -test_debug 'show_files $tree1'

... this one.  And consolidating them into a single logical piece
may make sense.

But it seems not quite complete and needs a bit more cleaning.  For
example, ...

> -test_expect_success 'prepare path3/file1 and index' '
> +	git update-index --add path2/file0 &&
> +	tree1=$(git write-tree) &&
>  	mkdir path3 &&
>  	date >path3/file1 &&
> -	git update-index --add path3/file1
> -'
> -
> -test_expect_success 'write tree with path3/file1' '
> -	tree2=$(git write-tree)
> -'
> -
> -test_debug 'show_files $tree2'
> -
> -test_expect_success 'read previously written tree and checkout.' '
> +	git update-index --add path3/file1 &&
> +	tree2=$(git write-tree) &&
>  	rm -fr path3 &&
>  	git read-tree -m $tree1 &&
> -	git checkout-index -f -a
> -'
> -
> -test_debug 'show_files $tree1'
> -
> -test_expect_success 'add a symlink' '
> -	test_ln_s_add path2 path3
> -'
> -
> -test_expect_success 'write tree with symlink path3' '
> -	tree3=$(git write-tree)
> -'

... we used to write out $tree3 here only because ...

> -
> -test_debug 'show_files $tree3'

... we use it to debug that tree here.  In the updated version, we
still write out ...

> -# Morten says "Got that?" here.
> -# Test begins.
> -
> -test_expect_success 'read previously written tree and checkout.' '
> +	git checkout-index -f -a &&
> +	test_ln_s_add path2 path3 &&
> +	tree3=$(git write-tree) &&

... the same tree3 here, but because we lost the test debug, we no
longer use the resulting tree object name.

>  	git read-tree $tree2 &&
> -	git checkout-index -f -a
> -'
> -
> -test_debug 'show_files $tree2'
> -
> -test_expect_success 'checking out conflicting path with -f' '
> +	git checkout-index -f -a &&
>  	test_path_is_dir_not_symlink path2 &&
>  	test_path_is_dir_not_symlink path3 &&
>  	test_path_is_file_not_symlink path2/file0 &&

I didn't go through the updated version with fine toothed comb, so
there may be other "why is this thing left?" and/or "this update
changes what is being tested, no?" gotchas that I missed.

In any case, can you update the patch so that it applies cleanly to
a more recent "master" to resurrect the good bits out of what you
have?

Thanks.

[PATCH v6] t2000: consolidate second scenario into a single test block

[Zakariyah Ali]

The second test scenario in t2000 consists of several fragmented
test_expect_success blocks that handle data setup, tree writes,
execution of git-checkout-index, and final state validation.

Consolidate these nine separate blocks into a single self-contained
test block. This follows the modern Git testing standard where setup,
execution, and validation of a single logical scenario are kept
together.

As a result of this consolidation, the show_files() helper and its
associated test_debug calls are no longer used and have been removed.
This also removes a dependency on the non-portable 'find -ls' command.

Helped-by: Karthik Nayak <karthik.188@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
I'm sorry for replying very late; it was because of some commitments I had to attend to which I've been able to settle.

Changes since v5:
- Consolidate segmented test blocks in the second scenario into a single unit.
- Remove unused show_files debug function and tree3 variable.
- Ensure file setup is inside test_expect_success blocks.

 t/t2000-conflict-when-checking-files-out.sh | 65 +++------------------
 1 file changed, 8 insertions(+), 57 deletions(-)

diff --git a/t/t2000-conflict-when-checking-files-out.sh b/t/t2000-conflict-when-checking-files-out.sh
index af199d8191..7b61370549 100755
--- a/t/t2000-conflict-when-checking-files-out.sh
+++ b/t/t2000-conflict-when-checking-files-out.sh
@@ -23,17 +23,6 @@ test_description='git conflicts when checking files out test.'
 
 . ./test-lib.sh
 
-show_files() {
-	# show filesystem files, just [-dl] for type and name
-	find path? -ls |
-	sed -e 's/^[0-9]* * [0-9]* * \([-bcdl]\)[^ ]* *[0-9]* *[^ ]* *[^ ]* *[0-9]* [A-Z][a-z][a-z] [0-9][0-9] [^ ]* /fs: \1 /'
-	# what's in the cache, just mode and name
-	git ls-files --stage |
-	sed -e 's/^\([0-9]*\) [0-9a-f]* [0-3] /ca: \1 /'
-	# what's in the tree, just mode and name.
-	git ls-tree -r "$1" |
-	sed -e 's/^\([0-9]*\)	[^ ]*	[0-9a-f]*	/tr: \1 /'
-}
 
 test_expect_success 'prepare files path0 and path1/file1' '
 	date >path0 &&
@@ -83,59 +72,21 @@ test_expect_success SYMLINKS 'checkout-index -f twice with --prefix' '
 # path path3 is occupied by a non-directory.  With "-f" it should remove
 # the symlink path3 and create directory path3 and file path3/file1.
 
-test_expect_success 'prepare path2/file0 and index' '
+test_expect_success 'checkout-index -f resolves symlink conflict on leading path' '
 	mkdir path2 &&
 	date >path2/file0 &&
-	git update-index --add path2/file0
-'
-
-test_expect_success 'write tree with path2/file0' '
-	tree1=$(git write-tree)
-'
-
-test_debug 'show_files $tree1'
-
-test_expect_success 'prepare path3/file1 and index' '
+	git update-index --add path2/file0 &&
+	tree1=$(git write-tree) &&
 	mkdir path3 &&
 	date >path3/file1 &&
-	git update-index --add path3/file1
-'
-
-test_expect_success 'write tree with path3/file1' '
-	tree2=$(git write-tree)
-'
-
-test_debug 'show_files $tree2'
-
-test_expect_success 'read previously written tree and checkout.' '
+	git update-index --add path3/file1 &&
+	tree2=$(git write-tree) &&
 	rm -fr path3 &&
 	git read-tree -m $tree1 &&
-	git checkout-index -f -a
-'
-
-test_debug 'show_files $tree1'
-
-test_expect_success 'add a symlink' '
-	test_ln_s_add path2 path3
-'
-
-test_expect_success 'write tree with symlink path3' '
-	tree3=$(git write-tree)
-'
-
-test_debug 'show_files $tree3'
-
-# Morten says "Got that?" here.
-# Test begins.
-
-test_expect_success 'read previously written tree and checkout.' '
+	git checkout-index -f -a &&
+	test_ln_s_add path2 path3 &&
 	git read-tree $tree2 &&
-	git checkout-index -f -a
-'
-
-test_debug 'show_files $tree2'
-
-test_expect_success 'checking out conflicting path with -f' '
+	git checkout-index -f -a &&
 	test_path_is_dir_not_symlink path2 &&
 	test_path_is_dir_not_symlink path3 &&
 	test_path_is_file_not_symlink path2/file0 &&
-- 
2.43.0

Re: [PATCH v6] t2000: consolidate second scenario into a single test block

[Zakariyah Ali]

Hi everyone,

Just a gentle reminder on this v6 patch:
https://lore.kernel.org/git/20260429103607.406339-1-zakariyahali100@gmail.com/

I would be looking forward to your review.

Thanks,
Zakariyah Ali

Re: [PATCH v6] t2000: consolidate second scenario into a single test block

[Junio C Hamano]

Zakariyah Ali <zakariyahali100@gmail.com> writes:

> The second test scenario in t2000 consists of several fragmented
> test_expect_success blocks that handle data setup, tree writes,
> execution of git-checkout-index, and final state validation.
>
> Consolidate these nine separate blocks into a single self-contained
> test block. This follows the modern Git testing standard where setup,
> execution, and validation of a single logical scenario are kept
> together.
>
> As a result of this consolidation, the show_files() helper and its
> associated test_debug calls are no longer used and have been removed.
> This also removes a dependency on the non-portable 'find -ls' command.

The patch, at first glance, looked quite messy but it turns out that
it is mostly just a lot of removals of (1) early test closure
followed by the start of the next test or (2) test_debug calls in
between.  The only thing that was slightly outside that pattern was
the computation of tree3, whose result was not even used for
test_debug in the original.

Will mark the topic for 'next'.  Thanks.

Re: [PATCH v6] t2000: consolidate second scenario into a single test

[Zakariyah Ali]

Dear Junio,

Thank you very much for your patient, guidance and feedback throughout the development of this patch series. It has been an invaluable learning experience for me.

While my initial goal with these contributions was to participate in GSoC internship, but I was unable to do so this time, however I have found the process of contributing to the Git ecosystem very rewarding. I am excited to stay involved and look forward to making more contributions in the future.

Also, I am a software engineer with over four years of experience in the field. I am currently seeking new opportunities, specifically entry-level or internship roles where I can continue to grow. If you happen to know of any openings or could offer any advice or assistance, I would be extremely grateful.

Thank you again for your time and for everything you do for the Git project.

Best regards,
Zakariyah Ali