t2000: modernize path checks to use helper functions

This is my GSoC microproject submission modernizing test path checks in t/t2000-conflict-when-checking-files-out.sh.

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

This improves test clarity and consistency across the test suite.

I'm excited to contribute to Git and look forward to your feedback!

Zakariyah Ali (2):
t2000: consolidate second scenario into a single test block
t2000: cleanup unused debug code and variables

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

base-commit: 60f07c4f5c5f81c8a994d9e06b31a4a3a1679864
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2256%2Falibaba0010%2Fmodernize-test-path-checking-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2256/alibaba0010/modernize-test-path-checking-v1
Pull-Request: https://github.com/git/git/pull/2256

gitgitgadget

From: Zakariyah Ali <[email protected]>

Now that the test script has been modernised, consolidate the eight separate test_expect_success blocks that together form the second test scenario (setup, tree writes, checkout, symlink creation, and final state check) into one self-contained block.

This makes it easier to read: data set-up, the operations being tested, and the expected outcome are now all in one place.

---
 t/t2000-conflict-when-checking-files-out.sh | 55 ++++-----------------
 1 file changed, 9 insertions(+), 46 deletions(-)

index af199d8191..43ec901f9e 100755
@@ -83,59 +83,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'
-
-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 &&
+	tree3=$(git write-tree) &&
 	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 &&

gitgitgadget

From: Zakariyah Ali <[email protected]>

Remove the show_files function which is no longer used after removing test_debug calls, and remove an unused tree3 variable assignment in the second test scenario.

These cleanups address feedback from Junio C Hamano regarding the modernization of this test script.

---
 t/t2000-conflict-when-checking-files-out.sh | 12 ------------
 1 file changed, 12 deletions(-)

index 43ec901f9e..7b61370549 100755
@@ -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 &&

@@ -96,7 +85,6 @@ test_expect_success 'checkout-index -f resolves symlink conflict on leading path
 	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 &&

gitgitgadget

I do not quite get where you intend to fit these two patches.

This is fairly old, v2.54.0-rc2~9.

But the thing is, your earlier clean-up to this t2000 script 4a9e0972 (t2000: consolidate second scenario into a single test block, 2026-04-29) was queued on za/t2000-modernise-more was merged to 'master' at b5d94909 (Merge branch 'za/t2000-modernise-more', 2026-05-21). But what is most curious about these two patches is that the [PATCH 1/2] starts like so:

From: Zakariyah Ali <[email protected]>

Now that the test script has been modernised, consolidate the eight separate test_expect_success blocks ...

I take that to be a reference to your previous effort in za/t2000-modernise-more topic. But these two patches are changing the code as if that did not even exist.

If the za/t2000-modernise-more topic were still not merged to 'next', sending in replacement patches works just fine. but a new patch that ignores anything that have already been merged to 'next' or 'master' is counter-productive.