[Outreachy] [PATCH v2] t/t7601: use "test_path_is_file"etc. instead of "test -f"

Dorcas AnonoLitunya· Oct 18, 2023, 12:45 UTC · lore

Some tests in t7601 use "test -f" and "test ! -f" to see if a path exists or is missing.

Use test_path_is_file and test_path_is_missing helper functions to clarify these tests a bit better. This especially matters for the "missing" case because "test ! -f F" will be happy if "F" exists as a directory, but the intent of the test is that "F" should not exist, even as a directory. The updated code expresses this better.

Signed-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>
---
 t/t7601-merge-pull-config.sh | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Show changes to t/t7601-merge-pull-config.sh +12 −12

diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index bd238d89b0..e08767df66 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '
 
 test_expect_success 'merge c1 with c2' '
 	git reset --hard c1 &&
-	test -f c0.c &&
-	test -f c1.c &&
-	test ! -f c2.c &&
-	test ! -f c3.c &&
+	test_path_is_file c0.c &&
+	test_path_is_file c1.c &&
+	test_path_is_missing c2.c &&
+	test_path_is_missing c3.c &&
 	git merge c2 &&
-	test -f c1.c &&
-	test -f c2.c
+	test_path_is_file c1.c &&
+	test_path_is_file c2.c
 '
 
 test_expect_success 'fast-forward pull succeeds with "true" in pull.ff' '
@@ -411,8 +411,8 @@ test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
 	git reset --hard c1 &&
 	git config pull.twohead ours &&
 	git merge c2 &&
-	test -f c1.c &&
-	! test -f c2.c
+	test_path_is_file c1.c &&
+	test_path_is_missing c2.c
 '
 
 test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
@@ -431,10 +431,10 @@ test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octo
 	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
 	test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
 	git diff --exit-code &&
-	test -f c0.c &&
-	test -f c1.c &&
-	test -f c2.c &&
-	test -f c3.c
+	test_path_is_file c0.c &&
+	test_path_is_file c1.c &&
+	test_path_is_file c2.c &&
+	test_path_is_file c3.c
 '
 
 conflict_count()

-- 
2.42.0.345.gaab89be2eb

Junio C Hamano· Oct 18, 2023, 23:59 UTC · re: Dorcas AnonoLitunya · lore

Re: [Outreachy] [PATCH v2] t/t7601: use "test_path_is_file"etc. instead of "test -f"

Dorcas AnonoLitunya <anonolitunya@gmail.com> writes:

Show 13 quoted lines

> Some tests in t7601 use "test -f" and "test ! -f" to see if a path
> exists or is missing.
>
> Use test_path_is_file and test_path_is_missing helper functions to
> clarify these tests a bit better. This especially matters for the
> "missing" case because "test ! -f F" will be happy if "F" exists as a
> directory, but the intent of the test is that "F" should not exist, even
> as a directory. The updated code expresses this better.
>
> Signed-off-by: Dorcas AnonoLitunya <anonolitunya@gmail.com>
> ---
>  t/t7601-merge-pull-config.sh | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)

Nicely done; it seems that the title line got garbled whitespace for some reason but I'll fix them up while queuing.

Thanks.  Will queue.

Show 50 quoted lines

> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> index bd238d89b0..e08767df66 100755
> --- a/t/t7601-merge-pull-config.sh
> +++ b/t/t7601-merge-pull-config.sh
> @@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '
>  
>  test_expect_success 'merge c1 with c2' '
>  	git reset --hard c1 &&
> -	test -f c0.c &&
> -	test -f c1.c &&
> -	test ! -f c2.c &&
> -	test ! -f c3.c &&
> +	test_path_is_file c0.c &&
> +	test_path_is_file c1.c &&
> +	test_path_is_missing c2.c &&
> +	test_path_is_missing c3.c &&
>  	git merge c2 &&
> -	test -f c1.c &&
> -	test -f c2.c
> +	test_path_is_file c1.c &&
> +	test_path_is_file c2.c
>  '
>  
>  test_expect_success 'fast-forward pull succeeds with "true" in pull.ff' '
> @@ -411,8 +411,8 @@ test_expect_success 'merge c1 with c2 (ours in pull.twohead)' '
>  	git reset --hard c1 &&
>  	git config pull.twohead ours &&
>  	git merge c2 &&
> -	test -f c1.c &&
> -	! test -f c2.c
> +	test_path_is_file c1.c &&
> +	test_path_is_missing c2.c
>  '
>  
>  test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
> @@ -431,10 +431,10 @@ test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octo
>  	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
>  	test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
>  	git diff --exit-code &&
> -	test -f c0.c &&
> -	test -f c1.c &&
> -	test -f c2.c &&
> -	test -f c3.c
> +	test_path_is_file c0.c &&
> +	test_path_is_file c1.c &&
> +	test_path_is_file c2.c &&
> +	test_path_is_file c3.c
>  '
>  
>  conflict_count()

v2t/t7601: use "test_path_is_file"etc. instead of "test -f"