[GSoC][PATCH] t2000: modernize path checks to use helper functions

[GSoC][PATCH] t2000: modernize path checks to use helper functions

[Zakariyah Ali]

Replace bare 'test -f/-d' and 'test ! -h ... && test -f/-d'
assertions with their dedicated test_path_is_* helpers.

These helpers are better than the previous 'test' commands
because they produce clearer diagnostic output on failure.
For example, instead of a bare exit code, the helper outputs
'File path0 doesn't exist', which makes test failures
significantly easier to debug.

The replacements are:
- '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'

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] t2000: modernize path checks to use helper functions

[Pablo]

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

This should be sent as v2 in reply to your previous v1, not a new thread.
read Documentation/SubmittingPatches and
Documentation/MyFirstContribution as I mentioned in your v1.

>
> Replace bare 'test -f/-d' and 'test ! -h ... && test -f/-d'
> assertions with their dedicated test_path_is_* helpers.
>
> These helpers are better than the previous 'test' commands
> because they produce clearer diagnostic output on failure.
> For example, instead of a bare exit code, the helper outputs
> 'File path0 doesn't exist', which makes test failures
> significantly easier to debug.
>
> The replacements are:
> - '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'
>

The commit message is better now, but it's very verbose. The list
about what helpers have you changed can be looked at by any reviewer
with git log -p and the example only describes one helper, you should
be able to make it more general with something like: reports 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
>
>

Code looks OK.