* [PATCH] t9146: replace test -d/-f with appropriate test_path_is_* function
@ 2024-02-11 14:53 Chandra Pratap via GitGitGadget
2024-02-11 17:58 ` Eric Sunshine
2024-02-12 19:17 ` [PATCH v2] t9146: replace test -d/-e/-f " Chandra Pratap via GitGitGadget
0 siblings, 2 replies; 5+ messages in thread
From: Chandra Pratap via GitGitGadget @ 2024-02-11 14:53 UTC (permalink / raw)
To: git; +Cc: Chandra Pratap, Chandra Pratap
From: Chandra Pratap <chandrapratap3519@gmail.com>
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 <chandrapratap3519@gmail.com>
---
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(-)
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 09606f1b3cf..532f5baa208 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] t9146: replace test -d/-f with appropriate test_path_is_* function
2024-02-11 14:53 [PATCH] t9146: replace test -d/-f with appropriate test_path_is_* function Chandra Pratap via GitGitGadget
@ 2024-02-11 17:58 ` Eric Sunshine
2024-02-12 19:17 ` [PATCH v2] t9146: replace test -d/-e/-f " Chandra Pratap via GitGitGadget
1 sibling, 0 replies; 5+ messages in thread
From: Eric Sunshine @ 2024-02-11 17:58 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
On Sun, Feb 11, 2024 at 9:53 AM Chandra Pratap via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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...
> diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
> @@ -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
> @@ -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.
> - 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`.
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] t9146: replace test -d/-e/-f with appropriate test_path_is_* function
2024-02-11 14:53 [PATCH] t9146: replace test -d/-f with appropriate test_path_is_* function Chandra Pratap via GitGitGadget
2024-02-11 17:58 ` Eric Sunshine
@ 2024-02-12 19:17 ` Chandra Pratap via GitGitGadget
2024-02-12 20:31 ` Junio C Hamano
2024-02-14 17:50 ` [PATCH v3] " Chandra Pratap via GitGitGadget
1 sibling, 2 replies; 5+ messages in thread
From: Chandra Pratap via GitGitGadget @ 2024-02-12 19:17 UTC (permalink / raw)
To: git; +Cc: Chandra Pratap, Chandra Pratap
From: Chandra Pratap <chandrapratap3519@gmail.com>
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 <sunshine@sunshineco.com>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
t9146: replace test -d/-f with appropriate test_path_is_* function
cc: Eric Sunshine sunshine@sunshineco.com
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 <chandrapratap3519@gmail.com>
## 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 <sunshine@sunshineco.com>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
## 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(-)
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 09606f1b3cf..6bf94ad802c 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] t9146: replace test -d/-e/-f with appropriate test_path_is_* function
2024-02-12 19:17 ` [PATCH v2] t9146: replace test -d/-e/-f " Chandra Pratap via GitGitGadget
@ 2024-02-12 20:31 ` Junio C Hamano
2024-02-14 17:50 ` [PATCH v3] " Chandra Pratap via GitGitGadget
1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2024-02-12 20:31 UTC (permalink / raw)
To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Chandra Pratap <chandrapratap3519@gmail.com>
>
> The helper functions test_path_is_* provide better debugging
> information than test -d/-e/-f.
Correct.
> 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.
> Replace "test -f" with "test_path_is_file" at places where we check
> for existent files.
OK.
> Replace "test ! -e" with "test_path_is_missing" where we check for
> non-existent directories.
OK.
> 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.
> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] t9146: replace test -d/-e/-f with appropriate test_path_is_* function
2024-02-12 19:17 ` [PATCH v2] t9146: replace test -d/-e/-f " Chandra Pratap via GitGitGadget
2024-02-12 20:31 ` Junio C Hamano
@ 2024-02-14 17:50 ` Chandra Pratap via GitGitGadget
1 sibling, 0 replies; 5+ messages in thread
From: Chandra Pratap via GitGitGadget @ 2024-02-14 17:50 UTC (permalink / raw)
To: git; +Cc: Chandra Pratap, Chandra Pratap
From: Chandra Pratap <chandrapratap3519@gmail.com>
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 <sunshine@sunshineco.com>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
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 <sunshine@sunshineco.com>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
@@ 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(-)
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 09606f1b3cf..926ac814394 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-14 17:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-11 14:53 [PATCH] t9146: replace test -d/-f with appropriate test_path_is_* function Chandra Pratap via GitGitGadget
2024-02-11 17:58 ` Eric Sunshine
2024-02-12 19:17 ` [PATCH v2] t9146: replace test -d/-e/-f " Chandra Pratap via GitGitGadget
2024-02-12 20:31 ` Junio C Hamano
2024-02-14 17:50 ` [PATCH v3] " Chandra Pratap via GitGitGadget
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.