* [PATCH] t7001-mv.sh:modernizing test script using function
@ 2022-10-30 17:20 Debra Obondo via GitGitGadget
2022-10-30 18:00 ` Taylor Blau
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Debra Obondo via GitGitGadget @ 2022-10-30 17:20 UTC (permalink / raw)
To: git; +Cc: Debra Obondo, Debra Obondo
From: Debra Obondo <debraobondo@gmail.com>
Test script to verify the presence/absence of files, paths, directories,
symlinks and other features in 'git mv' command are using the command
format:
'test (-e|f|d|h|...)'
Replace them with helper functions of format:
'test_path_is_*'
Signed-off-by: Debra Obondo <debraobondo@gmail.com>
---
[PATCH] [OUTREACHY] t7001-mv.sh : Use test_path_is_* functions in test
script
Older test scripts use the command 'test -' to verify the presence or
absence of features such as files, directories and symbolic links. This
however requires slightly complicated uses, such as 'test ! -f '. The
helper functions 'test_path_is_' located in t/test-lib-functions.sh have
taken into account all scenarios of the 'test -*' to reduce errors. This
was a microproject to replace them with the helper functions.
Test script to verify the presence/absence of files, paths,
directories,symboliclinks and many other features in mv command are
using the command format
'test -(e|f|s|h|...).
Replace them with helper functions of format
'test_path_is_*'
Signed-off-by: Debra Obondo debraobondo@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1372%2Ffobiasic07%2Ft7001-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1372/fobiasic07/t7001-v1
Pull-Request: https://github.com/git/git/pull/1372
t/t7001-mv.sh | 62 +++++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 8c37bceb336..aa97a2f8e1c 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -60,8 +60,8 @@ test_expect_success 'checking the commit' '
test_expect_success 'mv --dry-run does not move file' '
git mv -n path0/COPYING MOVED &&
- test -f path0/COPYING &&
- test ! -f MOVED
+ test_path_is_file path0/COPYING &&
+ ! test_path_is_file MOVED
'
test_expect_success 'checking -k on non-existing file' '
@@ -71,25 +71,25 @@ test_expect_success 'checking -k on non-existing file' '
test_expect_success 'checking -k on untracked file' '
>untracked1 &&
git mv -k untracked1 path0 &&
- test -f untracked1 &&
- test ! -f path0/untracked1
+ test_path_is_file untracked1 &&
+ ! test_path_is_file path0/untracked1
'
test_expect_success 'checking -k on multiple untracked files' '
>untracked2 &&
git mv -k untracked1 untracked2 path0 &&
- test -f untracked1 &&
- test -f untracked2 &&
- test ! -f path0/untracked1 &&
- test ! -f path0/untracked2
+ test_path_is_file untracked1 &&
+ test_path_is_file untracked2 &&
+ ! test_path_is_file path0/untracked1 &&
+ ! test_path_is_file path0/untracked2
'
test_expect_success 'checking -f on untracked file with existing target' '
>path0/untracked1 &&
test_must_fail git mv -f untracked1 path0 &&
- test ! -f .git/index.lock &&
- test -f untracked1 &&
- test -f path0/untracked1
+ ! test_path_is_file .git/index.lock &&
+ test_path_is_file untracked1 &&
+ test_path_is_file path0/untracked1
'
# clean up the mess in case bad things happen
@@ -215,8 +215,8 @@ test_expect_success 'absolute pathname' '
git add sub/file &&
git mv sub "$(pwd)/in" &&
- ! test -d sub &&
- test -d in &&
+ ! test_path_is_dir sub &&
+ test_path_is_dir in &&
git ls-files --error-unmatch in/file
)
'
@@ -234,8 +234,8 @@ test_expect_success 'absolute pathname outside should fail' '
git add sub/file &&
test_must_fail git mv sub "$out/out" &&
- test -d sub &&
- ! test -d ../in &&
+ test_path_is_dir sub &&
+ ! test_path_is_dir ../in &&
git ls-files --error-unmatch sub/file
)
'
@@ -295,8 +295,8 @@ test_expect_success 'git mv should overwrite symlink to a file' '
git add moved &&
test_must_fail git mv moved symlink &&
git mv -f moved symlink &&
- ! test -e moved &&
- test -f symlink &&
+ ! test_path_exists moved &&
+ test_path_is_file symlink &&
test "$(cat symlink)" = 1 &&
git update-index --refresh &&
git diff-files --quiet
@@ -312,13 +312,13 @@ test_expect_success 'git mv should overwrite file with a symlink' '
git add moved &&
test_must_fail git mv symlink moved &&
git mv -f symlink moved &&
- ! test -e symlink &&
+ ! test_path_exists symlink &&
git update-index --refresh &&
git diff-files --quiet
'
test_expect_success SYMLINKS 'check moved symlink' '
- test -h moved
+ test_path_is_symlink moved
'
rm -f moved symlink
@@ -352,7 +352,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm
) &&
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
+ ! test_path_exists sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -372,7 +372,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu
) &&
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
+ ! test_path_exists sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ -389,7 +389,7 @@ test_expect_success 'git mv moves a submodule with gitfile' '
entry="$(git ls-files --stage sub | cut -f 1)" &&
mkdir mod &&
git -C mod mv ../sub/ . &&
- ! test -e sub &&
+ ! test_path_exists sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ -408,7 +408,7 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '
mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
+ ! test_path_exists sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -423,13 +423,13 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta
entry="$(git ls-files --stage sub | cut -f 1)" &&
mkdir mod &&
test_must_fail git mv sub mod/sub 2>actual.err &&
- test -s actual.err &&
- test -e sub &&
+ test_file_not_empty actual.err &&
+ test_path_exists sub &&
git diff-files --quiet -- sub &&
git add .gitmodules &&
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
+ ! test_path_exists sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -447,7 +447,7 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule
mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_cmp expect.err actual.err &&
- ! test -e sub &&
+ ! test_path_exists sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -460,7 +460,7 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' '
git submodule update &&
mkdir mod &&
git mv -n sub mod/sub 2>actual.err &&
- test -f sub/.git &&
+ test_path_is_file sub/.git &&
git diff-index --exit-code HEAD &&
git update-index --refresh &&
git diff-files --quiet -- sub .gitmodules
@@ -474,10 +474,10 @@ test_expect_success 'checking out a commit before submodule moved needs manual u
git status -s sub2 >actual &&
echo "?? sub2/" >expected &&
test_cmp expected actual &&
- ! test -f sub/.git &&
- test -f sub2/.git &&
+ ! test_path_is_file sub/.git &&
+ test_path_is_file sub2/.git &&
git submodule update &&
- test -f sub/.git &&
+ test_path_is_file sub/.git &&
rm -rf sub2 &&
git diff-index --exit-code HEAD &&
git update-index --refresh &&
base-commit: 1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886
--
gitgitgadget
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] t7001-mv.sh:modernizing test script using function
2022-10-30 17:20 [PATCH] t7001-mv.sh:modernizing test script using function Debra Obondo via GitGitGadget
@ 2022-10-30 18:00 ` Taylor Blau
2022-10-31 18:04 ` Martin Ågren
2022-11-03 18:39 ` [PATCH v2] t7001-mv.sh: modernizing test script using functions Debra Obondo via GitGitGadget
2 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2022-10-30 18:00 UTC (permalink / raw)
To: Debra Obondo via GitGitGadget; +Cc: git, Debra Obondo
On Sun, Oct 30, 2022 at 05:20:41PM +0000, Debra Obondo via GitGitGadget wrote:
> t/t7001-mv.sh | 62 +++++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 31 deletions(-)
Looks good. Will queue, thanks.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] t7001-mv.sh:modernizing test script using function
2022-10-30 17:20 [PATCH] t7001-mv.sh:modernizing test script using function Debra Obondo via GitGitGadget
2022-10-30 18:00 ` Taylor Blau
@ 2022-10-31 18:04 ` Martin Ågren
2022-11-01 1:14 ` Taylor Blau
2022-11-03 18:39 ` [PATCH v2] t7001-mv.sh: modernizing test script using functions Debra Obondo via GitGitGadget
2 siblings, 1 reply; 8+ messages in thread
From: Martin Ågren @ 2022-10-31 18:04 UTC (permalink / raw)
To: Debra Obondo via GitGitGadget; +Cc: Git Mailing List, Debra Obondo
Hi Debra,
On Sun, 30 Oct 2022 at 18:35, Debra Obondo via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Debra Obondo <debraobondo@gmail.com>
>
> Test script to verify the presence/absence of files, paths, directories,
> symlinks and other features in 'git mv' command are using the command
> format:
>
> 'test (-e|f|d|h|...)'
>
> Replace them with helper functions of format:
>
> 'test_path_is_*'
This is a good idea.
The subject of this patch could use a space after the colon. You could
also write "modernize" to give an order to the code base. So something
like
t7001-mv.sh: modernize test script using function
perhaps. "Function" is a bit vague, perhaps.
I wanted to comment on this:
> test_expect_success 'mv --dry-run does not move file' '
> git mv -n path0/COPYING MOVED &&
> - test -f path0/COPYING &&
> - test ! -f MOVED
> + test_path_is_file path0/COPYING &&
> + ! test_path_is_file MOVED
> '
It is my understanding that we prefer to only use such a helper when we
really expect the file to exist. If the path is not a file, this helper
prints a helpful message before returning with an error.
Here, this means we will emit this 'helpful'
File MOVED doesn't exist
on every test run, when really everything is as it should. And if the
file is actually there, i.e., we have a bug, we'll emit nothing -- but
that is precisely when we would want some diagnostics such as
Path exists:
... MOVED ...
to show us that the file actually exists, contrary to the test's
expectations.
Such output is precisely what `test_path_is_missing` would give us. :-)
My gut feeling is that where this patch adds "! test_path_foo", it
should use "test_path_bar" instead, for various values of "foo" and
"bar". What do you think about that?
Martin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] t7001-mv.sh:modernizing test script using function
2022-10-31 18:04 ` Martin Ågren
@ 2022-11-01 1:14 ` Taylor Blau
0 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2022-11-01 1:14 UTC (permalink / raw)
To: Martin Ågren
Cc: Debra Obondo via GitGitGadget, Git Mailing List, Debra Obondo
On Mon, Oct 31, 2022 at 07:04:20PM +0100, Martin Ågren wrote:
> Hi Debra,
>
> On Sun, 30 Oct 2022 at 18:35, Debra Obondo via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Debra Obondo <debraobondo@gmail.com>
> >
> > Test script to verify the presence/absence of files, paths, directories,
> > symlinks and other features in 'git mv' command are using the command
> > format:
> >
> > 'test (-e|f|d|h|...)'
> >
> > Replace them with helper functions of format:
> >
> > 'test_path_is_*'
>
> This is a good idea.
>
> The subject of this patch could use a space after the colon. You could
> also write "modernize" to give an order to the code base. So something
> like
>
> t7001-mv.sh: modernize test script using function
>
> perhaps. "Function" is a bit vague, perhaps.
>
> I wanted to comment on this:
>
> > test_expect_success 'mv --dry-run does not move file' '
> > git mv -n path0/COPYING MOVED &&
> > - test -f path0/COPYING &&
> > - test ! -f MOVED
> > + test_path_is_file path0/COPYING &&
> > + ! test_path_is_file MOVED
> > '
>
> It is my understanding that we prefer to only use such a helper when we
> really expect the file to exist. If the path is not a file, this helper
> prints a helpful message before returning with an error.
>
> Here, this means we will emit this 'helpful'
>
> File MOVED doesn't exist
>
> on every test run, when really everything is as it should. And if the
> file is actually there, i.e., we have a bug, we'll emit nothing -- but
> that is precisely when we would want some diagnostics such as
>
> Path exists:
> ... MOVED ...
>
> to show us that the file actually exists, contrary to the test's
> expectations.
>
> Such output is precisely what `test_path_is_missing` would give us. :-)
>
> My gut feeling is that where this patch adds "! test_path_foo", it
> should use "test_path_bar" instead, for various values of "foo" and
> "bar". What do you think about that?
All good suggestions, thanks. I'll hold this back while we wait for a
rerolled version.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] t7001-mv.sh: modernizing test script using functions
2022-10-30 17:20 [PATCH] t7001-mv.sh:modernizing test script using function Debra Obondo via GitGitGadget
2022-10-30 18:00 ` Taylor Blau
2022-10-31 18:04 ` Martin Ågren
@ 2022-11-03 18:39 ` Debra Obondo via GitGitGadget
2022-11-03 19:37 ` Martin Ågren
2022-11-04 15:05 ` [PATCH v3] " Debra Obondo via GitGitGadget
2 siblings, 2 replies; 8+ messages in thread
From: Debra Obondo via GitGitGadget @ 2022-11-03 18:39 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Martin Ågren, Debra Obondo, Debra Obondo
From: Debra Obondo <debraobondo@gmail.com>
Test script to verify the presence/absence of files, paths, directories,
symlinks and other features in 'git mv' command are using the command
format:
'test (-e|f|d|h|...)'
Replace them with helper functions of format:
'test_path_is_*'
Changes since v1
Replacing idiomatic helper functions
'! test_path_is_*'
with
'test_path_is_missing'
This uses values of 'test_path_bar' in place of '! test_path_foo' to
bring in the helpful factor of indicating the failure of tests after the
mv command has been used, that is, it echoes if the feature/test_path
exists.
Signed-off-by: Debra Obondo <debraobondo@gmail.com>
---
[PATCH v2] [OUTREACHY] t7001-mv.sh : Use test_path_is_* functions in
test script
Changes since v1:
Replacing idiomatic helper functions
'! test_path_is_*'
with
'test_path_is_missing'
This uses values of 'test_path_bar' in place of '! test_path_foo' to
bring in the helpful factor of indicating the failure of tests after the
mv command has been used, that is, it echoes if the feature/test_path
exists (This was suggested by Martin Ågren).
Older test scripts use the command 'test -' to verify the presence or
absence of features such as files, directories and symbolic links. This
however requires slightly complicated uses, such as 'test ! -f '. The
helper functions 'test_path_is_' located in t/test-lib-functions.sh have
taken into account all scenarios of the 'test -*' to reduce errors. This
was a microproject to replace them with the helper functions.
Test script to verify the presence/absence of files, paths,
directories,symboliclinks and many other features in mv command are
using the command format
'test -(e|f|s|h|...).
Replace them with helper functions of format
'test_path_is_*'
Signed-off-by: Debra Obondo debraobondo@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1372%2Ffobiasic07%2Ft7001-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1372/fobiasic07/t7001-v2
Pull-Request: https://github.com/git/git/pull/1372
Range-diff vs v1:
1: b977c4ad26a ! 1: c6640ebff66 t7001-mv.sh:modernizing test script using function
@@ Metadata
Author: Debra Obondo <debraobondo@gmail.com>
## Commit message ##
- t7001-mv.sh:modernizing test script using function
+ t7001-mv.sh: modernizing test script using functions
Test script to verify the presence/absence of files, paths, directories,
symlinks and other features in 'git mv' command are using the command
@@ Commit message
'test_path_is_*'
+ Changes since v1
+
+ Replacing idiomatic helper functions
+
+ '! test_path_is_*'
+
+ with
+
+ 'test_path_is_missing'
+
+ This uses values of 'test_path_bar' in place of '! test_path_foo' to
+ bring in the helpful factor of indicating the failure of tests after the
+ mv command has been used, that is, it echoes if the feature/test_path
+ exists.
+
Signed-off-by: Debra Obondo <debraobondo@gmail.com>
## t/t7001-mv.sh ##
@@ t/t7001-mv.sh: test_expect_success 'checking the commit' '
- test -f path0/COPYING &&
- test ! -f MOVED
+ test_path_is_file path0/COPYING &&
-+ ! test_path_is_file MOVED
++ test_path_is_missing MOVED
'
test_expect_success 'checking -k on non-existing file' '
@@ t/t7001-mv.sh: test_expect_success 'checking -k on non-existing file' '
- test -f untracked1 &&
- test ! -f path0/untracked1
+ test_path_is_file untracked1 &&
-+ ! test_path_is_file path0/untracked1
++ test_path_is_missing path0/untracked1
'
test_expect_success 'checking -k on multiple untracked files' '
@@ t/t7001-mv.sh: test_expect_success 'checking -k on non-existing file' '
- test ! -f path0/untracked2
+ test_path_is_file untracked1 &&
+ test_path_is_file untracked2 &&
-+ ! test_path_is_file path0/untracked1 &&
-+ ! test_path_is_file path0/untracked2
++ test_path_is_missing path0/untracked1 &&
++ test_path_is_missing path0/untracked2
'
test_expect_success 'checking -f on untracked file with existing target' '
@@ t/t7001-mv.sh: test_expect_success 'checking -k on non-existing file' '
- test ! -f .git/index.lock &&
- test -f untracked1 &&
- test -f path0/untracked1
-+ ! test_path_is_file .git/index.lock &&
++ test_path_is_missing .git/index.lock &&
+ test_path_is_file untracked1 &&
+ test_path_is_file path0/untracked1
'
@@ t/t7001-mv.sh: test_expect_success 'absolute pathname' '
git mv sub "$(pwd)/in" &&
- ! test -d sub &&
- test -d in &&
-+ ! test_path_is_dir sub &&
++ test_path_is_missing sub &&
+ test_path_is_dir in &&
git ls-files --error-unmatch in/file
)
@@ t/t7001-mv.sh: test_expect_success 'absolute pathname outside should fail' '
- test -d sub &&
- ! test -d ../in &&
+ test_path_is_dir sub &&
-+ ! test_path_is_dir ../in &&
++ test_path_is_missing ../in &&
git ls-files --error-unmatch sub/file
)
'
@@ t/t7001-mv.sh: test_expect_success 'git mv should overwrite symlink to a file' '
git mv -f moved symlink &&
- ! test -e moved &&
- test -f symlink &&
-+ ! test_path_exists moved &&
++ test_path_is_missing moved &&
+ test_path_is_file symlink &&
test "$(cat symlink)" = 1 &&
git update-index --refresh &&
@@ t/t7001-mv.sh: test_expect_success 'git mv should overwrite file with a symlink'
test_must_fail git mv symlink moved &&
git mv -f symlink moved &&
- ! test -e symlink &&
-+ ! test_path_exists symlink &&
++ test_path_is_missing symlink &&
git update-index --refresh &&
git diff-files --quiet
'
@@ t/t7001-mv.sh: test_expect_success 'git mv moves a submodule with a .git directo
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
-+ ! test_path_exists sub &&
++ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ t/t7001-mv.sh: test_expect_success 'git mv moves a submodule with a .git directo
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
-+ ! test_path_exists sub &&
++ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ t/t7001-mv.sh: test_expect_success 'git mv moves a submodule with gitfile' '
mkdir mod &&
git -C mod mv ../sub/ . &&
- ! test -e sub &&
-+ ! test_path_exists sub &&
++ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ t/t7001-mv.sh: test_expect_success 'mv does not complain when no .gitmodules fil
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
-+ ! test_path_exists sub &&
++ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ t/t7001-mv.sh: test_expect_success 'mv will error out on a modified .gitmodules
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
-+ ! test_path_exists sub &&
++ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ t/t7001-mv.sh: test_expect_success 'mv issues a warning when section is not foun
git mv sub mod/sub 2>actual.err &&
test_cmp expect.err actual.err &&
- ! test -e sub &&
-+ ! test_path_exists sub &&
++ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ t/t7001-mv.sh: test_expect_success 'checking out a commit before submodule moved
test_cmp expected actual &&
- ! test -f sub/.git &&
- test -f sub2/.git &&
-+ ! test_path_is_file sub/.git &&
++ test_path_is_missing sub/.git &&
+ test_path_is_file sub2/.git &&
git submodule update &&
- test -f sub/.git &&
t/t7001-mv.sh | 62 +++++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 8c37bceb336..d72cef88264 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -60,8 +60,8 @@ test_expect_success 'checking the commit' '
test_expect_success 'mv --dry-run does not move file' '
git mv -n path0/COPYING MOVED &&
- test -f path0/COPYING &&
- test ! -f MOVED
+ test_path_is_file path0/COPYING &&
+ test_path_is_missing MOVED
'
test_expect_success 'checking -k on non-existing file' '
@@ -71,25 +71,25 @@ test_expect_success 'checking -k on non-existing file' '
test_expect_success 'checking -k on untracked file' '
>untracked1 &&
git mv -k untracked1 path0 &&
- test -f untracked1 &&
- test ! -f path0/untracked1
+ test_path_is_file untracked1 &&
+ test_path_is_missing path0/untracked1
'
test_expect_success 'checking -k on multiple untracked files' '
>untracked2 &&
git mv -k untracked1 untracked2 path0 &&
- test -f untracked1 &&
- test -f untracked2 &&
- test ! -f path0/untracked1 &&
- test ! -f path0/untracked2
+ test_path_is_file untracked1 &&
+ test_path_is_file untracked2 &&
+ test_path_is_missing path0/untracked1 &&
+ test_path_is_missing path0/untracked2
'
test_expect_success 'checking -f on untracked file with existing target' '
>path0/untracked1 &&
test_must_fail git mv -f untracked1 path0 &&
- test ! -f .git/index.lock &&
- test -f untracked1 &&
- test -f path0/untracked1
+ test_path_is_missing .git/index.lock &&
+ test_path_is_file untracked1 &&
+ test_path_is_file path0/untracked1
'
# clean up the mess in case bad things happen
@@ -215,8 +215,8 @@ test_expect_success 'absolute pathname' '
git add sub/file &&
git mv sub "$(pwd)/in" &&
- ! test -d sub &&
- test -d in &&
+ test_path_is_missing sub &&
+ test_path_is_dir in &&
git ls-files --error-unmatch in/file
)
'
@@ -234,8 +234,8 @@ test_expect_success 'absolute pathname outside should fail' '
git add sub/file &&
test_must_fail git mv sub "$out/out" &&
- test -d sub &&
- ! test -d ../in &&
+ test_path_is_dir sub &&
+ test_path_is_missing ../in &&
git ls-files --error-unmatch sub/file
)
'
@@ -295,8 +295,8 @@ test_expect_success 'git mv should overwrite symlink to a file' '
git add moved &&
test_must_fail git mv moved symlink &&
git mv -f moved symlink &&
- ! test -e moved &&
- test -f symlink &&
+ test_path_is_missing moved &&
+ test_path_is_file symlink &&
test "$(cat symlink)" = 1 &&
git update-index --refresh &&
git diff-files --quiet
@@ -312,13 +312,13 @@ test_expect_success 'git mv should overwrite file with a symlink' '
git add moved &&
test_must_fail git mv symlink moved &&
git mv -f symlink moved &&
- ! test -e symlink &&
+ test_path_is_missing symlink &&
git update-index --refresh &&
git diff-files --quiet
'
test_expect_success SYMLINKS 'check moved symlink' '
- test -h moved
+ test_path_is_symlink moved
'
rm -f moved symlink
@@ -352,7 +352,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm
) &&
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -372,7 +372,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu
) &&
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ -389,7 +389,7 @@ test_expect_success 'git mv moves a submodule with gitfile' '
entry="$(git ls-files --stage sub | cut -f 1)" &&
mkdir mod &&
git -C mod mv ../sub/ . &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ -408,7 +408,7 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '
mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -423,13 +423,13 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta
entry="$(git ls-files --stage sub | cut -f 1)" &&
mkdir mod &&
test_must_fail git mv sub mod/sub 2>actual.err &&
- test -s actual.err &&
- test -e sub &&
+ test_file_not_empty actual.err &&
+ test_path_exists sub &&
git diff-files --quiet -- sub &&
git add .gitmodules &&
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -447,7 +447,7 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule
mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_cmp expect.err actual.err &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -460,7 +460,7 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' '
git submodule update &&
mkdir mod &&
git mv -n sub mod/sub 2>actual.err &&
- test -f sub/.git &&
+ test_path_is_file sub/.git &&
git diff-index --exit-code HEAD &&
git update-index --refresh &&
git diff-files --quiet -- sub .gitmodules
@@ -474,10 +474,10 @@ test_expect_success 'checking out a commit before submodule moved needs manual u
git status -s sub2 >actual &&
echo "?? sub2/" >expected &&
test_cmp expected actual &&
- ! test -f sub/.git &&
- test -f sub2/.git &&
+ test_path_is_missing sub/.git &&
+ test_path_is_file sub2/.git &&
git submodule update &&
- test -f sub/.git &&
+ test_path_is_file sub/.git &&
rm -rf sub2 &&
git diff-index --exit-code HEAD &&
git update-index --refresh &&
base-commit: 1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886
--
gitgitgadget
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] t7001-mv.sh: modernizing test script using functions
2022-11-03 18:39 ` [PATCH v2] t7001-mv.sh: modernizing test script using functions Debra Obondo via GitGitGadget
@ 2022-11-03 19:37 ` Martin Ågren
2022-11-04 15:05 ` [PATCH v3] " Debra Obondo via GitGitGadget
1 sibling, 0 replies; 8+ messages in thread
From: Martin Ågren @ 2022-11-03 19:37 UTC (permalink / raw)
To: Debra Obondo via GitGitGadget; +Cc: git, Taylor Blau, Debra Obondo
On Thu, 3 Nov 2022 at 19:39, Debra Obondo via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Debra Obondo <debraobondo@gmail.com>
>
> Test script to verify the presence/absence of files, paths, directories,
> symlinks and other features in 'git mv' command are using the command
> format:
>
> 'test (-e|f|d|h|...)'
>
> Replace them with helper functions of format:
>
> 'test_path_is_*'
Ok.
> Changes since v1
>
> Replacing idiomatic helper functions
>
> '! test_path_is_*'
>
> with
>
> 'test_path_is_missing'
>
The above "Changes since v1" and what I've quoted here should probably
be dropped. We prefer our patches to look as if they've appeared out of
the blue in perfect shape. :-)
> This uses values of 'test_path_bar' in place of '! test_path_foo' to
> bring in the helpful factor of indicating the failure of tests after the
> mv command has been used, that is, it echoes if the feature/test_path
> exists.
This paragraph is excellent. It describes why you've done the patch this
way. This looks much better than version 1 of the patch!
> Signed-off-by: Debra Obondo <debraobondo@gmail.com>
After removing the lines about changes since v1, this patch is
Reviewed-by: Martin Ågren <martin.agren@gmail.com>
> ---
> [PATCH v2] [OUTREACHY] t7001-mv.sh : Use test_path_is_* functions in
> test script
>
> Changes since v1:
>
> Replacing idiomatic helper functions
>
> '! test_path_is_*'
>
> with
>
> 'test_path_is_missing'
This place after the "---" line is an excellent place for including such
"here's what has changed since v1" comments. Good. They will not appear
in the final commit message.
Thanks for contributing!
Martin
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] t7001-mv.sh: modernizing test script using functions
2022-11-03 18:39 ` [PATCH v2] t7001-mv.sh: modernizing test script using functions Debra Obondo via GitGitGadget
2022-11-03 19:37 ` Martin Ågren
@ 2022-11-04 15:05 ` Debra Obondo via GitGitGadget
2022-11-04 22:00 ` Taylor Blau
1 sibling, 1 reply; 8+ messages in thread
From: Debra Obondo via GitGitGadget @ 2022-11-04 15:05 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Martin Ågren, Debra Obondo, Debra Obondo
From: Debra Obondo <debraobondo@gmail.com>
Test script to verify the presence/absence of files, paths, directories,
symlinks and other features in 'git mv' command are using the command
format:
'test (-e|f|d|h|...)'
Replace them with helper functions of format:
'test_path_is_*'
Replacing idiomatic helper functions:
'! test_path_is_*'
with
'test_path_is_missing'
This uses values of 'test_path_bar' in place of '! test_path_foo' to
bring in the helpful factor of indicating the failure of tests after the
mv command has been used, that is, it echoes if the feature/test_path
exists.
Signed-off-by: Debra Obondo <debraobondo@gmail.com>
---
[PATCH v3] [OUTREACHY] t7001-mv.sh : Use test_path_is_* functions in
test script
Changes since prevous 2 versions:
Replacing idiomatic helper functions
'! test_path_is_*'
with
'test_path_is_missing'
This uses values of 'test_path_bar' in place of '! test_path_foo' to
bring in the helpful factor of indicating the failure of tests after the
mv command has been used, that is, it echoes if the feature/test_path
exists .
Older test scripts use the command 'test -' to verify the presence or
absence of features such as files, directories and symbolic links. This
however requires slightly complicated uses, such as 'test ! -f '. The
helper functions 'test_path_is_' located in t/test-lib-functions.sh have
taken into account all scenarios of the 'test -' to reduce errors. This
was a microproject to replace them with the helper functions.
Test script to verify the presence/absence of files, paths,
directories,symboliclinks and many other features in mv command are
using the command format
'test -(e|f|s|h|...).
Replace them with helper functions of format
'test_path_is_*'
Signed-off-by: Debra Obondo debraobondo@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1372%2Ffobiasic07%2Ft7001-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1372/fobiasic07/t7001-v3
Pull-Request: https://github.com/git/git/pull/1372
Range-diff vs v2:
1: c6640ebff66 ! 1: 04176190ffd t7001-mv.sh: modernizing test script using functions
@@ Commit message
'test_path_is_*'
- Changes since v1
-
- Replacing idiomatic helper functions
+ Replacing idiomatic helper functions:
'! test_path_is_*'
t/t7001-mv.sh | 62 +++++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 8c37bceb336..d72cef88264 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -60,8 +60,8 @@ test_expect_success 'checking the commit' '
test_expect_success 'mv --dry-run does not move file' '
git mv -n path0/COPYING MOVED &&
- test -f path0/COPYING &&
- test ! -f MOVED
+ test_path_is_file path0/COPYING &&
+ test_path_is_missing MOVED
'
test_expect_success 'checking -k on non-existing file' '
@@ -71,25 +71,25 @@ test_expect_success 'checking -k on non-existing file' '
test_expect_success 'checking -k on untracked file' '
>untracked1 &&
git mv -k untracked1 path0 &&
- test -f untracked1 &&
- test ! -f path0/untracked1
+ test_path_is_file untracked1 &&
+ test_path_is_missing path0/untracked1
'
test_expect_success 'checking -k on multiple untracked files' '
>untracked2 &&
git mv -k untracked1 untracked2 path0 &&
- test -f untracked1 &&
- test -f untracked2 &&
- test ! -f path0/untracked1 &&
- test ! -f path0/untracked2
+ test_path_is_file untracked1 &&
+ test_path_is_file untracked2 &&
+ test_path_is_missing path0/untracked1 &&
+ test_path_is_missing path0/untracked2
'
test_expect_success 'checking -f on untracked file with existing target' '
>path0/untracked1 &&
test_must_fail git mv -f untracked1 path0 &&
- test ! -f .git/index.lock &&
- test -f untracked1 &&
- test -f path0/untracked1
+ test_path_is_missing .git/index.lock &&
+ test_path_is_file untracked1 &&
+ test_path_is_file path0/untracked1
'
# clean up the mess in case bad things happen
@@ -215,8 +215,8 @@ test_expect_success 'absolute pathname' '
git add sub/file &&
git mv sub "$(pwd)/in" &&
- ! test -d sub &&
- test -d in &&
+ test_path_is_missing sub &&
+ test_path_is_dir in &&
git ls-files --error-unmatch in/file
)
'
@@ -234,8 +234,8 @@ test_expect_success 'absolute pathname outside should fail' '
git add sub/file &&
test_must_fail git mv sub "$out/out" &&
- test -d sub &&
- ! test -d ../in &&
+ test_path_is_dir sub &&
+ test_path_is_missing ../in &&
git ls-files --error-unmatch sub/file
)
'
@@ -295,8 +295,8 @@ test_expect_success 'git mv should overwrite symlink to a file' '
git add moved &&
test_must_fail git mv moved symlink &&
git mv -f moved symlink &&
- ! test -e moved &&
- test -f symlink &&
+ test_path_is_missing moved &&
+ test_path_is_file symlink &&
test "$(cat symlink)" = 1 &&
git update-index --refresh &&
git diff-files --quiet
@@ -312,13 +312,13 @@ test_expect_success 'git mv should overwrite file with a symlink' '
git add moved &&
test_must_fail git mv symlink moved &&
git mv -f symlink moved &&
- ! test -e symlink &&
+ test_path_is_missing symlink &&
git update-index --refresh &&
git diff-files --quiet
'
test_expect_success SYMLINKS 'check moved symlink' '
- test -h moved
+ test_path_is_symlink moved
'
rm -f moved symlink
@@ -352,7 +352,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm
) &&
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -372,7 +372,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu
) &&
mkdir mod &&
git mv sub mod/sub &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ -389,7 +389,7 @@ test_expect_success 'git mv moves a submodule with gitfile' '
entry="$(git ls-files --stage sub | cut -f 1)" &&
mkdir mod &&
git -C mod mv ../sub/ . &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
echo mod/sub >expected &&
@@ -408,7 +408,7 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '
mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -423,13 +423,13 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta
entry="$(git ls-files --stage sub | cut -f 1)" &&
mkdir mod &&
test_must_fail git mv sub mod/sub 2>actual.err &&
- test -s actual.err &&
- test -e sub &&
+ test_file_not_empty actual.err &&
+ test_path_exists sub &&
git diff-files --quiet -- sub &&
git add .gitmodules &&
git mv sub mod/sub 2>actual.err &&
test_must_be_empty actual.err &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -447,7 +447,7 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule
mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_cmp expect.err actual.err &&
- ! test -e sub &&
+ test_path_is_missing sub &&
test "$entry" = "$(git ls-files --stage mod/sub | cut -f 1)" &&
git -C mod/sub status &&
git update-index --refresh &&
@@ -460,7 +460,7 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' '
git submodule update &&
mkdir mod &&
git mv -n sub mod/sub 2>actual.err &&
- test -f sub/.git &&
+ test_path_is_file sub/.git &&
git diff-index --exit-code HEAD &&
git update-index --refresh &&
git diff-files --quiet -- sub .gitmodules
@@ -474,10 +474,10 @@ test_expect_success 'checking out a commit before submodule moved needs manual u
git status -s sub2 >actual &&
echo "?? sub2/" >expected &&
test_cmp expected actual &&
- ! test -f sub/.git &&
- test -f sub2/.git &&
+ test_path_is_missing sub/.git &&
+ test_path_is_file sub2/.git &&
git submodule update &&
- test -f sub/.git &&
+ test_path_is_file sub/.git &&
rm -rf sub2 &&
git diff-index --exit-code HEAD &&
git update-index --refresh &&
base-commit: 1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886
--
gitgitgadget
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] t7001-mv.sh: modernizing test script using functions
2022-11-04 15:05 ` [PATCH v3] " Debra Obondo via GitGitGadget
@ 2022-11-04 22:00 ` Taylor Blau
0 siblings, 0 replies; 8+ messages in thread
From: Taylor Blau @ 2022-11-04 22:00 UTC (permalink / raw)
To: Debra Obondo via GitGitGadget
Cc: git, Taylor Blau, Martin Ågren, Debra Obondo
Hi Debra,
On Fri, Nov 04, 2022 at 03:05:52PM +0000, Debra Obondo via GitGitGadget wrote:
> t/t7001-mv.sh | 62 +++++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 31 deletions(-)
This round looks great. Will queue, thanks.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-04 22:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-30 17:20 [PATCH] t7001-mv.sh:modernizing test script using function Debra Obondo via GitGitGadget
2022-10-30 18:00 ` Taylor Blau
2022-10-31 18:04 ` Martin Ågren
2022-11-01 1:14 ` Taylor Blau
2022-11-03 18:39 ` [PATCH v2] t7001-mv.sh: modernizing test script using functions Debra Obondo via GitGitGadget
2022-11-03 19:37 ` Martin Ågren
2022-11-04 15:05 ` [PATCH v3] " Debra Obondo via GitGitGadget
2022-11-04 22:00 ` Taylor Blau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).