Git development
 help / color / mirror / Atom feed
From: Zakariyah Ali <zakariyahali100@gmail.com>
To: git@vger.kernel.org
Cc: Zakariyah Ali <zakariyahali100@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v4 1/1] t2000: modernize overall structure and path checks
Date: Sun,  5 Apr 2026 02:11:34 +0100	[thread overview]
Message-ID: <20260405011135.125912-1-zakariyahali100@gmail.com> (raw)
In-Reply-To: <20260327234019.95591-1-zakariyahali100@gmail.com>

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


  parent reply	other threads:[~2026-04-05  1:11 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-26  0:15 Github Patch Zakariyah Ali
2026-03-26  0:54 ` Pablo
2026-03-26 19:26 ` [GSoC PATCH v2] t2000: modernize path checks with test_path_is_* helpers Zakariyah Ali
2026-03-26 20:29   ` Junio C Hamano
2026-03-27 23:40   ` [GSoC][PATCH v3] t2000: modernise overall structure Zakariyah Ali
2026-03-30 12:31     ` Zakariyah Ali
2026-04-01 17:09     ` Tian Yuchen
2026-04-05  1:11     ` Zakariyah Ali [this message]
2026-04-05 22:04       ` [PATCH v4 1/1] t2000: modernize overall structure and path checks Karthik Nayak
2026-04-06 17:36         ` Tian Yuchen
2026-04-07  3:44       ` [PATCH v5] " Zakariyah Ali
2026-04-07 14:29         ` Junio C Hamano
2026-04-07 16:10           ` Junio C Hamano
2026-04-29 10:36         ` [PATCH v6] t2000: consolidate second scenario into a single test block Zakariyah Ali
2026-05-05  6:42           ` Zakariyah Ali
2026-05-12  6:15           ` Junio C Hamano
2026-05-12 20:01           ` [PATCH v6] t2000: consolidate second scenario into a single test Zakariyah Ali

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260405011135.125912-1-zakariyahali100@gmail.com \
    --to=zakariyahali100@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox