From: Duy Nguyen <pclouds@gmail.com>
To: Rohit Ashiwal via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>,
Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Subject: Re: [PATCH 1/1] tests: replace `test -(d|f)` with test_path_is_(dir|file)
Date: Tue, 26 Feb 2019 21:04:46 +0700 [thread overview]
Message-ID: <CACsJy8DG6+mmA5NT67V46=n1-5H_eh3779eE28YN4kcjb0Cq0A@mail.gmail.com> (raw)
In-Reply-To: <bf5eb045795579dd5d996e787e246996688cf4bf.1551188524.git.gitgitgadget@gmail.com>
On Tue, Feb 26, 2019 at 8:42 PM Rohit Ashiwal via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
>
> t3600-rm.sh: Previously we were using `test -(d|f)`
> to verify the presencee of a directory/file, but we
> already have helper functions, viz, test_path_is_dir
> and test_path_is_file with same functionality. This
It's not just the same (no point replacing then). It's better. When
test_path_is_xxx fails, you get an error message. If "test -xxx"
fails, you get a failed test with no clue what caused it.
> patch will replace `test -(d|f)` calls in t3600-rm.sh.
>
> Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
> ---
> t/t3600-rm.sh | 96 +++++++++++++++++++++++++--------------------------
> 1 file changed, 48 insertions(+), 48 deletions(-)
>
> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
> index 04e5d42bd3..dcaa2ab4d6 100755
> --- a/t/t3600-rm.sh
> +++ b/t/t3600-rm.sh
> @@ -137,8 +137,8 @@ test_expect_success 'Re-add foo and baz' '
> test_expect_success 'Modify foo -- rm should refuse' '
> echo >>foo &&
> test_must_fail git rm foo baz &&
> - test -f foo &&
> - test -f baz &&
> + test_path_is_file foo &&
> + test_path_is_file baz &&
> git ls-files --error-unmatch foo baz
> '
>
> @@ -159,8 +159,8 @@ test_expect_success 'Re-add foo and baz for HEAD tests' '
>
> test_expect_success 'foo is different in index from HEAD -- rm should refuse' '
> test_must_fail git rm foo baz &&
> - test -f foo &&
> - test -f baz &&
> + test_path_is_file foo &&
> + test_path_is_file baz &&
> git ls-files --error-unmatch foo baz
> '
>
> @@ -194,21 +194,21 @@ test_expect_success 'Recursive test setup' '
>
> test_expect_success 'Recursive without -r fails' '
> test_must_fail git rm frotz &&
> - test -d frotz &&
> - test -f frotz/nitfol
> + test_path_is_dir frotz &&
> + test_path_is_file frotz/nitfol
> '
>
> test_expect_success 'Recursive with -r but dirty' '
> echo qfwfq >>frotz/nitfol &&
> test_must_fail git rm -r frotz &&
> - test -d frotz &&
> - test -f frotz/nitfol
> + test_path_is_dir frotz &&
> + test_path_is_file frotz/nitfol
> '
>
> test_expect_success 'Recursive with -r -f' '
> git rm -f -r frotz &&
> - ! test -f frotz/nitfol &&
> - ! test -d frotz
> + ! test_path_is_file frotz/nitfol &&
> + ! test_path_is_dir frotz
> '
>
> test_expect_success 'Remove nonexistent file returns nonzero exit status' '
> @@ -254,7 +254,7 @@ test_expect_success 'rm removes subdirectories recursively' '
> echo content >dir/subdir/subsubdir/file &&
> git add dir/subdir/subsubdir/file &&
> git rm -f dir/subdir/subsubdir/file &&
> - ! test -d dir
> + ! test_path_is_dir dir
> '
>
> cat >expect <<EOF
> @@ -343,8 +343,8 @@ test_expect_success 'rm of a populated submodule with different HEAD fails unles
> git submodule update &&
> git -C submod checkout HEAD^ &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.modified actual &&
> git rm -f submod &&
> @@ -359,8 +359,8 @@ test_expect_success 'rm --cached leaves work tree of populated submodules and .g
> git reset --hard &&
> git submodule update &&
> git rm --cached submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno >actual &&
> test_cmp expect.cached actual &&
> git config -f .gitmodules submodule.sub.url &&
> @@ -371,7 +371,7 @@ test_expect_success 'rm --dry-run does not touch the submodule or .gitmodules' '
> git reset --hard &&
> git submodule update &&
> git rm -n submod &&
> - test -f submod/.git &&
> + test_path_is_file submod/.git &&
> git diff-index --exit-code HEAD
> '
>
> @@ -381,8 +381,8 @@ test_expect_success 'rm does not complain when no .gitmodules file is found' '
> git rm .gitmodules &&
> git rm submod >actual 2>actual.err &&
> test_must_be_empty actual.err &&
> - ! test -d submod &&
> - ! test -f submod/.git &&
> + ! test_path_is_dir submod &&
> + ! test_path_is_file submod/.git &&
> git status -s -uno >actual &&
> test_cmp expect.both_deleted actual
> '
> @@ -393,14 +393,14 @@ test_expect_success 'rm will error out on a modified .gitmodules file unless sta
> git config -f .gitmodules foo.bar true &&
> test_must_fail git rm submod >actual 2>actual.err &&
> test -s actual.err &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git diff-files --quiet -- submod &&
> git add .gitmodules &&
> git rm submod >actual 2>actual.err &&
> test_must_be_empty actual.err &&
> - ! test -d submod &&
> - ! test -f submod/.git &&
> + ! test_path_is_dir submod &&
> + ! test_path_is_file submod/.git &&
> git status -s -uno >actual &&
> test_cmp expect actual
> '
> @@ -413,8 +413,8 @@ test_expect_success 'rm issues a warning when section is not found in .gitmodule
> echo "warning: Could not find section in .gitmodules where path=submod" >expect.err &&
> git rm submod >actual 2>actual.err &&
> test_i18ncmp expect.err actual.err &&
> - ! test -d submod &&
> - ! test -f submod/.git &&
> + ! test_path_is_dir submod &&
> + ! test_path_is_file submod/.git &&
> git status -s -uno >actual &&
> test_cmp expect actual
> '
> @@ -424,8 +424,8 @@ test_expect_success 'rm of a populated submodule with modifications fails unless
> git submodule update &&
> echo X >submod/empty &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.modified_inside actual &&
> git rm -f submod &&
> @@ -439,8 +439,8 @@ test_expect_success 'rm of a populated submodule with untracked files fails unle
> git submodule update &&
> echo X >submod/untracked &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.modified_untracked actual &&
> git rm -f submod &&
> @@ -493,8 +493,8 @@ test_expect_success 'rm of a conflicted populated submodule with different HEAD
> git -C submod checkout HEAD^ &&
> test_must_fail git merge conflict2 &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.conflict actual &&
> git rm -f submod &&
> @@ -512,8 +512,8 @@ test_expect_success 'rm of a conflicted populated submodule with modifications f
> echo X >submod/empty &&
> test_must_fail git merge conflict2 &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.conflict actual &&
> git rm -f submod &&
> @@ -531,8 +531,8 @@ test_expect_success 'rm of a conflicted populated submodule with untracked files
> echo X >submod/untracked &&
> test_must_fail git merge conflict2 &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.conflict actual &&
> git rm -f submod &&
> @@ -552,13 +552,13 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director
> ) &&
> test_must_fail git merge conflict2 &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -d submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_dir submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.conflict actual &&
> test_must_fail git rm -f submod &&
> - test -d submod &&
> - test -d submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_dir submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.conflict actual &&
> git merge --abort &&
> @@ -586,8 +586,8 @@ test_expect_success 'rm of a populated submodule with a .git directory migrates
> rm -r ../.git/modules/sub
> ) &&
> git rm submod 2>output.err &&
> - ! test -d submod &&
> - ! test -d submod/.git &&
> + ! test_path_is_dir submod &&
> + ! test_path_is_dir submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test -s actual &&
> test_i18ngrep Migrating output.err
> @@ -624,8 +624,8 @@ test_expect_success 'rm of a populated nested submodule with different nested HE
> git submodule update --recursive &&
> git -C submod/subsubmod checkout HEAD^ &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.modified_inside actual &&
> git rm -f submod &&
> @@ -639,8 +639,8 @@ test_expect_success 'rm of a populated nested submodule with nested modification
> git submodule update --recursive &&
> echo X >submod/subsubmod/empty &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.modified_inside actual &&
> git rm -f submod &&
> @@ -654,8 +654,8 @@ test_expect_success 'rm of a populated nested submodule with nested untracked fi
> git submodule update --recursive &&
> echo X >submod/subsubmod/untracked &&
> test_must_fail git rm submod &&
> - test -d submod &&
> - test -f submod/.git &&
> + test_path_is_dir submod &&
> + test_path_is_file submod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test_cmp expect.modified_untracked actual &&
> git rm -f submod &&
> @@ -673,8 +673,8 @@ test_expect_success "rm absorbs submodule's nested .git directory" '
> GIT_WORK_TREE=. git config --unset core.worktree
> ) &&
> git rm submod 2>output.err &&
> - ! test -d submod &&
> - ! test -d submod/subsubmod/.git &&
> + ! test_path_is_dir submod &&
> + ! test_path_is_dir submod/subsubmod/.git &&
> git status -s -uno --ignore-submodules=none >actual &&
> test -s actual &&
> test_i18ngrep Migrating output.err
> --
> gitgitgadget
--
Duy
next prev parent reply other threads:[~2019-02-26 14:05 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-26 13:42 [PATCH 0/1] [GSoC][PATCH] tests: replace test -(d|f) with test_path_is_(dir|file) Rohit Ashiwal via GitGitGadget
2019-02-26 13:42 ` [PATCH 1/1] tests: replace `test -(d|f)` " Rohit Ashiwal via GitGitGadget
2019-02-26 14:04 ` Duy Nguyen [this message]
2019-02-26 16:10 ` Do test-path_is_{file,dir,exists} make sense anymore with -x? Ævar Arnfjörð Bjarmason
2019-02-26 17:04 ` SZEDER Gábor
2019-02-26 17:43 ` Jeff King
2019-02-26 19:39 ` SZEDER Gábor
2019-02-26 21:01 ` Jeff King
2019-03-03 16:04 ` SZEDER Gábor
2019-03-05 4:55 ` Jeff King
2019-03-04 14:36 ` SZEDER Gábor
2019-03-05 4:58 ` Jeff King
2019-02-26 17:48 ` Matthieu Moy
2019-02-26 18:24 ` Jeff King
2019-02-26 17:35 ` Jeff King
2019-02-26 19:58 ` Johannes Schindelin
2019-02-26 21:02 ` Jeff King
2019-02-27 10:01 ` Duy Nguyen
2019-03-01 2:52 ` Junio C Hamano
2019-02-26 16:01 ` [PATCH 1/1] tests: replace `test -(d|f)` with test_path_is_(dir|file) Johannes Schindelin
2019-02-26 16:30 ` Martin Ågren
2019-02-26 18:29 ` Rohit Ashiwal
2019-02-26 19:52 ` Johannes Schindelin
2019-02-26 20:01 ` Rohit Ashiwal
2019-02-27 5:49 ` Martin Ågren
2019-02-26 14:26 ` [PATCH v2 0/1] [GSoC][PATCH] t3600: use test_path_is_dir and test_path_is_file Rohit Ashiwal via GitGitGadget
2019-02-26 14:26 ` [PATCH v2 1/1] " Rohit Ashiwal via GitGitGadget
2019-02-26 16:37 ` SZEDER Gábor
2019-02-26 18:40 ` Rohit Ashiwal
2019-02-26 20:02 ` Johannes Schindelin
2019-02-26 20:05 ` Rohit Ashiwal
2019-02-26 22:48 ` [PATCH v3 0/1] [GSoC][PATCH] t3600: use test_path_is_* helper functions Rohit Ashiwal via GitGitGadget
2019-02-26 22:48 ` [PATCH v3 1/1] t3600: use test_path_is_* functions Rohit Ashiwal via GitGitGadget
2019-02-27 10:12 ` Duy Nguyen
2019-02-28 10:26 ` [PATCH v4 0/1] [GSoC][PATCH] t3600: use test_path_is_* helper functions Rohit Ashiwal via GitGitGadget
2019-02-28 10:26 ` [PATCH v4 1/1] t3600: use test_path_is_* functions Rohit Ashiwal via GitGitGadget
2019-02-28 19:02 ` [GSoC] acknowledging mistakes Rohit Ashiwal
2019-03-01 2:51 ` Junio C Hamano
2019-03-01 13:13 ` Feeling confused a little bit Rohit Ashiwal
2019-03-02 4:24 ` Rafael Ascensão
2019-03-02 14:46 ` Thomas Gummerer
2019-03-02 16:21 ` [GSoC] Thanking Rohit Ashiwal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACsJy8DG6+mmA5NT67V46=n1-5H_eh3779eE28YN4kcjb0Cq0A@mail.gmail.com' \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=rohit.ashiwal265@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.