t9146: replace test -d/-f with appropriate test_path_is_* function

PATCH Superseded Chandra Pratap via GitGitGadget 3 participants 5 messages Feb 11, 2024 – Feb 14, 2024

lore

Share & tools

lore

Chandra Pratap via GitGitGadget

author

2024-02-11 14:53 UTC #0 lore raw

PATCH 1/1 t9146: replace test -d/-f with appropriate test_path_is_* function

From: Chandra Pratap <[email protected]>

The helper functions test_path_is_* provide better debugging information than test -d/-e/-f.

Replace "! test -d" with "test_path_is_missing" at places where we check for non-existent directories.

Replace "test -f" with "test_path_is_file" and "test -d" with "test_path_is_dir" at places where we expect files or directories to exist.

Signed-off-by Chandra Pratap <[email protected]>

---
t9146: replace test -d/-f with appropriate test_path_is_* function

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1661%2FChand-ra%2Ftestfix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1661/Chand-ra/testfix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1661

 t/t9146-git-svn-empty-dirs.sh | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

t/t9146-git-svn-empty-dirs.sh +12 ~~-12~~

index 09606f1b3cf..532f5baa208 100755
@@ -20,7 +20,7 @@ test_expect_success 'empty directories exist' '
 		cd cloned &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
+			if test_path_is_missing "$i"
 			then
 				echo >&2 "$i does not exist" &&
 				exit 1

@@ -37,7 +37,7 @@ test_expect_success 'option automkdirs set to false' '
 		git svn fetch &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if test -d "$i"
+			if test_path_is_dir "$i"
 			then
 				echo >&2 "$i exists" &&
 				exit 1

@@ -52,7 +52,7 @@ test_expect_success 'more emptiness' '
 
 test_expect_success 'git svn rebase creates empty directory' '
 	( cd cloned && git svn rebase ) &&
-	test -d cloned/"! !"
+	test_path_is_dir cloned/"! !"
 '
 
 test_expect_success 'git svn mkdirs recreates empty directories' '

@@ -62,7 +62,7 @@ test_expect_success 'git svn mkdirs recreates empty directories' '
 		git svn mkdirs &&
 		for i in a b c d d/e d/e/f "weird file name" "! !"
 		do
-			if ! test -d "$i"
+			if test_path_is_missing "$i"
 			then
 				echo >&2 "$i does not exist" &&
 				exit 1

@@ -78,21 +78,21 @@ test_expect_success 'git svn mkdirs -r works' '
 		git svn mkdirs -r7 &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
+			if test_path_is_missing "$i"
 			then
 				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done &&
 
-		if test -d "! !"
+		if test_path_is_dir "! !"
 		then
 			echo >&2 "$i should not exist" &&
 			exit 1
 		fi &&
 
 		git svn mkdirs -r8 &&
-		if ! test -d "! !"
+		if test_path_is_missing "! !"
 		then
 			echo >&2 "$i not exist" &&
 			exit 1

@@ -114,7 +114,7 @@ test_expect_success 'empty directories in trunk exist' '
 		cd trunk &&
 		for i in a "weird file name"
 		do
-			if ! test -d "$i"
+			if test_path_is_missing "$i"
 			then
 				echo >&2 "$i does not exist" &&
 				exit 1

@@ -138,16 +138,16 @@ test_expect_success 'git svn gc-ed files work' '
 		cd removed &&
 		git svn gc &&
 		: Compress::Zlib may not be available &&
-		if test -f "$unhandled".gz
+		if test_path_is_file "$unhandled".gz
 		then
 			svn_cmd mkdir -m gz "$svnrepo"/gz &&
 			git reset --hard $(git rev-list HEAD | tail -1) &&
 			git svn rebase &&
-			test -f "$unhandled".gz &&
-			test -f "$unhandled" &&
+			test_path_is_file "$unhandled".gz &&
+			test_path_is_file "$unhandled" &&
 			for i in a b c "weird file name" gz "! !"
 			do
-				if ! test -d "$i"
+				if test_path_is_missing "$i"
 				then
 					echo >&2 "$i does not exist" &&
 					exit 1

base-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821

Signature · 1 line

gitgitgadget

Eric Sunshine

2024-02-11 17:58 UTC #1 lore raw

Quoted 9 lines · Chandra Pratap via GitGitGadget

The helper functions test_path_is_* provide better debugging information than test -d/-e/-f.
Replace "! test -d" with "test_path_is_missing" at places where we check for non-existent directories.
Replace "test -f" with "test_path_is_file" and "test -d" with "test_path_is_dir" at places where we expect files or directories to exist.

The aim of this patch makes sense, but the implementation needs some refinement...

t/t9146-git-svn-empty-dirs.sh test_expect_success 'empty directories exist' ' · line 25

Quoted 9 lines

t/t9146-git-svn-empty-dirs.sh +1 -1

@@ -20,7 +20,7 @@ test_expect_success 'empty directories exist' '
                for i in a b c d d/e d/e/f "weird file name"
                do
-                       if ! test -d "$i"
+                       if test_path_is_missing "$i"
                        then
                                echo >&2 "$i does not exist" &&
                                exit 1

The point of functions such as test_path_is_missing() is to _assert_ that some condition is true, thus allowing the test to succeed; if the condition is not true, then the function prints an error message and the test aborts and fails.

    test_path_is_missing () {
        if test -e "$1"
        then
            echo "Path exists:"
            ls -ld "$1"
            false
        fi
    }

It is meant to replace noisy code such as:

    if ! test -f bloop
    then
        echo >&2 "error message" &&
        exit 1
    fi &&
    other-code

with much simpler:

    test_path_exists bloop &&
    other-code

So, the changes made by this patch are incorrect in two ways...

First, the patch retains code which prints an error message even though this code becomes redundant since the test_path_foo() functions already take care of printing the error message.

Second, and more problematic, the patch incorrectly inverts the sense of what is being tested. For instance, the title of this test is "empty directories exist", and the body of the test asserts that the empty directories _do_ exist, but the patch changes the condition to assert that the directories do _not_ exist, which is wrong.

Taking these observations into account, this test should become:

    test_expect_success 'empty directories exist' '
        (
            cd cloned &&
            for i in a b c d d/e d/e/f "weird file name"
            do
                test_path_exists "$i" || exit 1
            done
        )
    '

Many of the other changes made by this patch suffer similar problems

t/t9146-git-svn-empty-dirs.sh test_expect_success 'git svn gc-ed files work' ' · line 142

Quoted 7 lines

t/t9146-git-svn-empty-dirs.sh +1 -1

@@ -138,16 +138,16 @@ test_expect_success 'git svn gc-ed files work' '
                : Compress::Zlib may not be available &&
-               if test -f "$unhandled".gz
+               if test_path_is_file "$unhandled".gz
                then
                        svn_cmd mkdir -m gz "$svnrepo"/gz &&
                        git reset --hard $(git rev-list HEAD | tail -1) &&

This change is wrong/undesirable for a different reason. Taking into account what was said above about test_path_is_file() being an _assertion_ that some condition is true, coupled with the comment above this if statement which says "Compress::Zlib may not be available", then this test -f is legitimately part of the control-flow of the function. It is not a mere assertion. Thus, replacing it with the assertion function test_path_is_file() breaks the test for the case when Compress::Zlib is not available.

t/t9146-git-svn-empty-dirs.sh test_expect_success 'git svn gc-ed files work' ' · line 147

Quoted 4 lines

t/t9146-git-svn-empty-dirs.sh +2 -2

-                       test -f "$unhandled".gz &&
-                       test -f "$unhandled" &&
+                       test_path_is_file "$unhandled".gz &&
+                       test_path_is_file "$unhandled" &&

These replacements are correct in that they replace the _assertion_ test -f with the equivalent assertion test_path_is_file.

Chandra Pratap via GitGitGadget

author

2024-02-12 19:17 UTC #2 lore raw

PATCH v2 1/1 t9146: replace test -d/-e/-f with appropriate test_path_is_* function

In reply to Chandra Pratap via GitGitGadget

From: Chandra Pratap <[email protected]>

The helper functions test_path_is_* provide better debugging information than test -d/-e/-f.

Replace "if ! test -d then <error message>" with "test_path_exists" and "test -d" with "test_path_is_dir" at places where we check for existent directories.

Replace "test -f" with "test_path_is_file" at places where we check for existent files.

Replace "test ! -e" with "test_path_is_missing" where we check for non-existent directories.

Helped-by Eric Sunshine <[email protected]>

Signed-off-by Chandra Pratap <[email protected]>

---
t9146: replace test -d/-f with appropriate test_path_is_* function

cc Eric Sunshine [email protected]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1661%2FChand-ra%2Ftestfix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1661/Chand-ra/testfix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1661

Range-diff vs v1:

1: 93fe9e9eef7 ! 1: 5734b9edd61 t9146: replace test -d/-f with appropriate test_path_is_* function
@@ Metadata
Author: Chandra Pratap <[email protected]>

## Commit message ##
- t9146: replace test -d/-f with appropriate test_path_is_* function
+ t9146: replace test -d/-e/-f with appropriate test_path_is_* function

The helper functions test_path_is_* provide better debugging information than test -d/-e/-f.

- Replace "! test -d" with "test_path_is_missing" at places where
- we check for non-existent directories.
+ Replace "if ! test -d then <error message>" with "test_path_exists"
+ and "test -d" with "test_path_is_dir" at places where we check for
+ existent directories.

- Replace "test -f" with "test_path_is_file" and "test -d" with
- "test_path_is_dir" at places where we expect files or directories
- to exist.
+ Replace "test -f" with "test_path_is_file" at places where we check
+ for existent files.

+ Replace "test ! -e" with "test_path_is_missing" where we check for
+ non-existent directories.
+
+ Helped-by: Eric Sunshine <[email protected]>

Signed-off-by Chandra Pratap <[email protected]>

## t/t9146-git-svn-empty-dirs.sh ##
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'empty directories exist' '
for i in a b c d d/e d/e/f "weird file name"
do
- if ! test -d "$i"
-+ if test_path_is_missing "$i"
- then
- echo >&2 "$i does not exist" &&
- exit 1
+- then
+- echo >&2 "$i does not exist" &&
+- exit 1
+- fi
++ test_path_exists "$i" || exit 1
+ done
+ )
+ '
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'option automkdirs set to false' '
git svn fetch &&
for i in a b c d d/e d/e/f "weird file name"
do
- if test -d "$i"
-+ if test_path_is_dir "$i"
- then
- echo >&2 "$i exists" &&
- exit 1
+- then
+- echo >&2 "$i exists" &&
+- exit 1
+- fi
++ test_path_is_missing "$i" || exit 1
+ done
+ )
+ '
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'more emptiness' '

test_expect_success 'git svn rebase creates empty directory' '
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn mkdirs recreates emp
for i in a b c d d/e d/e/f "weird file name" "! !"
do
- if ! test -d "$i"
-+ if test_path_is_missing "$i"
- then
- echo >&2 "$i does not exist" &&
- exit 1
+- then
+- echo >&2 "$i does not exist" &&
+- exit 1
+- fi
++ test_path_exists "$i" || exit 1
+ done
+ )
+ '
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn mkdirs -r works' '
git svn mkdirs -r7 &&
for i in a b c d d/e d/e/f "weird file name"
do
- if ! test -d "$i"
-+ if test_path_is_missing "$i"
- then
- echo >&2 "$i does not exist" &&
- exit 1
- fi
+- then
+- echo >&2 "$i does not exist" &&
+- exit 1
+- fi
++ test_path_exists "$i" || exit 1
done &&

- if test -d "! !"
-+ if test_path_is_dir "! !"
- then
- echo >&2 "$i should not exist" &&
- exit 1
- fi &&
+- then
+- echo >&2 "$i should not exist" &&
+- exit 1
+- fi &&
++ test_path_is_missing "! !" || exit 1 &&

git svn mkdirs -r8 &&
- if ! test -d "! !"
-+ if test_path_is_missing "! !"
- then
- echo >&2 "$i not exist" &&
- exit 1
+- then
+- echo >&2 "$i not exist" &&
+- exit 1
+- fi
++ test_path_exists "! !" || exit 1
+ )
+ '
+
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'empty directories in trunk exist' '
cd trunk &&
for i in a "weird file name"
do
- if ! test -d "$i"
-+ if test_path_is_missing "$i"
- then
- echo >&2 "$i does not exist" &&
- exit 1
+- then
+- echo >&2 "$i does not exist" &&
+- exit 1
+- fi
++ test_path_exists "$i" || exit 1
+ done
+ )
+ '
+@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'remove a top-level directory from svn' ' +
+ test_expect_success 'removed top-level directory does not exist' '
+ git svn clone "$svnrepo" removed &&
+- test ! -e removed/d
++ test_path_is_missing removed/d
+
+ '
+ unhandled=.git/svn/refs/remotes/git-svn/unhandled.log
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn gc-ed files work' '
- cd removed &&
- git svn gc &&
- : Compress::Zlib may not be available &&
-- if test -f "$unhandled".gz
-+ if test_path_is_file "$unhandled".gz
- then
svn_cmd mkdir -m gz "$svnrepo"/gz &&
git reset --hard $(git rev-list HEAD | tail -1) &&
git svn rebase &&
@@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn gc-ed files work' '
for i in a b c "weird file name" gz "! !"
do
- if ! test -d "$i"
-+ if test_path_is_missing "$i"
- then
- echo >&2 "$i does not exist" &&
- exit 1
+- then
+- echo >&2 "$i does not exist" &&
+- exit 1
+- fi
++ test_path_exists "$i" || exit 1
+ done
+ fi
+ )

 t/t9146-git-svn-empty-dirs.sh | 56 ++++++++---------------------------
 1 file changed, 12 insertions(+), 44 deletions(-)

t/t9146-git-svn-empty-dirs.sh +12 ~~-44~~

index 09606f1b3cf..6bf94ad802c 100755
@@ -20,11 +20,7 @@ test_expect_success 'empty directories exist' '
 		cd cloned &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_exists "$i" || exit 1
 		done
 	)
 '

@@ -37,11 +33,7 @@ test_expect_success 'option automkdirs set to false' '
 		git svn fetch &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if test -d "$i"
-			then
-				echo >&2 "$i exists" &&
-				exit 1
-			fi
+			test_path_is_missing "$i" || exit 1
 		done
 	)
 '

@@ -52,7 +44,7 @@ test_expect_success 'more emptiness' '
 
 test_expect_success 'git svn rebase creates empty directory' '
 	( cd cloned && git svn rebase ) &&
-	test -d cloned/"! !"
+	test_path_is_dir cloned/"! !"
 '
 
 test_expect_success 'git svn mkdirs recreates empty directories' '

@@ -62,11 +54,7 @@ test_expect_success 'git svn mkdirs recreates empty directories' '
 		git svn mkdirs &&
 		for i in a b c d d/e d/e/f "weird file name" "! !"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_exists "$i" || exit 1
 		done
 	)
 '

@@ -78,25 +66,13 @@ test_expect_success 'git svn mkdirs -r works' '
 		git svn mkdirs -r7 &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_exists "$i" || exit 1
 		done &&
 
-		if test -d "! !"
-		then
-			echo >&2 "$i should not exist" &&
-			exit 1
-		fi &&
+		test_path_is_missing "! !" || exit 1 &&
 
 		git svn mkdirs -r8 &&
-		if ! test -d "! !"
-		then
-			echo >&2 "$i not exist" &&
-			exit 1
-		fi
+		test_path_exists "! !" || exit 1
 	)
 '

@@ -114,11 +90,7 @@ test_expect_success 'empty directories in trunk exist' '
 		cd trunk &&
 		for i in a "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_exists "$i" || exit 1
 		done
 	)
 '

@@ -129,7 +101,7 @@ test_expect_success 'remove a top-level directory from svn' '
 
 test_expect_success 'removed top-level directory does not exist' '
 	git svn clone "$svnrepo" removed &&
-	test ! -e removed/d
+	test_path_is_missing removed/d
 
 '
 unhandled=.git/svn/refs/remotes/git-svn/unhandled.log

@@ -143,15 +115,11 @@ test_expect_success 'git svn gc-ed files work' '
 			svn_cmd mkdir -m gz "$svnrepo"/gz &&
 			git reset --hard $(git rev-list HEAD | tail -1) &&
 			git svn rebase &&
-			test -f "$unhandled".gz &&
-			test -f "$unhandled" &&
+			test_path_is_file "$unhandled".gz &&
+			test_path_is_file "$unhandled" &&
 			for i in a b c "weird file name" gz "! !"
 			do
-				if ! test -d "$i"
-				then
-					echo >&2 "$i does not exist" &&
-					exit 1
-				fi
+				test_path_exists "$i" || exit 1
 			done
 		fi
 	)

base-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821

Signature · 1 line

gitgitgadget

Junio C Hamano

2024-02-12 20:31 UTC #3 lore raw

Re: [PATCH v2] t9146: replace test -d/-e/-f with appropriate test_path_is_* function

Quoted 4 lines · Chandra Pratap via GitGitGadget

From: Chandra Pratap <[email protected]>
The helper functions test_path_is_* provide better debugging information than test -d/-e/-f.

Correct.

Quoted 3 lines

Replace "if ! test -d then <error message>" with "test_path_exists" and "test -d" with "test_path_is_dir" at places where we check for existent directories.

The former could result in misconversion, if the intention of the test was "we cannot have directory here; a regular file is OK", so we have to be a bit more careful than mechanical conversion.

Quoted 2 lines

Replace "test -f" with "test_path_is_file" at places where we check for existent files.

OK.

Quoted 2 lines

Replace "test ! -e" with "test_path_is_missing" where we check for non-existent directories.

OK.

t/t9146-git-svn-empty-dirs.sh test_expect_success 'empty directories exist' ' · line 23

Quoted 8 lines

t/t9146-git-svn-empty-dirs.sh +1 -5

 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_exists "$i" || exit 1

We were saying that we are OK if "$i" existed as a file (not a directory), but now we complain regardless of what "$i" is. Is that closer to what the test originally wanted to do? Just checking.

t/t9146-git-svn-empty-dirs.sh test_expect_success 'option automkdirs set to false' ' · line 36

Quoted 13 lines

t/t9146-git-svn-empty-dirs.sh +1 -5

 		done
 	)
 '

@@ -37,11 +33,7 @@ test_expect_success 'option automkdirs set to false' '
 		git svn fetch &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if test -d "$i"
-			then
-				echo >&2 "$i exists" &&
-				exit 1
-			fi
+			test_path_is_missing "$i" || exit 1

Ditto; are we sure the intention of the original is that nothing should be at "$i" (instead of "as long as it is not a directory, we are OK")? Just checking.

The same comment applies to all conversions to test_path_exists and test_path_is_missing where the original was not "test -e" or "! test -e". The other ones, like the change from "test -f" to "test_path_is_file", looked all correct.

Thanks.

Chandra Pratap via GitGitGadget

author

2024-02-14 17:50 UTC #4 lore raw

PATCH v3 1/1 t9146: replace test -d/-e/-f with appropriate test_path_is_* function

In reply to Chandra Pratap via GitGitGadget

From: Chandra Pratap <[email protected]>

The helper functions test_path_is_* provide better debugging information than test -d/-e/-f.

Replace "if ! test -d then <error message>" and "test -d" with "test_path_is_dir" at places where we check for existent directories.

Replace "test -f" with "test_path_is_file" at places where we check for existent files.

Replace "test ! -e" and "if test -d then <error message>" with "test_path_is_missing" where we check for non-existent directories.

Helped-by Eric Sunshine <[email protected]>

Signed-off-by Chandra Pratap <[email protected]>

---
t9146: replace test -d/-f with appropriate test_path_is_* function

I chose to retain "test_path_is_misssing" as a replacement for "if test -d then " because we initialize the repository at the start of the test with:

for i in a b c d d/e d/e/f "weird file name" do svn_cmd mkdir -m "mkdir $i" "$svnrepo"/"$i" || return 1 done

and then check for the existence of these directories in the following tests. I think this reproduces the behavior of the original tests close enough.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1661%2FChand-ra%2Ftestfix-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1661/Chand-ra/testfix-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1661

Range-diff vs v2:

1: 5734b9edd61 ! 1: 5024389e7a9 t9146: replace test -d/-e/-f with appropriate test_path_is_* function

     @@ Commit message
          The helper functions test_path_is_* provide better debugging
          information than test -d/-e/-f.
      
     -    Replace "if ! test -d then <error message>" with "test_path_exists"
     -    and "test -d" with "test_path_is_dir" at places where we check for
     -    existent directories.
     +    Replace "if ! test -d then <error message>" and "test -d" with
     +    "test_path_is_dir" at places where we check for existent directories.
      
          Replace "test -f" with "test_path_is_file" at places where we check
          for existent files.
      
     -    Replace "test ! -e" with "test_path_is_missing" where we check for
     -    non-existent directories.
     +    Replace "test ! -e" and "if test -d then <error message>" with
     +    "test_path_is_missing" where we check for non-existent directories.
      
          Helped-by: Eric Sunshine <[email protected]>
          Signed-off-by: Chandra Pratap <[email protected]>
     @@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'empty directories exist' '
      -				echo >&2 "$i does not exist" &&
      -				exit 1
      -			fi
     -+			test_path_exists "$i" || exit 1
     ++			test_path_is_dir "$i" || exit 1
       		done
       	)
       '
     @@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn mkdirs recreates emp
      -				echo >&2 "$i does not exist" &&
      -				exit 1
      -			fi
     -+			test_path_exists "$i" || exit 1
     ++			test_path_is_dir "$i" || exit 1
       		done
       	)
       '
     @@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn mkdirs -r works' '
      -				echo >&2 "$i does not exist" &&
      -				exit 1
      -			fi
     -+			test_path_exists "$i" || exit 1
     ++			test_path_is_dir "$i" || exit 1
       		done &&
       
      -		if test -d "! !"
     @@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn mkdirs -r works' '
      -			echo >&2 "$i not exist" &&
      -			exit 1
      -		fi
     -+		test_path_exists "! !" || exit 1
     ++		test_path_is_dir "! !" || exit 1
       	)
       '
       
     @@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'empty directories in trunk e
      -				echo >&2 "$i does not exist" &&
      -				exit 1
      -			fi
     -+			test_path_exists "$i" || exit 1
     ++			test_path_is_dir "$i" || exit 1
       		done
       	)
       '
     @@ t/t9146-git-svn-empty-dirs.sh: test_expect_success 'git svn gc-ed files work' '
      -					echo >&2 "$i does not exist" &&
      -					exit 1
      -				fi
     -+				test_path_exists "$i" || exit 1
     ++				test_path_is_dir "$i" || exit 1
       			done
       		fi
       	)

 t/t9146-git-svn-empty-dirs.sh | 56 ++++++++---------------------------
 1 file changed, 12 insertions(+), 44 deletions(-)

t/t9146-git-svn-empty-dirs.sh +12 ~~-44~~

index 09606f1b3cf..926ac814394 100755
@@ -20,11 +20,7 @@ test_expect_success 'empty directories exist' '
 		cd cloned &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_is_dir "$i" || exit 1
 		done
 	)
 '

@@ -37,11 +33,7 @@ test_expect_success 'option automkdirs set to false' '
 		git svn fetch &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if test -d "$i"
-			then
-				echo >&2 "$i exists" &&
-				exit 1
-			fi
+			test_path_is_missing "$i" || exit 1
 		done
 	)
 '

@@ -52,7 +44,7 @@ test_expect_success 'more emptiness' '
 
 test_expect_success 'git svn rebase creates empty directory' '
 	( cd cloned && git svn rebase ) &&
-	test -d cloned/"! !"
+	test_path_is_dir cloned/"! !"
 '
 
 test_expect_success 'git svn mkdirs recreates empty directories' '

@@ -62,11 +54,7 @@ test_expect_success 'git svn mkdirs recreates empty directories' '
 		git svn mkdirs &&
 		for i in a b c d d/e d/e/f "weird file name" "! !"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_is_dir "$i" || exit 1
 		done
 	)
 '

@@ -78,25 +66,13 @@ test_expect_success 'git svn mkdirs -r works' '
 		git svn mkdirs -r7 &&
 		for i in a b c d d/e d/e/f "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_is_dir "$i" || exit 1
 		done &&
 
-		if test -d "! !"
-		then
-			echo >&2 "$i should not exist" &&
-			exit 1
-		fi &&
+		test_path_is_missing "! !" || exit 1 &&
 
 		git svn mkdirs -r8 &&
-		if ! test -d "! !"
-		then
-			echo >&2 "$i not exist" &&
-			exit 1
-		fi
+		test_path_is_dir "! !" || exit 1
 	)
 '

@@ -114,11 +90,7 @@ test_expect_success 'empty directories in trunk exist' '
 		cd trunk &&
 		for i in a "weird file name"
 		do
-			if ! test -d "$i"
-			then
-				echo >&2 "$i does not exist" &&
-				exit 1
-			fi
+			test_path_is_dir "$i" || exit 1
 		done
 	)
 '

@@ -129,7 +101,7 @@ test_expect_success 'remove a top-level directory from svn' '
 
 test_expect_success 'removed top-level directory does not exist' '
 	git svn clone "$svnrepo" removed &&
-	test ! -e removed/d
+	test_path_is_missing removed/d
 
 '
 unhandled=.git/svn/refs/remotes/git-svn/unhandled.log

@@ -143,15 +115,11 @@ test_expect_success 'git svn gc-ed files work' '
 			svn_cmd mkdir -m gz "$svnrepo"/gz &&
 			git reset --hard $(git rev-list HEAD | tail -1) &&
 			git svn rebase &&
-			test -f "$unhandled".gz &&
-			test -f "$unhandled" &&
+			test_path_is_file "$unhandled".gz &&
+			test_path_is_file "$unhandled" &&
 			for i in a b c "weird file name" gz "! !"
 			do
-				if ! test -d "$i"
-				then
-					echo >&2 "$i does not exist" &&
-					exit 1
-				fi
+				test_path_is_dir "$i" || exit 1
 			done
 		fi
 	)

base-commit: 235986be822c9f8689be2e9a0b7804d0b1b6d821

Signature · 1 line

gitgitgadget