* [GSoC] [PATCH] test: avoid pipes in git related commands for test suite
@ 2018-03-13 20:19 Pratik Karki
2018-03-14 7:30 ` Eric Sunshine
0 siblings, 1 reply; 16+ messages in thread
From: Pratik Karki @ 2018-03-13 20:19 UTC (permalink / raw)
To: git; +Cc: Pratik Karki
This patch removes the necessity of pipes in git related commands for test suite.
Exit code of the upstream in a pipe is ignored so, it's use should be avoided. The fix for this is to write the output of the git command to a file and test the exit codes of both the commands being linked by pipe.
Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
t/t7001-mv.sh | 24 ++++++++++++------------
t/t9104-git-svn-follow-parent.sh | 4 ++--
t/t9110-git-svn-use-svm-props.sh | 36 ++++++++++++++++++------------------
t/t9111-git-svn-use-svnsync-props.sh | 36 ++++++++++++++++++------------------
t/t9114-git-svn-dcommit-merge.sh | 8 ++++----
t/t9130-git-svn-authors-file.sh | 16 ++++++++--------
t/t9138-git-svn-authors-prog.sh | 28 ++++++++++++++--------------
t/t9153-git-svn-rewrite-uuid.sh | 8 ++++----
8 files changed, 80 insertions(+), 80 deletions(-)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 6e5031f56..0dcf1fa3e 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -21,8 +21,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path1/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path1/COPYING" actual'
test_expect_success \
'moving the file back into subdirectory' \
@@ -35,8 +35,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path1/COPYING..*path0/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path1/COPYING..*path0/COPYING" actual'
test_expect_success \
'checking -k on non-existing file' \
@@ -116,10 +116,10 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/README..*path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
+ git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/README..*path2/README" actual'
test_expect_success \
'succeed when source is a prefix of destination' \
@@ -135,10 +135,10 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/COPYING..*path1/path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/README..*path1/path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path2/COPYING..*path1/path2/COPYING" actual &&
+ git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path2/README..*path1/path2/README" actual'
test_expect_success \
'do not move directory over existing directory' \
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index cd480edf1..284d1224e 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -204,8 +204,8 @@ test_expect_success "follow-parent is atomic" '
test_expect_success "track multi-parent paths" '
svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
git svn multi-fetch &&
- test $(git cat-file commit refs/remotes/glob | \
- grep "^parent " | wc -l) -eq 2
+ test $(git cat-file commit refs/remotes/glob >actual &&
+ grep "^parent " actual | wc -l) -eq 2
'
test_expect_success "multi-fetch continues to work" "
diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
index dde0a3c22..a1a00c298 100755
--- a/t/t9110-git-svn-use-svm-props.sh
+++ b/t/t9110-git-svn-use-svm-props.sh
@@ -21,32 +21,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_expect_success 'find commit based on SVN revision number' "
diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
index 22b6e5ee7..5306a87f3 100755
--- a/t/t9111-git-svn-use-svnsync-props.sh
+++ b/t/t9111-git-svn-use-svnsync-props.sh
@@ -20,32 +20,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_done
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 50bca62de..c945c3758 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -68,7 +68,7 @@ test_debug 'gitk --all & sleep 1'
test_expect_success 'verify pre-merge ancestry' "
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual && grep '^friend$' actual
"
test_expect_success 'git svn dcommit merges' "
@@ -82,12 +82,12 @@ test_expect_success 'verify post-merge ancestry' "
x\$(git rev-parse --verify refs/remotes/origin/trunk) &&
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual && grep '^friend$' actual
"
test_expect_success 'verify merge commit message' "
- git rev-list --pretty=raw -1 refs/heads/svn | \
- grep \" Merge branch 'merge' into svn\"
+ git rev-list --pretty=raw -1 refs/heads/svn >actual &&
+ grep \" Merge branch 'merge' into svn\" actual
"
test_done
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index 41264818c..e12f8cf3b 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -27,10 +27,10 @@ test_expect_success 'imported 2 revisions successfully' '
(
cd x
test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 2 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author BBBBBBB BBBBBBB <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author AAAAAAA AAAAAAA <aa@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author BBBBBBB BBBBBBB <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author AAAAAAA AAAAAAA <aa@example\.com> " actual
)
'
@@ -44,10 +44,10 @@ test_expect_success 'continues to import once authors have been added' '
cd x
git svn fetch --authors-file=../svn-authors &&
test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 4 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author DDDDDDD DDDDDDD <dd@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author CCCCCCC CCCCCCC <cc@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author DDDDDDD DDDDDDD <dd@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author CCCCCCC CCCCCCC <cc@example\.com> " actual
)
'
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 7d7e9d46b..5b04c2b40 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -44,24 +44,24 @@ test_expect_success 'imported 6 revisions successfully' '
test_expect_success 'authors-prog ran correctly' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author ee-foo <ee-foo@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~2 | \
- grep "^author dd <dd@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~3 | \
- grep "^author cc <cc@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~4 | \
- grep "^author bb <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~5 | \
- grep "^author aa <aa@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author ee-foo <ee-foo@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~2 >actual &&
+ grep "^author dd <dd@sub\.example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~3 >actual &&
+ grep "^author cc <cc@sub\.example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~4 >actual &&
+ grep "^author bb <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~5 >actual &&
+ grep "^author aa <aa@example\.com> " actual
)
'
test_expect_success 'authors-file overrode authors-prog' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> " actual
)
'
@@ -73,8 +73,8 @@ test_expect_success 'authors-prog handled special characters in username' '
(
cd x &&
git svn --authors-prog=../svn-authors-prog fetch &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn |
- grep "^author xyz; touch evil <xyz; touch evil@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author xyz; touch evil <xyz; touch evil@example\.com> " actual &&
! test -f evil
)
'
diff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh
index 372ef1568..3b00a9135 100755
--- a/t/t9153-git-svn-rewrite-uuid.sh
+++ b/t/t9153-git-svn-rewrite-uuid.sh
@@ -16,10 +16,10 @@ test_expect_success 'load svn repo' "
"
test_expect_success 'verify uuid' "
- git cat-file commit refs/remotes/git-svn~0 | \
- grep '^git-svn-id: .*@2 $uuid$' &&
- git cat-file commit refs/remotes/git-svn~1 | \
- grep '^git-svn-id: .*@1 $uuid$'
+ git cat-file commit refs/remotes/git-svn~0 >actual &&
+ grep '^git-svn-id: .*@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/git-svn~1 >actual &&
+ grep '^git-svn-id: .*@1 $uuid$' actual
"
test_done
--
2.16.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [GSoC] [PATCH] test: avoid pipes in git related commands for test suite
2018-03-13 20:19 [GSoC] [PATCH] test: avoid pipes in git related commands for test suite Pratik Karki
@ 2018-03-14 7:30 ` Eric Sunshine
2018-03-14 9:57 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2018-03-14 7:30 UTC (permalink / raw)
To: Pratik Karki; +Cc: Git List
Thanks for the patch. See comments below...
On Tue, Mar 13, 2018 at 4:19 PM, Pratik Karki <predatoramigo@gmail.com> wrote:
> This patch removes the necessity of pipes in git related commands for test suite.
>
> Exit code of the upstream in a pipe is ignored so, it's use should be avoided. The fix for this is to write the output of the git command to a file and test the exit codes of both the commands being linked by pipe.
Please wrap commit messages to fit in about 72 columns; this one is
far too wide.
On the Git project, commit messages are written in imperative mood, as
if telling the codebase to "do something". So, instead of writing
"This patch removes...", you could word it "Remove..." or "Avoid...".
It's misleading to say that the patch "removes the _necessity_ of
pipes" since pipes were not used out of necessity; they were probably
just a convenience and seemed reasonable at the time, but later
experience has shown that they can be problematic for the reason you
give in the second paragraph.
Taking these observations into consideration, perhaps you could
rewrite the commit message something like this:
Avoid using pipes downstream of Git commands since the exit codes
of commands upstream of pipes get swallowed, thus potentially
hiding failure of those commands. Instead, capture Git command
output to a file apply the downstream command(s) to that file.
More comments below...
> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
> ---
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> @@ -116,10 +116,10 @@ test_expect_success \
> test_expect_success \
> 'checking the commit' \
> - 'git diff-tree -r -M --name-status HEAD^ HEAD | \
> - grep "^R100..*path0/COPYING..*path2/COPYING" &&
> - git diff-tree -r -M --name-status HEAD^ HEAD | \
> - grep "^R100..*path0/README..*path2/README"'
> + 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
> + grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
> + git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
> + grep "^R100..*path0/README..*path2/README" actual'
Although this "mechanical" transformation is technically correct, it
is nevertheless wasteful. The exact same "git diff-tree ..." command
is run twice, and both times output is captured to file 'actual',
which makes the second invocation superfluous. Instead, a better
transformation would be:
git diff-tree ... >actual &&
grep ... actual &&
grep ... actual
The same observation applies to other transformations in this patch.
> diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
> @@ -204,8 +204,8 @@ test_expect_success "follow-parent is atomic" '
> test_expect_success "track multi-parent paths" '
> svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
> git svn multi-fetch &&
> - test $(git cat-file commit refs/remotes/glob | \
> - grep "^parent " | wc -l) -eq 2
> + test $(git cat-file commit refs/remotes/glob >actual &&
> + grep "^parent " actual | wc -l) -eq 2
> '
This is not a great transformation. If "git cat-file" fails, then
neither 'grep' nor 'wc' will run, and the result will be as if 'test'
was called without an argument before "-eq". For example:
% test $(false >actual && grep "^parent " actual | wc -l) -eq 2
test: -eq: unary operator expected
It would be better to run "git cat-file" outside of "test $(...)". For instance:
git cat-file ... >actual &&
test $(grep ... actual | wc -l) -eq 2
Alternately, you could take advantage of the test_line_count() helper function:
git cat-file ... >actual &&
grep ... actual >actual2 &&
test_line_count = 2 actual2
> diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
> @@ -21,32 +21,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
> test_expect_success 'verify metadata for /bar' "
> - git cat-file commit refs/remotes/bar | \
> - grep '^git-svn-id: $bar_url@12 $uuid$' &&
> - git cat-file commit refs/remotes/bar~1 | \
> - grep '^git-svn-id: $bar_url@11 $uuid$' &&
> - git cat-file commit refs/remotes/bar~2 | \
> - grep '^git-svn-id: $bar_url@10 $uuid$' &&
> - git cat-file commit refs/remotes/bar~3 | \
> - grep '^git-svn-id: $bar_url@9 $uuid$' &&
> - git cat-file commit refs/remotes/bar~4 | \
> - grep '^git-svn-id: $bar_url@6 $uuid$' &&
> - git cat-file commit refs/remotes/bar~5 | \
> - grep '^git-svn-id: $bar_url@1 $uuid$'
> + git cat-file commit refs/remotes/bar >actual &&
> + grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~1 >actual &&
> + grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~2 >actual &&
> + grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~3 >actual &&
> + grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~4 >actual &&
> + grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~5 >actual &&
> + grep '^git-svn-id: $bar_url@1 $uuid$' actual
> "
An indented line in the original shows that it is a continuation of
the preceding line. However, in the revised code, that is not so, thus
it probably makes sense to drop the indentation.
The same comment applies to several additional cases snipped from this reply.
> diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
> index 50bca62de..c945c3758 100755
> --- a/t/t9114-git-svn-dcommit-merge.sh
> +++ b/t/t9114-git-svn-dcommit-merge.sh
> @@ -68,7 +68,7 @@ test_debug 'gitk --all & sleep 1'
> test_expect_success 'verify pre-merge ancestry' "
> test x\$(git rev-parse --verify refs/heads/svn^2) = \
> x\$(git rev-parse --verify refs/heads/merge) &&
> - git cat-file commit refs/heads/svn^ | grep '^friend$'
> + git cat-file commit refs/heads/svn^ >actual && grep '^friend$' actual
> "
Style: split the line at the '&&'...
git cat-file ... >actual &&
grep ... actual
The same comment applies to another test snipped from this reply.
Aside: The current patch wants to solve the problem of exit code being
swallowed down pipes, however, this and other tests are afflicted by a
similar problem with $(...) also swallowing the exit code. A failure
of "git rev-parse" could potentially go unnoticed inside "test
x$(...)". Fixing that is outside the scope of the current patch,
however, a follow-on patch to fix that problem (if you feel so
inclined) might transform it something like this:
git rev-parse ... >rev1 &&
git rev-parse ... >rev2 &&
test_cmp rev1 rev2 &&
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [GSoC] [PATCH] test: avoid pipes in git related commands for test suite
2018-03-14 7:30 ` Eric Sunshine
@ 2018-03-14 9:57 ` Ævar Arnfjörð Bjarmason
2018-03-14 18:22 ` Eric Sunshine
0 siblings, 1 reply; 16+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-03-14 9:57 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Pratik Karki, Git List
On Wed, Mar 14 2018, Eric Sunshine jotted:
> Thanks for the patch. See comments below...
>
> On Tue, Mar 13, 2018 at 4:19 PM, Pratik Karki <predatoramigo@gmail.com> wrote:
>> This patch removes the necessity of pipes in git related commands for test suite.
>>
>> Exit code of the upstream in a pipe is ignored so, it's use should be avoided. The fix for this is to write the output of the git command to a file and test the exit codes of both the commands being linked by pipe.
>
> Please wrap commit messages to fit in about 72 columns; this one is
> far too wide.
>
> On the Git project, commit messages are written in imperative mood, as
> if telling the codebase to "do something". So, instead of writing
> "This patch removes...", you could word it "Remove..." or "Avoid...".
>
> It's misleading to say that the patch "removes the _necessity_ of
> pipes" since pipes were not used out of necessity; they were probably
> just a convenience and seemed reasonable at the time, but later
> experience has shown that they can be problematic for the reason you
> give in the second paragraph.
>
> Taking these observations into consideration, perhaps you could
> rewrite the commit message something like this:
>
> Avoid using pipes downstream of Git commands since the exit codes
> of commands upstream of pipes get swallowed, thus potentially
> hiding failure of those commands. Instead, capture Git command
> output to a file apply the downstream command(s) to that file.
>
> More comments below...
Makes sense.
>> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
>> ---
>> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
>> @@ -116,10 +116,10 @@ test_expect_success \
>> test_expect_success \
>> 'checking the commit' \
>> - 'git diff-tree -r -M --name-status HEAD^ HEAD | \
>> - grep "^R100..*path0/COPYING..*path2/COPYING" &&
>> - git diff-tree -r -M --name-status HEAD^ HEAD | \
>> - grep "^R100..*path0/README..*path2/README"'
>> + 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
>> + grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
>> + git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
>> + grep "^R100..*path0/README..*path2/README" actual'
>
> Although this "mechanical" transformation is technically correct, it
> is nevertheless wasteful. The exact same "git diff-tree ..." command
> is run twice, and both times output is captured to file 'actual',
> which makes the second invocation superfluous. Instead, a better
> transformation would be:
>
> git diff-tree ... >actual &&
> grep ... actual &&
> grep ... actual
>
> The same observation applies to other transformations in this patch.
I think we have to be careful to not be overly picky with rejecting
mechanical transformations that fix bugs on the basis that while we're
at it the test could also be rewritten.
I.e. this bug was there before, maybe we should purely focus on just
replacing the harmful pipe pattern that hides errors in this series and
leave rewriting the actual test logic for a later patch.
>> diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
>> @@ -204,8 +204,8 @@ test_expect_success "follow-parent is atomic" '
>> test_expect_success "track multi-parent paths" '
>> svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
>> git svn multi-fetch &&
>> - test $(git cat-file commit refs/remotes/glob | \
>> - grep "^parent " | wc -l) -eq 2
>> + test $(git cat-file commit refs/remotes/glob >actual &&
>> + grep "^parent " actual | wc -l) -eq 2
>> '
>
> This is not a great transformation. If "git cat-file" fails, then
> neither 'grep' nor 'wc' will run, and the result will be as if 'test'
> was called without an argument before "-eq". For example:
>
> % test $(false >actual && grep "^parent " actual | wc -l) -eq 2
> test: -eq: unary operator expected
>
> It would be better to run "git cat-file" outside of "test $(...)". For instance:
>
> git cat-file ... >actual &&
> test $(grep ... actual | wc -l) -eq 2
>
> Alternately, you could take advantage of the test_line_count() helper function:
>
> git cat-file ... >actual &&
> grep ... actual >actual2 &&
> test_line_count = 2 actual2
In this case though as you rightly point out the rewrite is introducing
a regression, which should definitely be fixed.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [GSoC] [PATCH] test: avoid pipes in git related commands for test suite
2018-03-14 9:57 ` Ævar Arnfjörð Bjarmason
@ 2018-03-14 18:22 ` Eric Sunshine
2018-03-15 17:04 ` Junio C Hamano
0 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2018-03-14 18:22 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Pratik Karki, Git List
On Wed, Mar 14, 2018 at 5:57 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Wed, Mar 14 2018, Eric Sunshine jotted:
>> On Tue, Mar 13, 2018 at 4:19 PM, Pratik Karki <predatoramigo@gmail.com> wrote:
>>> - 'git diff-tree -r -M --name-status HEAD^ HEAD | \
>>> - grep "^R100..*path0/COPYING..*path2/COPYING" &&
>>> - git diff-tree -r -M --name-status HEAD^ HEAD | \
>>> - grep "^R100..*path0/README..*path2/README"'
>>> + 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
>>> + grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
>>> + git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
>>> + grep "^R100..*path0/README..*path2/README" actual'
>>
>> Although this "mechanical" transformation is technically correct, it
>> is nevertheless wasteful. The exact same "git diff-tree ..." command
>> is run twice, and both times output is captured to file 'actual',
>> which makes the second invocation superfluous. Instead, a better
>> transformation would be:
>>
>> git diff-tree ... >actual &&
>> grep ... actual &&
>> grep ... actual
>>
> I think we have to be careful to not be overly picky with rejecting
> mechanical transformations that fix bugs on the basis that while we're
> at it the test could also be rewritten.
>
> I.e. this bug was there before, maybe we should purely focus on just
> replacing the harmful pipe pattern that hides errors in this series and
> leave rewriting the actual test logic for a later patch.
Thanks for presenting an opposing opinion. While I understand your
position, the reason for my suggested transformation is that if the
patch already transformed the code in the way suggested, it would
increase my confidence, as a reviewer, that the patch author had
_studied_ and _understood_ the code. Increased confidence is
especially important for mechanical transformations since -- as seen
in the unsnipped review comment below -- blindly-applied mechanical
transformations can be suboptimal or outright incorrect.
It's also the sort of review comment I would make even to very
seasoned project participants[1].
[1]: https://public-inbox.org/git/CAPig+cQLmYQeRhPxvZHmY7gApnbE25H_KoSWs-ZjuBo4BruimQ@mail.gmail.com/
>>> - test $(git cat-file commit refs/remotes/glob | \
>>> - grep "^parent " | wc -l) -eq 2
>>> + test $(git cat-file commit refs/remotes/glob >actual &&
>>> + grep "^parent " actual | wc -l) -eq 2
>>
>> This is not a great transformation. If "git cat-file" fails, then
>> neither 'grep' nor 'wc' will run, and the result will be as if 'test'
>> was called without an argument before "-eq". For example:
>>
>> % test $(false >actual && grep "^parent " actual | wc -l) -eq 2
>> test: -eq: unary operator expected
>>
>> It would be better to run "git cat-file" outside of "test $(...)". For instance:
>>
>> git cat-file ... >actual &&
>> test $(grep ... actual | wc -l) -eq 2
>>
>> Alternately, you could take advantage of the test_line_count() helper function:
>>
>> git cat-file ... >actual &&
>> grep ... actual >actual2 &&
>> test_line_count = 2 actual2
>
> In this case though as you rightly point out the rewrite is introducing
> a regression, which should definitely be fixed.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [GSoC] [PATCH] test: avoid pipes in git related commands for test suite
2018-03-14 18:22 ` Eric Sunshine
@ 2018-03-15 17:04 ` Junio C Hamano
2018-03-19 17:32 ` [GSoC][PATCH] " Pratik Karki
0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2018-03-15 17:04 UTC (permalink / raw)
To: Eric Sunshine
Cc: Ævar Arnfjörð Bjarmason, Pratik Karki, Git List
Eric Sunshine <sunshine@sunshineco.com> writes:
> Thanks for presenting an opposing opinion. While I understand your
> position, the reason for my suggested transformation is that if the
> patch already transformed the code in the way suggested, it would
> increase my confidence, as a reviewer, that the patch author had
> _studied_ and _understood_ the code. Increased confidence is
> especially important for mechanical transformations since -- as seen
> in the unsnipped review comment below -- blindly-applied mechanical
> transformations can be suboptimal or outright incorrect.
>
> It's also the sort of review comment I would make even to very
> seasoned project participants[1].
>
> [1]: https://public-inbox.org/git/CAPig+cQLmYQeRhPxvZHmY7gApnbE25H_KoSWs-ZjuBo4BruimQ@mail.gmail.com/
Yes, it is a good example that mechanical conversions are often
mind-numbing and make even seasoned participants miss trivially
obvious improvement opportunities ;-)
It however is OK to be more lenient to newer participants and allow
deferring such "while at it, make it right" on top of "minimally
required for correctness", in order to encourage them by getting
something to the tree early ;-)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [GSoC][PATCH] test: avoid pipes in git related commands for test suite
2018-03-15 17:04 ` Junio C Hamano
@ 2018-03-19 17:32 ` Pratik Karki
2018-03-21 11:02 ` Eric Sunshine
0 siblings, 1 reply; 16+ messages in thread
From: Pratik Karki @ 2018-03-19 17:32 UTC (permalink / raw)
To: git; +Cc: Pratik Karki
Thank you Eric Sunshine,
I have done as you had instructed me. I look forward to more
understanding of the codebase and would love to fix
"git rev-parse" problems in my follow-on patches.
Thank you for the professional review comment.
Sorry for late follow-on patch, I got tied up with my university stuffs.
Please do review this patch as before. I will correct it if needed.
Cheers,
Pratik Karki
Avoid using pipes downstream of Git commands since the exit codes
of commands upstream of pipes get swallowed, thus potentially
hiding failure of those commands. Instead, capture Git command
output to a file apply the downstream command(s) to that file.
Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
t/t5300-pack-object.sh | 10 +++---
t/t5510-fetch.sh | 8 ++---
t/t7001-mv.sh | 22 ++++++-------
t/t7003-filter-branch.sh | 9 ++++--
t/t9104-git-svn-follow-parent.sh | 16 +++++----
t/t9108-git-svn-glob.sh | 14 ++++----
t/t9109-git-svn-multi-glob.sh | 28 +++++++++-------
t/t9110-git-svn-use-svm-props.sh | 42 ++++++++++++------------
t/t9111-git-svn-use-svnsync-props.sh | 36 ++++++++++-----------
t/t9114-git-svn-dcommit-merge.sh | 10 +++---
t/t9130-git-svn-authors-file.sh | 28 +++++++++-------
t/t9138-git-svn-authors-prog.sh | 31 +++++++++---------
t/t9153-git-svn-rewrite-uuid.sh | 8 ++---
t/t9168-git-svn-partially-globbed-names.sh | 34 +++++++++++--------
t/t9350-fast-export.sh | 52 ++++++++++++++++--------------
15 files changed, 187 insertions(+), 161 deletions(-)
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 9c68b9925..91207ae0c 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -311,9 +311,9 @@ test_expect_success 'unpacking with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
- PACK6=$( (
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$(git pack-objects test-5 < actual) &&
+ PACK6=$((
echo "$LIST"
echo "$LI"
echo "$ST"
@@ -358,8 +358,8 @@ test_expect_success 'index-pack with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$(git pack-objects test-5 < actual) &&
PACK6=$( (
echo "$LIST"
echo "$LI"
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 668c54be4..c7b284138 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -693,8 +693,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
test_commit looooooooooooong-tag &&
(
cd full-output &&
- git -c fetch.output=full fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=full fetch origin >actual2 2>&1 &&
+ grep -e "->" actual2 | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/master
@@ -708,8 +708,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
test_commit extraaa &&
(
cd compact &&
- git -c fetch.output=compact fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=compact fetch origin >actual2 2>&1 &&
+ grep -e "->" actual2 | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/*
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 6e5031f56..00aa9e45b 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -21,8 +21,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path1/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path1/COPYING" actual'
test_expect_success \
'moving the file back into subdirectory' \
@@ -35,8 +35,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path1/COPYING..*path0/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path1/COPYING..*path0/COPYING" actual'
test_expect_success \
'checking -k on non-existing file' \
@@ -116,10 +116,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/README..*path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
+ grep "^R100..*path0/README..*path2/README" actual'
test_expect_success \
'succeed when source is a prefix of destination' \
@@ -135,10 +134,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/COPYING..*path1/path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/README..*path1/path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path2/COPYING..*path1/path2/COPYING" actual &&
+ grep "^R100..*path2/README..*path1/path2/README" actual'
test_expect_success \
'do not move directory over existing directory' \
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..82c9c2825 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -187,7 +187,8 @@ test_expect_success 'author information is preserved' '
test \$GIT_COMMIT != $(git rev-parse master) || \
echo Hallo" \
preserved-author) &&
- test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l)
+ git rev-list --author="B V Uips" preserved-author > actual &&
+ test 1 = $(wc -l < actual)
'
test_expect_success "remove a certain author's commits" '
@@ -205,7 +206,8 @@ test_expect_success "remove a certain author's commits" '
cnt1=$(git rev-list master | wc -l) &&
cnt2=$(git rev-list removed-author | wc -l) &&
test $cnt1 -eq $(($cnt2 + 1)) &&
- test 0 = $(git rev-list --author="B V Uips" removed-author | wc -l)
+ git rev-list --author="B V Uips" removed-author >actual &&
+ test 0 = $(wc -l < actual)
'
test_expect_success 'barf on invalid name' '
@@ -258,7 +260,8 @@ test_expect_success 'Subdirectory filter with disappearing trees' '
git commit -m "Re-adding foo" &&
git filter-branch -f --subdirectory-filter foo &&
- test $(git rev-list master | wc -l) = 3
+ git rev-list master >actual &&
+ test_line_count = 3 actual
'
test_expect_success 'Tag name filtering retains tag message' '
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index cd480edf1..a532c49af 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -33,8 +33,8 @@ test_expect_success 'init and fetch a moved directory' '
git svn fetch -i thunk &&
test "$(git rev-parse --verify refs/remotes/thunk@2)" \
= "$(git rev-parse --verify refs/remotes/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/thunk:readme |\
- sed -n -e "3p")" = goodbye &&
+ git cat-file blob refs/remotes/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye &&
test -z "$(git config --get svn-remote.svn.fetch \
"^trunk:refs/remotes/thunk@2$")"
'
@@ -48,8 +48,8 @@ test_expect_success 'init and fetch from one svn-remote' '
git svn fetch -i svn/thunk &&
test "$(git rev-parse --verify refs/remotes/svn/trunk)" \
= "$(git rev-parse --verify refs/remotes/svn/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/svn/thunk:readme |\
- sed -n -e "3p")" = goodbye
+ git cat-file blob refs/remotes/svn/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye
'
test_expect_success 'follow deleted parent' '
@@ -107,7 +107,8 @@ test_expect_success 'follow deleted directory' '
git svn init --minimize-url -i glob "$svnrepo"/glob &&
git svn fetch -i glob &&
test "$(git cat-file blob refs/remotes/glob:blob/bye)" = hi &&
- test "$(git ls-tree refs/remotes/glob | wc -l )" -eq 1
+ git ls-tree refs/remotes/glob >actual &&
+ test_line_count = 1 actual
'
# ref: r9270 of the Subversion repository: (http://svn.collab.net/repos/svn)
@@ -204,8 +205,9 @@ test_expect_success "follow-parent is atomic" '
test_expect_success "track multi-parent paths" '
svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
git svn multi-fetch &&
- test $(git cat-file commit refs/remotes/glob | \
- grep "^parent " | wc -l) -eq 2
+ git cat-file commit refs/remotes/glob >actual &&
+ grep "^parent " actual > actual2 &&
+ test_line_count = 2 actual2
'
test_expect_success "multi-fetch continues to work" "
diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
index a94286c8e..e01f3553f 100755
--- a/t/t9108-git-svn-glob.sh
+++ b/t/t9108-git-svn-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual > output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/start >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/two/branches/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 > output.two &&
test_cmp expect.two output.two
'
diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
index 8d99e848d..a09fc3e14 100755
--- a/t/t9109-git-svn-multi-glob.sh
+++ b/t/t9109-git-svn-multi-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual > output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/v1/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/v1/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual2 &&
+ test_line_count = 6 actual2 &&
+ git rev-list refs/remotes/two/branches/v1/start >actual3 &&
+ test_line_count = 3 actual3 &&
test $(git rev-parse refs/remotes/two/branches/v1/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/v1/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual4 &&
+ sed -e "s/^.\{41\}//" actual4 > output.two &&
test_cmp expect.two output.two
'
cat > expect.four <<EOF
@@ -120,18 +122,20 @@ test_expect_success 'test another branch' '
git config --add svn-remote.four.url "$svnrepo" &&
git config --add svn-remote.four.fetch trunk:refs/remotes/four/trunk &&
git config --add svn-remote.four.branches \
- "branches/*/*:refs/remotes/four/branches/*/*" &&
+ "branches/*/*:refs/remotes/four/branches/*/*" &&
git config --add svn-remote.four.tags \
- "tags/*:refs/remotes/four/tags/*" &&
+ "tags/*:refs/remotes/four/tags/*" &&
git svn fetch four &&
- test $(git rev-list refs/remotes/four/tags/next | wc -l) -eq 5 &&
- test $(git rev-list refs/remotes/four/branches/v2/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/four/tags/next >actual &&
+ test_line_count = 5 actual &&
+ git rev-list refs/remotes/four/branches/v2/start >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/four/branches/v2/start~2) = \
$(git rev-parse refs/remotes/four/trunk) &&
test $(git rev-parse refs/remotes/four/tags/next~2) = \
$(git rev-parse refs/remotes/four/branches/v2/start) &&
- git log --pretty=oneline refs/remotes/four/tags/next | \
- sed -e "s/^.\{41\}//" > output.four &&
+ git log --pretty=oneline refs/remotes/four/tags/next >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 > output.four &&
test_cmp expect.four output.four
'
diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
index dde0a3c22..4200b567f 100755
--- a/t/t9110-git-svn-use-svm-props.sh
+++ b/t/t9110-git-svn-use-svm-props.sh
@@ -21,38 +21,38 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual1 &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual1 &&
+ git cat-file commit refs/remotes/bar~2 >actual2 &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual2 &&
+ git cat-file commit refs/remotes/bar~3 >actual3 &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual3 &&
+ git cat-file commit refs/remotes/bar~4 >actual4 &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual4 &&
+ git cat-file commit refs/remotes/bar~5 >actual5 &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual5
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual1 &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual1
"
test_expect_success 'find commit based on SVN revision number' "
- git svn find-rev r12 |
- grep $(git rev-parse HEAD)
- "
+ git svn find-rev r12 >actual &&
+ grep $(git rev-parse HEAD) actual
+ "
test_expect_success 'empty rebase' "
git svn rebase
diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
index 22b6e5ee7..a4225c9f6 100755
--- a/t/t9111-git-svn-use-svnsync-props.sh
+++ b/t/t9111-git-svn-use-svnsync-props.sh
@@ -20,32 +20,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual1 &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual1 &&
+ git cat-file commit refs/remotes/bar~2 >actual2 &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual2 &&
+ git cat-file commit refs/remotes/bar~3 >actual3 &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual3 &&
+ git cat-file commit refs/remotes/bar~4 >actual4 &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual4 &&
+ git cat-file commit refs/remotes/bar~5 >actual5 &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual5
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual1 &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual1
"
test_done
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 50bca62de..32317d6bc 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -68,7 +68,8 @@ test_debug 'gitk --all & sleep 1'
test_expect_success 'verify pre-merge ancestry' "
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'git svn dcommit merges' "
@@ -82,12 +83,13 @@ test_expect_success 'verify post-merge ancestry' "
x\$(git rev-parse --verify refs/remotes/origin/trunk) &&
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'verify merge commit message' "
- git rev-list --pretty=raw -1 refs/heads/svn | \
- grep \" Merge branch 'merge' into svn\"
+ git rev-list --pretty=raw -1 refs/heads/svn >actual &&
+ grep \" Merge branch 'merge' into svn\" actual
"
test_done
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index 41264818c..a0f10b2d2 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -26,11 +26,12 @@ test_expect_success 'start import with incomplete authors file' '
test_expect_success 'imported 2 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 2 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author BBBBBBB BBBBBBB <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author AAAAAAA AAAAAAA <aa@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 2 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author BBBBBBB BBBBBBB <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual1 &&
+ grep "^author AAAAAAA AAAAAAA <aa@example\.com> " actual1
)
'
@@ -43,11 +44,12 @@ test_expect_success 'continues to import once authors have been added' '
(
cd x
git svn fetch --authors-file=../svn-authors &&
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 4 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author DDDDDDD DDDDDDD <dd@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author CCCCCCC CCCCCCC <cc@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 4 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author DDDDDDD DDDDDDD <dd@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual1 &&
+ grep "^author CCCCCCC CCCCCCC <cc@example\.com> " actual1
)
'
@@ -102,8 +104,10 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' '
test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
git svn clone "$svnrepo" gitconfig.clone &&
cd gitconfig.clone &&
- nr_ex=$(git log | grep "^Author:.*example.com" | wc -l) &&
- nr_rev=$(git rev-list HEAD | wc -l) &&
+ nr_ex=$(git log >actual &&
+ grep "^Author:.*example.com" actual | wc -l) &&
+ nr_rev=$(git rev-list HEAD >actual &&
+ wc -l < actual) &&
test $nr_rev -eq $nr_ex
)
'
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 7d7e9d46b..f684f5578 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -37,31 +37,32 @@ test_expect_success 'import authors with prog and file' '
test_expect_success 'imported 6 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 6
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 6 actual
)
'
test_expect_success 'authors-prog ran correctly' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author ee-foo <ee-foo@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~2 | \
- grep "^author dd <dd@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~3 | \
- grep "^author cc <cc@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~4 | \
- grep "^author bb <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~5 | \
- grep "^author aa <aa@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author ee-foo <ee-foo@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~2 >actual2 &&
+ grep "^author dd <dd@sub\.example\.com> " actual2 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~3 >actual3 &&
+ grep "^author cc <cc@sub\.example\.com> " actual3 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~4 >actual4 &&
+ grep "^author bb <bb@example\.com> " actual4 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~5 >actual5 &&
+ grep "^author aa <aa@example\.com> " actual5
)
'
test_expect_success 'authors-file overrode authors-prog' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> " actual
)
'
@@ -73,8 +74,8 @@ test_expect_success 'authors-prog handled special characters in username' '
(
cd x &&
git svn --authors-prog=../svn-authors-prog fetch &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn |
- grep "^author xyz; touch evil <xyz; touch evil@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author xyz; touch evil <xyz; touch evil@example\.com> " actual &&
! test -f evil
)
'
diff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh
index 372ef1568..6cd28bb9a 100755
--- a/t/t9153-git-svn-rewrite-uuid.sh
+++ b/t/t9153-git-svn-rewrite-uuid.sh
@@ -16,10 +16,10 @@ test_expect_success 'load svn repo' "
"
test_expect_success 'verify uuid' "
- git cat-file commit refs/remotes/git-svn~0 | \
- grep '^git-svn-id: .*@2 $uuid$' &&
- git cat-file commit refs/remotes/git-svn~1 | \
- grep '^git-svn-id: .*@1 $uuid$'
+ git cat-file commit refs/remotes/git-svn~0 >actual &&
+ grep '^git-svn-id: .*@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/git-svn~1 >actual1 &&
+ grep '^git-svn-id: .*@1 $uuid$' actual1
"
test_done
diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
index 8b22f2272..df6f3a974 100755
--- a/t/t9168-git-svn-partially-globbed-names.sh
+++ b/t/t9168-git-svn-partially-globbed-names.sh
@@ -48,8 +48,8 @@ test_expect_success 'test refspec prefixed globbing' '
git config --add svn-remote.svn.tags\
"tags/t_*/src/a:refs/remotes/tags/t_*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.end &&
+ git log --pretty=oneline refs/remotes/tags/t_end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/t_end~1)" = \
"$(git rev-parse refs/remotes/branches/b_start)" &&
@@ -78,14 +78,16 @@ test_expect_success 'test left-hand-side only prefixed globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/t_end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/b_start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/t_end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/b_start >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/two/branches/b_start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/t_end~3) = \
$(git rev-parse refs/remotes/two/branches/b_start) &&
- git log --pretty=oneline refs/remotes/two/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/t_end >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 >output.two &&
test_cmp expect.two output.two
'
@@ -118,14 +120,16 @@ test_expect_success 'test prefixed globs match just prefix' '
svn_cmd up
) &&
git svn fetch three &&
- test $(git rev-list refs/remotes/three/branches/b_ | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/three/tags/t_ | wc -l) -eq 3 &&
+ git rev-list refs/remotes/three/branches/b_ >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/three/tags/t_ >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/three/branches/b_~1) = \
$(git rev-parse refs/remotes/three/trunk) &&
test $(git rev-parse refs/remotes/three/tags/t_~1) = \
$(git rev-parse refs/remotes/three/branches/b_) &&
- git log --pretty=oneline refs/remotes/three/tags/t_ | \
- sed -e "s/^.\{41\}//" >output.three &&
+ git log --pretty=oneline refs/remotes/three/tags/t_ >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 >output.three &&
test_cmp expect.three output.three
'
@@ -186,14 +190,16 @@ test_expect_success 'test globbing in the middle of the word' '
svn_cmd up
) &&
git svn fetch five &&
- test $(git rev-list refs/remotes/five/branches/abcde | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/five/tags/fghij | wc -l) -eq 3 &&
+ git rev-list refs/remotes/five/branches/abcde >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/five/tags/fghij >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/five/branches/abcde~1) = \
$(git rev-parse refs/remotes/five/trunk) &&
test $(git rev-parse refs/remotes/five/tags/fghij~1) = \
$(git rev-parse refs/remotes/five/branches/abcde) &&
- git log --pretty=oneline refs/remotes/five/tags/fghij | \
- sed -e "s/^.\{41\}//" >output.five &&
+ git log --pretty=oneline refs/remotes/five/tags/fghij >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 >output.five &&
test_cmp expect.five output.five
'
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 866ddf605..45176742b 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -43,20 +43,20 @@ test_expect_success 'fast-export | fast-import' '
MUSS=$(git rev-parse --verify muss) &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --all |
+ git fast-export --all >actual &&
(cd new &&
git fast-import &&
test $MASTER = $(git rev-parse --verify refs/heads/master) &&
test $REIN = $(git rev-parse --verify refs/tags/rein) &&
test $WER = $(git rev-parse --verify refs/heads/wer) &&
- test $MUSS = $(git rev-parse --verify refs/tags/muss))
+ test $MUSS = $(git rev-parse --verify refs/tags/muss)) < actual
'
test_expect_success 'fast-export master~2..master' '
- git fast-export master~2..master |
- sed "s/master/partial/" |
+ git fast-export master~2..master >actual2 &&
+ sed "s/master/partial/" actual2 |
(cd new &&
git fast-import &&
test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
@@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
test_tick &&
echo rosten >file &&
git commit -s -m den file &&
- git fast-export wer^..wer |
- sed "s/wer/i18n/" |
- (cd new &&
- git fast-import &&
- git cat-file commit i18n | grep "Áéí óú")
+ git fast-export wer^..wer >actual3 &&
+ sed "s/wer/i18n/" actual3 |
+ (cd new &&
+ git fast-import &&
+ git cat-file commit i18n >actual4 &&
+ grep "Áéí óú" actual4)
'
test_expect_success 'import/export-marks' '
@@ -87,18 +88,18 @@ test_expect_success 'import/export-marks' '
git fast-export --export-marks=tmp-marks HEAD &&
test -s tmp-marks &&
test_line_count = 3 tmp-marks &&
+ git fast-export --import-marks=tmp-marks\
+ --export-marks=tmp-marks HEAD >actual &&
test $(
- git fast-export --import-marks=tmp-marks\
- --export-marks=tmp-marks HEAD |
- grep ^commit |
+ grep ^commit actual |
wc -l) \
-eq 0 &&
echo change > file &&
git commit -m "last commit" file &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual2 &&
test $(
- git fast-export --import-marks=tmp-marks \
- --export-marks=tmp-marks HEAD |
- grep ^commit\ |
+ grep ^commit\ actual2 |
wc -l) \
-eq 1 &&
test_line_count = 4 tmp-marks
@@ -184,7 +185,7 @@ test_expect_success 'submodule fast-export | fast-import' '
rm -rf new &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --signed-tags=strip --all |
+ git fast-export --signed-tags=strip --all >actual &&
(cd new &&
git fast-import &&
test "$SUBENT1" = "$(git ls-tree refs/heads/master^ sub)" &&
@@ -192,7 +193,7 @@ test_expect_success 'submodule fast-export | fast-import' '
git checkout master &&
git submodule init &&
git submodule update &&
- cmp sub/file ../sub/file)
+ cmp sub/file ../sub/file) < actual
'
@@ -361,18 +362,20 @@ test_expect_failure 'no exact-ref revisions included' '
)
'
-test_expect_success 'path limiting with import-marks does not lose unmodified files' '
+test_expect_success 'path limiting with import-marks does not lose unmodified files' '
git checkout -b simple marks~2 &&
git fast-export --export-marks=marks simple -- file > /dev/null &&
echo more content >> file &&
test_tick &&
git commit -mnext file &&
- git fast-export --import-marks=marks simple -- file file0 | grep file0
+ git fast-export --import-marks=marks simple -- file file0 >actual &&
+ grep file0 actual
'
-test_expect_success 'full-tree re-shows unmodified files' '
+test_expect_success 'full-tree re-shows unmodified files' '
git checkout -f simple &&
- test $(git fast-export --full-tree simple | grep -c file0) -eq 3
+ git fast-export --full-tree simple >actual &&
+ test $(grep -c file0 actual) -eq 3
'
test_expect_success 'set-up a few more tags for tag export tests' '
@@ -505,8 +508,8 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
'
test_expect_success 'use refspec' '
- git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
- grep "^commit " | sort | uniq > actual &&
+ git fast-export --refspec refs/heads/master:refs/heads/foobar master >actual2 &&
+ grep "^commit " actual2 | sort | uniq > actual &&
echo "commit refs/heads/foobar" > expected &&
test_cmp expected actual
'
@@ -534,7 +537,8 @@ test_expect_success 'when using -C, do not declare copy when source of copy is a
git -C src commit -m 2nd_commit &&
test_create_repo dst &&
- git -C src fast-export --all -C | git -C dst fast-import &&
+ git -C src fast-export --all -C > actual &&
+ git -C dst fast-import < actual &&
git -C src show >expected &&
git -C dst show >actual &&
test_cmp expected actual
--
2.16.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH] test: avoid pipes in git related commands for test suite
2018-03-19 17:32 ` [GSoC][PATCH] " Pratik Karki
@ 2018-03-21 11:02 ` Eric Sunshine
2018-03-21 15:23 ` [GSoC][PATCH v3] test: avoid pipes in git related commands for test Pratik Karki
0 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2018-03-21 11:02 UTC (permalink / raw)
To: Pratik Karki; +Cc: Git List
On Mon, Mar 19, 2018 at 1:32 PM, Pratik Karki <predatoramigo@gmail.com> wrote:
> Thank you Eric Sunshine,
> I have done as you had instructed me. I look forward to more
> understanding of the codebase and would love to fix
> "git rev-parse" problems in my follow-on patches.
> Thank you for the professional review comment.
>
> Sorry for late follow-on patch, I got tied up with my university stuffs.
No need to apologize; this is not a race. It's better to take time
preparing submissions carefully, than trying to rush them out.
> Please do review this patch as before. I will correct it if needed.
Below comments are meant to be instructive and constructive.
> Cheers,
> Pratik Karki
Place a "-- >8--" scissor line right here so that git-am knows where
the commit message begins; otherwise, all of the above commentary will
undesirably be included in the commit message.
> [PATCH] test: avoid pipes in git related commands for test suite
As this is the second attempt at this patch, the subject should be
"[PATCH v2]". Also, as an aid to reviewers -- who see a lot of patches
each day and are likely to forget details of each submission -- please
include a link in the commentary (not in the actual commit message)
pointing at the previous iteration, like this[1].
[1]: https://public-inbox.org/git/20180313201945.8409-1-predatoramigo@gmail.com/
> Avoid using pipes downstream of Git commands since the exit codes
> of commands upstream of pipes get swallowed, thus potentially
> hiding failure of those commands. Instead, capture Git command
> output to a file apply the downstream command(s) to that file.
This rewrite of the commit message which I suggested in [2] has a
grammatical error (which I noticed immediately after hitting "Send").
Unfortunately, you copied it verbatim, so the error is reproduced
here. Specifically, you want to insert "and" between "file" and
"apply":
... capture Git command output to a file _and_ apply...
[2]: https://public-inbox.org/git/CAPig+cRPzyw525ODC4=-E7w=zbpbhVN2eqxSYDSLij5wfW8S_A@mail.gmail.com/
> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
> ---
> t/t5300-pack-object.sh | 10 +++---
> t/t5510-fetch.sh | 8 ++---
> t/t7001-mv.sh | 22 ++++++-------
> t/t7003-filter-branch.sh | 9 ++++--
> t/t9104-git-svn-follow-parent.sh | 16 +++++----
> t/t9108-git-svn-glob.sh | 14 ++++----
> t/t9109-git-svn-multi-glob.sh | 28 +++++++++-------
> t/t9110-git-svn-use-svm-props.sh | 42 ++++++++++++------------
> t/t9111-git-svn-use-svnsync-props.sh | 36 ++++++++++-----------
> t/t9114-git-svn-dcommit-merge.sh | 10 +++---
> t/t9130-git-svn-authors-file.sh | 28 +++++++++-------
> t/t9138-git-svn-authors-prog.sh | 31 +++++++++---------
> t/t9153-git-svn-rewrite-uuid.sh | 8 ++---
> t/t9168-git-svn-partially-globbed-names.sh | 34 +++++++++++--------
> t/t9350-fast-export.sh | 52 ++++++++++++++++--------------
> 15 files changed, 187 insertions(+), 161 deletions(-)
The goal of iterating a patch or patch series is to converge to a
point at which the submission is in good enough shape to be accepted.
Ideally, each iteration should involve fewer changes than the previous
attempt.
Version 1 of this patch touched only 8 files, however, this version
touches 15, and is now uncomfortably large and difficult to review in
a single sitting (it took over 1.5 hours). Rather than converging, it
has instead diverged, and is thus potentially further from being in an
acceptable state than it would have been if v2 had merely addressed
the problems identified by the v1 review.
While the desire to address these additional cases is admirable, it is
better to focus on "landing" the current patch (getting it accepted)
before expanding your efforts; it's also more reviewer-friendly to
stay focused, especially with patches, such as this, which involve
primarily mechanical changes (which tend to be mind-numbing to
review).
> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> @@ -311,9 +311,9 @@ test_expect_success 'unpacking with --strict' '
> rm -f .git/index &&
> tail -n 10 LIST | git update-index --index-info &&
> ST=$(git write-tree) &&
> - PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
> - git pack-objects test-5 ) &&
> - PACK6=$( (
> + git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
> + PACK5=$(git pack-objects test-5 < actual) &&
> + PACK6=$((
Losing the space between the two left parentheses is wrong. $( ( foo )
), which captures the output of subshell running 'foo', has very
different meaning than $((foo)), which performs arithmetic. This
change turns it into $(( foo) ), which, at best, is undefined.
Although bash seems to tolerate this change, other more strict shells
barf on it.
> echo "$LIST"
> echo "$LI"
> echo "$ST"
> @@ -358,8 +358,8 @@ test_expect_success 'index-pack with --strict' '
> rm -f .git/index &&
> tail -n 10 LIST | git update-index --index-info &&
> ST=$(git write-tree) &&
> - PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
> - git pack-objects test-5 ) &&
> + git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
> + PACK5=$(git pack-objects test-5 < actual) &&
> PACK6=$( (
> echo "$LIST"
> echo "$LI"
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> @@ -187,7 +187,8 @@ test_expect_success 'author information is preserved' '
> test \$GIT_COMMIT != $(git rev-parse master) || \
> echo Hallo" \
> preserved-author) &&
> - test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l)
> + git rev-list --author="B V Uips" preserved-author > actual &&
Style: drop space after '>'
> + test 1 = $(wc -l < actual)
Style: drop space after '<'
You could also take advantage of test_line_count() rather than using 'wc':
test_line_count = 1 actual
It's a judgment call whether or not to convert this to use
test_line_count(), however, since you made such a conversion later in
this file, you should do it here too. Or, don't use test_line_count()
in this file. It's not so important whether you do or not, but it is
important that you be consistent about it (which is not currently the
case).
> @@ -205,7 +206,8 @@ test_expect_success "remove a certain author's commits" '
> cnt1=$(git rev-list master | wc -l) &&
> cnt2=$(git rev-list removed-author | wc -l) &&
> test $cnt1 -eq $(($cnt2 + 1)) &&
> - test 0 = $(git rev-list --author="B V Uips" removed-author | wc -l)
> + git rev-list --author="B V Uips" removed-author >actual &&
> + test 0 = $(wc -l < actual)
Ditto: drop space before '<'
Ditto: test_line_count = 0 actual
> @@ -258,7 +260,8 @@ test_expect_success 'Subdirectory filter with disappearing trees' '
> git filter-branch -f --subdirectory-filter foo &&
> - test $(git rev-list master | wc -l) = 3
> + git rev-list master >actual &&
> + test_line_count = 3 actual
> '
Here you used test_line_count().
> diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
> @@ -204,8 +205,9 @@ test_expect_success "follow-parent is atomic" '
> test_expect_success "track multi-parent paths" '
> svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
> git svn multi-fetch &&
> - test $(git cat-file commit refs/remotes/glob | \
> - grep "^parent " | wc -l) -eq 2
> + git cat-file commit refs/remotes/glob >actual &&
> + grep "^parent " actual > actual2 &&
Style: drop space after '>'
> + test_line_count = 2 actual2
> diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
> @@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
> git config --add svn-remote.svn.tags\
> "tags/*/src/a:refs/remotes/tags/*" &&
> git svn multi-fetch &&
> - git log --pretty=oneline refs/remotes/tags/end | \
> - sed -e "s/^.\{41\}//" > output.end &&
> + git log --pretty=oneline refs/remotes/tags/end >actual &&
> + sed -e "s/^.\{41\}//" actual > output.end &&
This is not a new problem, but since you're touching it, drop space after '>'.
> test_cmp expect.end output.end &&
> @@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
> git svn fetch two &&
> - test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
> - test $(git rev-list refs/remotes/two/branches/start | wc -l) -eq 3 &&
> + git rev-list refs/remotes/two/tags/end >actual &&
> + test_line_count = 6 actual &&
> + git rev-list refs/remotes/two/branches/start >actual2 &&
> + test_line_count = 3 actual2 &&
It's better to name all these files "actual", rather than inventing
names "actual2", "actual3", etc. Those invented names mislead readers
into thinking that there might be some interrelation between the files
which requires them all to exist at once. But, this is not the case.
Those files serve no purpose after their associated test_line_count(),
and you can clearly indicate such by overwriting each time by reusing
the name "actual".
> test $(git rev-parse refs/remotes/two/branches/start~2) = \
> $(git rev-parse refs/remotes/two/trunk) &&
> test $(git rev-parse refs/remotes/two/tags/end~3) = \
> $(git rev-parse refs/remotes/two/branches/start) &&
> - git log --pretty=oneline refs/remotes/two/tags/end | \
> - sed -e "s/^.\{41\}//" > output.two &&
> + git log --pretty=oneline refs/remotes/two/tags/end >actual3 &&
> + sed -e "s/^.\{41\}//" actual3 > output.two &&
Style: drop space after '>'
> test_cmp expect.two output.two
> diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
> @@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
> git config --add svn-remote.svn.tags\
> "tags/*/src/a:refs/remotes/tags/*" &&
> git svn multi-fetch &&
> - git log --pretty=oneline refs/remotes/tags/end | \
> - sed -e "s/^.\{41\}//" > output.end &&
> + git log --pretty=oneline refs/remotes/tags/end >actual &&
> + sed -e "s/^.\{41\}//" actual > output.end &&
Style: drop space after '>'
> test_cmp expect.end output.end &&
> @@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
> git svn fetch two &&
> - test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
> - test $(git rev-list refs/remotes/two/branches/v1/start | wc -l) -eq 3 &&
> + git rev-list refs/remotes/two/tags/end >actual2 &&
> + test_line_count = 6 actual2 &&
> + git rev-list refs/remotes/two/branches/v1/start >actual3 &&
> + test_line_count = 3 actual3 &&
Ditto: name all these files "actual" rather than unnecessarily
inventing unique names
> test $(git rev-parse refs/remotes/two/branches/v1/start~2) = \
> $(git rev-parse refs/remotes/two/trunk) &&
> test $(git rev-parse refs/remotes/two/tags/end~3) = \
> $(git rev-parse refs/remotes/two/branches/v1/start) &&
> - git log --pretty=oneline refs/remotes/two/tags/end | \
> - sed -e "s/^.\{41\}//" > output.two &&
> + git log --pretty=oneline refs/remotes/two/tags/end >actual4 &&
> + sed -e "s/^.\{41\}//" actual4 > output.two &&
Style: drop space after '>'
> test_cmp expect.two output.two
> @@ -120,18 +122,20 @@ test_expect_success 'test another branch' '
> git config --add svn-remote.four.url "$svnrepo" &&
> git config --add svn-remote.four.fetch trunk:refs/remotes/four/trunk &&
> git config --add svn-remote.four.branches \
> - "branches/*/*:refs/remotes/four/branches/*/*" &&
> + "branches/*/*:refs/remotes/four/branches/*/*" &&
> git config --add svn-remote.four.tags \
> - "tags/*:refs/remotes/four/tags/*" &&
> + "tags/*:refs/remotes/four/tags/*" &&
I guess you sneaked in a whitespace change here which is unrelated to
the stated purpose of this patch, thus acted as a speed bump during
review. If this was the only instance in this test script of
whitespace needing correction, then it _might_ be okay to include it
in this patch, however, that's not the case. There are many other such
instance in this test script which could be corrected, so it doesn't
make sense to single out these two and ignore all the others.
Therefore, you should omit this change.
> git svn fetch four &&
> - test $(git rev-list refs/remotes/four/tags/next | wc -l) -eq 5 &&
> - test $(git rev-list refs/remotes/four/branches/v2/start | wc -l) -eq 3 &&
> + git rev-list refs/remotes/four/tags/next >actual &&
> + test_line_count = 5 actual &&
> + git rev-list refs/remotes/four/branches/v2/start >actual2 &&
> + test_line_count = 3 actual2 &&
Ditto: name all these files "actual" rather than unnecessarily
inventing unique names
> test $(git rev-parse refs/remotes/four/branches/v2/start~2) = \
> $(git rev-parse refs/remotes/four/trunk) &&
> test $(git rev-parse refs/remotes/four/tags/next~2) = \
> $(git rev-parse refs/remotes/four/branches/v2/start) &&
> - git log --pretty=oneline refs/remotes/four/tags/next | \
> - sed -e "s/^.\{41\}//" > output.four &&
> + git log --pretty=oneline refs/remotes/four/tags/next >actual3 &&
> + sed -e "s/^.\{41\}//" actual3 > output.four &&
Style: drop space after '>'
> test_cmp expect.four output.four
> diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
> @@ -21,38 +21,38 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
> test_expect_success 'verify metadata for /bar' "
> - git cat-file commit refs/remotes/bar | \
> - grep '^git-svn-id: $bar_url@12 $uuid$' &&
> - git cat-file commit refs/remotes/bar~1 | \
> - grep '^git-svn-id: $bar_url@11 $uuid$' &&
> - git cat-file commit refs/remotes/bar~2 | \
> - grep '^git-svn-id: $bar_url@10 $uuid$' &&
> - git cat-file commit refs/remotes/bar~3 | \
> - grep '^git-svn-id: $bar_url@9 $uuid$' &&
> - git cat-file commit refs/remotes/bar~4 | \
> - grep '^git-svn-id: $bar_url@6 $uuid$' &&
> - git cat-file commit refs/remotes/bar~5 | \
> - grep '^git-svn-id: $bar_url@1 $uuid$'
> + git cat-file commit refs/remotes/bar >actual &&
> + grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~1 >actual1 &&
> + grep '^git-svn-id: $bar_url@11 $uuid$' actual1 &&
> + git cat-file commit refs/remotes/bar~2 >actual2 &&
> + grep '^git-svn-id: $bar_url@10 $uuid$' actual2 &&
> + git cat-file commit refs/remotes/bar~3 >actual3 &&
> + grep '^git-svn-id: $bar_url@9 $uuid$' actual3 &&
> + git cat-file commit refs/remotes/bar~4 >actual4 &&
> + grep '^git-svn-id: $bar_url@6 $uuid$' actual4 &&
> + git cat-file commit refs/remotes/bar~5 >actual5 &&
> + grep '^git-svn-id: $bar_url@1 $uuid$' actual5
> "
Ditto: name all these files "actual" rather than unnecessarily
inventing unique names
Same comment applies to many more tests below; I won't be repeating it
beyond this point.
> diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
> @@ -102,8 +104,10 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' '
> test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
> git svn clone "$svnrepo" gitconfig.clone &&
> cd gitconfig.clone &&
> - nr_ex=$(git log | grep "^Author:.*example.com" | wc -l) &&
> - nr_rev=$(git rev-list HEAD | wc -l) &&
> + nr_ex=$(git log >actual &&
> + grep "^Author:.*example.com" actual | wc -l) &&
> + nr_rev=$(git rev-list HEAD >actual &&
> + wc -l < actual) &&
> test $nr_rev -eq $nr_ex
This transformation is effectively bogus, as explained already in my
review[3] of v1. If git-log or git-rev-list fails, variable nr_ex or
nr_rev will have an empty value, thus 'test' will error out. Move the
git-log and git-rev-list invocations out of the $(...):
git log >actual &&
nr_ex=$(grep "..." actual | wc -l) &&
git rev-list HEAD >actual &&
nr_rev=$(wc -l <actual) &&
test $nr_rev -eq $nr_ex
Also, style: drop space after '<'
[3]: https://public-inbox.org/git/CAPig+cRPzyw525ODC4=-E7w=zbpbhVN2eqxSYDSLij5wfW8S_A@mail.gmail.com/
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> @@ -43,20 +43,20 @@ test_expect_success 'fast-export | fast-import' '
> MUSS=$(git rev-parse --verify muss) &&
> mkdir new &&
> git --git-dir=new/.git init &&
> - git fast-export --all |
> + git fast-export --all >actual &&
> (cd new &&
> git fast-import &&
> test $MASTER = $(git rev-parse --verify refs/heads/master) &&
> test $REIN = $(git rev-parse --verify refs/tags/rein) &&
> test $WER = $(git rev-parse --verify refs/heads/wer) &&
> - test $MUSS = $(git rev-parse --verify refs/tags/muss))
> + test $MUSS = $(git rev-parse --verify refs/tags/muss)) < actual
Style: drop space after '<'
> '
>
> test_expect_success 'fast-export master~2..master' '
>
> - git fast-export master~2..master |
> - sed "s/master/partial/" |
> + git fast-export master~2..master >actual2 &&
> + sed "s/master/partial/" actual2 |
Not sure why you named this "actual2" rather than just "actual".
> (cd new &&
> git fast-import &&
> test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
> @@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
> git commit -s -m den file &&
> - git fast-export wer^..wer |
> - sed "s/wer/i18n/" |
> - (cd new &&
> - git fast-import &&
> - git cat-file commit i18n | grep "Áéí óú")
> + git fast-export wer^..wer >actual3 &&
> + sed "s/wer/i18n/" actual3 |
Ditto: Why "actual3" rather than "actual"?
> + (cd new &&
> + git fast-import &&
> + git cat-file commit i18n >actual4 &&
> + grep "Áéí óú" actual4)
> @@ -87,18 +88,18 @@ test_expect_success 'import/export-marks' '
> git fast-export --export-marks=tmp-marks HEAD &&
> test -s tmp-marks &&
> test_line_count = 3 tmp-marks &&
> + git fast-export --import-marks=tmp-marks\
> + --export-marks=tmp-marks HEAD >actual &&
Style: not a new problem, but add space before \ since you're touching it
> test $(
> - git fast-export --import-marks=tmp-marks\
> - --export-marks=tmp-marks HEAD |
> - grep ^commit |
> + grep ^commit actual |
> wc -l) \
> -eq 0 &&
> @@ -192,7 +193,7 @@ test_expect_success 'submodule fast-export | fast-import' '
> git checkout master &&
> git submodule init &&
> git submodule update &&
> - cmp sub/file ../sub/file)
> + cmp sub/file ../sub/file) < actual
Style: drop space after '<'
> @@ -361,18 +362,20 @@ test_expect_failure 'no exact-ref revisions included' '
> -test_expect_success 'path limiting with import-marks does not lose unmodified files' '
> +test_expect_success 'path limiting with import-marks does not lose unmodified files' '
It's probably better not to sneak in whitespace changes, especially if
you're not fixing all of them in this script (and this isn't even the
correct fix). Same comment applies below.
> -test_expect_success 'full-tree re-shows unmodified files' '
> +test_expect_success 'full-tree re-shows unmodified files' '
> @@ -505,8 +508,8 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
> test_expect_success 'use refspec' '
> - git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
> - grep "^commit " | sort | uniq > actual &&
> + git fast-export --refspec refs/heads/master:refs/heads/foobar master >actual2 &&
> + grep "^commit " actual2 | sort | uniq > actual &&
Style: drop space before '>' (and probably the next line too)
> echo "commit refs/heads/foobar" > expected &&
> @@ -534,7 +537,8 @@ test_expect_success 'when using -C, do not declare copy when source of copy is a
> test_create_repo dst &&
> - git -C src fast-export --all -C | git -C dst fast-import &&
> + git -C src fast-export --all -C > actual &&
> + git -C dst fast-import < actual &&
Style: drop space before '>' and '<'
> git -C src show >expected &&
> git -C dst show >actual &&
> test_cmp expected actual
^ permalink raw reply [flat|nested] 16+ messages in thread
* [GSoC][PATCH v3] test: avoid pipes in git related commands for test
2018-03-21 11:02 ` Eric Sunshine
@ 2018-03-21 15:23 ` Pratik Karki
2018-03-21 18:11 ` Junio C Hamano
0 siblings, 1 reply; 16+ messages in thread
From: Pratik Karki @ 2018-03-21 15:23 UTC (permalink / raw)
To: Git List; +Cc: Pratik Karki, Eric Sunshine
Thank you Eric, for the review. This is follow on patch[1].
The changes in patch increased from v1 to v2 because I
got excited to work in Git codebase and I tried to
fix the exisiting problems as much as possible.
Hence, the large number of changes.
>> test_cmp expect.two output.two
>> @@ -120,18 +122,20 @@ test_expect_success 'test another branch' '
>> git config --add svn-remote.four.url "$svnrepo" &&
>> git config --add svn-remote.four.fetch trunk:refs/remotes/four/trunk &&
>> git config --add svn-remote.four.branches \
>> - "branches/*/*:refs/remotes/four/branches/*/*" &&
>> + "branches/*/*:refs/remotes/four/branches/*/*" &&
>> git config --add svn-remote.four.tags \
>> - "tags/*:refs/remotes/four/tags/*" &&
>> + "tags/*:refs/remotes/four/tags/*" &&
>I guess you sneaked in a whitespace change here which is unrelated to
>the stated purpose of this patch, thus acted as a speed bump during
>review. If this was the only instance in this test script of
>whitespace needing correction, then it _might_ be okay to include it
>in this patch, however, that's not the case. There are many other such
>instance in this test script which could be corrected, so it doesn't
>make sense to single out these two and ignore all the others.
>Therefore, you should omit this change.
I used tabify in Emacs and it must have messed up the whitespace
change. I read SubmittingPatches guideline[2] for git where it
is said that whitespace check must be done and git community is
picky about it and 'git diff --check' must be done before commit.
If I change this change back to original the 'git diff --check'
reports whitespace identation with space. So, isn't this
supposedly a fix?
[1]: https://public-inbox.org/git/20180319173204.31952-1-predatoramigo@gmail.com/
[2]: https://github.com/git/git/blob/master/Documentation/SubmittingPatches
------------------------------------- >8----------------------------------------
Avoid using pipes downstream of Git commands since the exit
codes of commands upstream of pipes get swallowed, thus potentially hiding
failure of those commands. Instead, capture Git command output to a file and
apply the downstream command(s) to that file.
Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
t/t5300-pack-object.sh | 10 +++---
t/t5510-fetch.sh | 8 ++---
t/t7001-mv.sh | 22 ++++++------
t/t7003-filter-branch.sh | 9 +++--
t/t9104-git-svn-follow-parent.sh | 16 +++++----
t/t9108-git-svn-glob.sh | 14 ++++----
t/t9109-git-svn-multi-glob.sh | 38 +++++++++++----------
t/t9110-git-svn-use-svm-props.sh | 42 +++++++++++------------
t/t9111-git-svn-use-svnsync-props.sh | 36 ++++++++++----------
t/t9114-git-svn-dcommit-merge.sh | 10 +++---
t/t9130-git-svn-authors-file.sh | 28 +++++++++-------
t/t9138-git-svn-authors-prog.sh | 31 ++++++++---------
t/t9153-git-svn-rewrite-uuid.sh | 8 ++---
t/t9168-git-svn-partially-globbed-names.sh | 34 +++++++++++--------
t/t9350-fast-export.sh | 54 ++++++++++++++++--------------
15 files changed, 193 insertions(+), 167 deletions(-)
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 9c68b9925..707208284 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -311,9 +311,9 @@ test_expect_success 'unpacking with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
- PACK6=$( (
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$( git pack-objects test-5 <actual ) &&
+ PACK6=$((
echo "$LIST"
echo "$LI"
echo "$ST"
@@ -358,8 +358,8 @@ test_expect_success 'index-pack with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$( git pack-objects test-5 <actual ) &&
PACK6=$( (
echo "$LIST"
echo "$LI"
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 668c54be4..c7b284138 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -693,8 +693,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
test_commit looooooooooooong-tag &&
(
cd full-output &&
- git -c fetch.output=full fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=full fetch origin >actual2 2>&1 &&
+ grep -e "->" actual2 | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/master
@@ -708,8 +708,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
test_commit extraaa &&
(
cd compact &&
- git -c fetch.output=compact fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=compact fetch origin >actual2 2>&1 &&
+ grep -e "->" actual2 | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/*
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 6e5031f56..00aa9e45b 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -21,8 +21,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path1/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path1/COPYING" actual'
test_expect_success \
'moving the file back into subdirectory' \
@@ -35,8 +35,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path1/COPYING..*path0/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path1/COPYING..*path0/COPYING" actual'
test_expect_success \
'checking -k on non-existing file' \
@@ -116,10 +116,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/README..*path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
+ grep "^R100..*path0/README..*path2/README" actual'
test_expect_success \
'succeed when source is a prefix of destination' \
@@ -135,10 +134,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/COPYING..*path1/path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/README..*path1/path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path2/COPYING..*path1/path2/COPYING" actual &&
+ grep "^R100..*path2/README..*path1/path2/README" actual'
test_expect_success \
'do not move directory over existing directory' \
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..6a28b6cce 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -187,7 +187,8 @@ test_expect_success 'author information is preserved' '
test \$GIT_COMMIT != $(git rev-parse master) || \
echo Hallo" \
preserved-author) &&
- test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l)
+ git rev-list --author="B V Uips" preserved-author >actual &&
+ test_line_count = 1 actual
'
test_expect_success "remove a certain author's commits" '
@@ -205,7 +206,8 @@ test_expect_success "remove a certain author's commits" '
cnt1=$(git rev-list master | wc -l) &&
cnt2=$(git rev-list removed-author | wc -l) &&
test $cnt1 -eq $(($cnt2 + 1)) &&
- test 0 = $(git rev-list --author="B V Uips" removed-author | wc -l)
+ git rev-list --author="B V Uips" removed-author >actual &&
+ test_line_count = 0 actual
'
test_expect_success 'barf on invalid name' '
@@ -258,7 +260,8 @@ test_expect_success 'Subdirectory filter with disappearing trees' '
git commit -m "Re-adding foo" &&
git filter-branch -f --subdirectory-filter foo &&
- test $(git rev-list master | wc -l) = 3
+ git rev-list master >actual &&
+ test_line_count = 3 actual
'
test_expect_success 'Tag name filtering retains tag message' '
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index cd480edf1..a735fa371 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -33,8 +33,8 @@ test_expect_success 'init and fetch a moved directory' '
git svn fetch -i thunk &&
test "$(git rev-parse --verify refs/remotes/thunk@2)" \
= "$(git rev-parse --verify refs/remotes/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/thunk:readme |\
- sed -n -e "3p")" = goodbye &&
+ git cat-file blob refs/remotes/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye &&
test -z "$(git config --get svn-remote.svn.fetch \
"^trunk:refs/remotes/thunk@2$")"
'
@@ -48,8 +48,8 @@ test_expect_success 'init and fetch from one svn-remote' '
git svn fetch -i svn/thunk &&
test "$(git rev-parse --verify refs/remotes/svn/trunk)" \
= "$(git rev-parse --verify refs/remotes/svn/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/svn/thunk:readme |\
- sed -n -e "3p")" = goodbye
+ git cat-file blob refs/remotes/svn/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye
'
test_expect_success 'follow deleted parent' '
@@ -107,7 +107,8 @@ test_expect_success 'follow deleted directory' '
git svn init --minimize-url -i glob "$svnrepo"/glob &&
git svn fetch -i glob &&
test "$(git cat-file blob refs/remotes/glob:blob/bye)" = hi &&
- test "$(git ls-tree refs/remotes/glob | wc -l )" -eq 1
+ git ls-tree refs/remotes/glob >actual &&
+ test_line_count = 1 actual
'
# ref: r9270 of the Subversion repository: (http://svn.collab.net/repos/svn)
@@ -204,8 +205,9 @@ test_expect_success "follow-parent is atomic" '
test_expect_success "track multi-parent paths" '
svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
git svn multi-fetch &&
- test $(git cat-file commit refs/remotes/glob | \
- grep "^parent " | wc -l) -eq 2
+ git cat-file commit refs/remotes/glob >actual &&
+ grep "^parent " actual >actual2 &&
+ test_line_count = 2 actual2
'
test_expect_success "multi-fetch continues to work" "
diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
index a94286c8e..6990f6436 100755
--- a/t/t9108-git-svn-glob.sh
+++ b/t/t9108-git-svn-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
index 8d99e848d..5abffe1a4 100755
--- a/t/t9109-git-svn-multi-glob.sh
+++ b/t/t9109-git-svn-multi-glob.sh
@@ -41,14 +41,14 @@ test_expect_success 'test refspec globbing' '
) &&
git config --add svn-remote.svn.url "$svnrepo" &&
git config --add svn-remote.svn.fetch \
- "trunk/src/a:refs/remotes/trunk" &&
+ "trunk/src/a:refs/remotes/trunk" &&
git config --add svn-remote.svn.branches \
- "branches/*/*/src/a:refs/remotes/branches/*/*" &&
+ "branches/*/*/src/a:refs/remotes/branches/*/*" &&
git config --add svn-remote.svn.tags\
- "tags/*/src/a:refs/remotes/tags/*" &&
+ "tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/v1/start)" &&
@@ -65,9 +65,9 @@ test_expect_success 'test left-hand-side only globbing' '
git config --add svn-remote.two.url "$svnrepo" &&
git config --add svn-remote.two.fetch trunk:refs/remotes/two/trunk &&
git config --add svn-remote.two.branches \
- "branches/*/*:refs/remotes/two/branches/*/*" &&
+ "branches/*/*:refs/remotes/two/branches/*/*" &&
git config --add svn-remote.two.tags \
- "tags/*:refs/remotes/two/tags/*" &&
+ "tags/*:refs/remotes/two/tags/*" &&
(
cd tmp &&
echo "try try" >> tags/end/src/b/readme &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/v1/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/v1/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/v1/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/v1/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
cat > expect.four <<EOF
@@ -120,18 +122,20 @@ test_expect_success 'test another branch' '
git config --add svn-remote.four.url "$svnrepo" &&
git config --add svn-remote.four.fetch trunk:refs/remotes/four/trunk &&
git config --add svn-remote.four.branches \
- "branches/*/*:refs/remotes/four/branches/*/*" &&
+ "branches/*/*:refs/remotes/four/branches/*/*" &&
git config --add svn-remote.four.tags \
- "tags/*:refs/remotes/four/tags/*" &&
+ "tags/*:refs/remotes/four/tags/*" &&
git svn fetch four &&
- test $(git rev-list refs/remotes/four/tags/next | wc -l) -eq 5 &&
- test $(git rev-list refs/remotes/four/branches/v2/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/four/tags/next >actual &&
+ test_line_count = 5 actual &&
+ git rev-list refs/remotes/four/branches/v2/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/four/branches/v2/start~2) = \
$(git rev-parse refs/remotes/four/trunk) &&
test $(git rev-parse refs/remotes/four/tags/next~2) = \
$(git rev-parse refs/remotes/four/branches/v2/start) &&
- git log --pretty=oneline refs/remotes/four/tags/next | \
- sed -e "s/^.\{41\}//" > output.four &&
+ git log --pretty=oneline refs/remotes/four/tags/next >actual &&
+ sed -e "s/^.\{41\}//" actual >output.four &&
test_cmp expect.four output.four
'
diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
index dde0a3c22..dbd9590a7 100755
--- a/t/t9110-git-svn-use-svm-props.sh
+++ b/t/t9110-git-svn-use-svm-props.sh
@@ -21,38 +21,38 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_expect_success 'find commit based on SVN revision number' "
- git svn find-rev r12 |
- grep $(git rev-parse HEAD)
- "
+ git svn find-rev r12 >actual &&
+ grep $(git rev-parse HEAD) actual
+ "
test_expect_success 'empty rebase' "
git svn rebase
diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
index 22b6e5ee7..a4225c9f6 100755
--- a/t/t9111-git-svn-use-svnsync-props.sh
+++ b/t/t9111-git-svn-use-svnsync-props.sh
@@ -20,32 +20,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual1 &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual1 &&
+ git cat-file commit refs/remotes/bar~2 >actual2 &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual2 &&
+ git cat-file commit refs/remotes/bar~3 >actual3 &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual3 &&
+ git cat-file commit refs/remotes/bar~4 >actual4 &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual4 &&
+ git cat-file commit refs/remotes/bar~5 >actual5 &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual5
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual1 &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual1
"
test_done
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 50bca62de..32317d6bc 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -68,7 +68,8 @@ test_debug 'gitk --all & sleep 1'
test_expect_success 'verify pre-merge ancestry' "
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'git svn dcommit merges' "
@@ -82,12 +83,13 @@ test_expect_success 'verify post-merge ancestry' "
x\$(git rev-parse --verify refs/remotes/origin/trunk) &&
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'verify merge commit message' "
- git rev-list --pretty=raw -1 refs/heads/svn | \
- grep \" Merge branch 'merge' into svn\"
+ git rev-list --pretty=raw -1 refs/heads/svn >actual &&
+ grep \" Merge branch 'merge' into svn\" actual
"
test_done
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index 41264818c..7752a1fae 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -26,11 +26,12 @@ test_expect_success 'start import with incomplete authors file' '
test_expect_success 'imported 2 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 2 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author BBBBBBB BBBBBBB <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author AAAAAAA AAAAAAA <aa@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 2 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author BBBBBBB BBBBBBB <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author AAAAAAA AAAAAAA <aa@example\.com> " actual
)
'
@@ -43,11 +44,12 @@ test_expect_success 'continues to import once authors have been added' '
(
cd x
git svn fetch --authors-file=../svn-authors &&
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 4 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author DDDDDDD DDDDDDD <dd@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author CCCCCCC CCCCCCC <cc@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 4 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author DDDDDDD DDDDDDD <dd@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author CCCCCCC CCCCCCC <cc@example\.com> " actual
)
'
@@ -102,8 +104,10 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' '
test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
git svn clone "$svnrepo" gitconfig.clone &&
cd gitconfig.clone &&
- nr_ex=$(git log | grep "^Author:.*example.com" | wc -l) &&
- nr_rev=$(git rev-list HEAD | wc -l) &&
+ git log >actual &&
+ nr_ex=$(grep "^Author:.*example.com" actual | wc -l) &&
+ git rev-list HEAD >actual &&
+ nr_rev=$(wc -l <actual) &&
test $nr_rev -eq $nr_ex
)
'
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 7d7e9d46b..f684f5578 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -37,31 +37,32 @@ test_expect_success 'import authors with prog and file' '
test_expect_success 'imported 6 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 6
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 6 actual
)
'
test_expect_success 'authors-prog ran correctly' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author ee-foo <ee-foo@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~2 | \
- grep "^author dd <dd@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~3 | \
- grep "^author cc <cc@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~4 | \
- grep "^author bb <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~5 | \
- grep "^author aa <aa@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author ee-foo <ee-foo@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~2 >actual2 &&
+ grep "^author dd <dd@sub\.example\.com> " actual2 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~3 >actual3 &&
+ grep "^author cc <cc@sub\.example\.com> " actual3 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~4 >actual4 &&
+ grep "^author bb <bb@example\.com> " actual4 &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~5 >actual5 &&
+ grep "^author aa <aa@example\.com> " actual5
)
'
test_expect_success 'authors-file overrode authors-prog' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> " actual
)
'
@@ -73,8 +74,8 @@ test_expect_success 'authors-prog handled special characters in username' '
(
cd x &&
git svn --authors-prog=../svn-authors-prog fetch &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn |
- grep "^author xyz; touch evil <xyz; touch evil@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author xyz; touch evil <xyz; touch evil@example\.com> " actual &&
! test -f evil
)
'
diff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh
index 372ef1568..6cd28bb9a 100755
--- a/t/t9153-git-svn-rewrite-uuid.sh
+++ b/t/t9153-git-svn-rewrite-uuid.sh
@@ -16,10 +16,10 @@ test_expect_success 'load svn repo' "
"
test_expect_success 'verify uuid' "
- git cat-file commit refs/remotes/git-svn~0 | \
- grep '^git-svn-id: .*@2 $uuid$' &&
- git cat-file commit refs/remotes/git-svn~1 | \
- grep '^git-svn-id: .*@1 $uuid$'
+ git cat-file commit refs/remotes/git-svn~0 >actual &&
+ grep '^git-svn-id: .*@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/git-svn~1 >actual1 &&
+ grep '^git-svn-id: .*@1 $uuid$' actual1
"
test_done
diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
index 8b22f2272..df6f3a974 100755
--- a/t/t9168-git-svn-partially-globbed-names.sh
+++ b/t/t9168-git-svn-partially-globbed-names.sh
@@ -48,8 +48,8 @@ test_expect_success 'test refspec prefixed globbing' '
git config --add svn-remote.svn.tags\
"tags/t_*/src/a:refs/remotes/tags/t_*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.end &&
+ git log --pretty=oneline refs/remotes/tags/t_end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/t_end~1)" = \
"$(git rev-parse refs/remotes/branches/b_start)" &&
@@ -78,14 +78,16 @@ test_expect_success 'test left-hand-side only prefixed globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/t_end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/b_start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/t_end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/b_start >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/two/branches/b_start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/t_end~3) = \
$(git rev-parse refs/remotes/two/branches/b_start) &&
- git log --pretty=oneline refs/remotes/two/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/t_end >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 >output.two &&
test_cmp expect.two output.two
'
@@ -118,14 +120,16 @@ test_expect_success 'test prefixed globs match just prefix' '
svn_cmd up
) &&
git svn fetch three &&
- test $(git rev-list refs/remotes/three/branches/b_ | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/three/tags/t_ | wc -l) -eq 3 &&
+ git rev-list refs/remotes/three/branches/b_ >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/three/tags/t_ >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/three/branches/b_~1) = \
$(git rev-parse refs/remotes/three/trunk) &&
test $(git rev-parse refs/remotes/three/tags/t_~1) = \
$(git rev-parse refs/remotes/three/branches/b_) &&
- git log --pretty=oneline refs/remotes/three/tags/t_ | \
- sed -e "s/^.\{41\}//" >output.three &&
+ git log --pretty=oneline refs/remotes/three/tags/t_ >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 >output.three &&
test_cmp expect.three output.three
'
@@ -186,14 +190,16 @@ test_expect_success 'test globbing in the middle of the word' '
svn_cmd up
) &&
git svn fetch five &&
- test $(git rev-list refs/remotes/five/branches/abcde | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/five/tags/fghij | wc -l) -eq 3 &&
+ git rev-list refs/remotes/five/branches/abcde >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/five/tags/fghij >actual2 &&
+ test_line_count = 3 actual2 &&
test $(git rev-parse refs/remotes/five/branches/abcde~1) = \
$(git rev-parse refs/remotes/five/trunk) &&
test $(git rev-parse refs/remotes/five/tags/fghij~1) = \
$(git rev-parse refs/remotes/five/branches/abcde) &&
- git log --pretty=oneline refs/remotes/five/tags/fghij | \
- sed -e "s/^.\{41\}//" >output.five &&
+ git log --pretty=oneline refs/remotes/five/tags/fghij >actual3 &&
+ sed -e "s/^.\{41\}//" actual3 >output.five &&
test_cmp expect.five output.five
'
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 866ddf605..116b4e5f8 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -43,20 +43,20 @@ test_expect_success 'fast-export | fast-import' '
MUSS=$(git rev-parse --verify muss) &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --all |
+ git fast-export --all >actual &&
(cd new &&
git fast-import &&
test $MASTER = $(git rev-parse --verify refs/heads/master) &&
test $REIN = $(git rev-parse --verify refs/tags/rein) &&
test $WER = $(git rev-parse --verify refs/heads/wer) &&
- test $MUSS = $(git rev-parse --verify refs/tags/muss))
+ test $MUSS = $(git rev-parse --verify refs/tags/muss)) <actual
'
test_expect_success 'fast-export master~2..master' '
- git fast-export master~2..master |
- sed "s/master/partial/" |
+ git fast-export master~2..master >actual &&
+ sed "s/master/partial/" actual |
(cd new &&
git fast-import &&
test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
@@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
test_tick &&
echo rosten >file &&
git commit -s -m den file &&
- git fast-export wer^..wer |
- sed "s/wer/i18n/" |
- (cd new &&
- git fast-import &&
- git cat-file commit i18n | grep "Áéí óú")
+ git fast-export wer^..wer >actual &&
+ sed "s/wer/i18n/" actual |
+ (cd new &&
+ git fast-import &&
+ git cat-file commit i18n >actual &&
+ grep "Áéí óú" actual)
'
test_expect_success 'import/export-marks' '
@@ -87,18 +88,18 @@ test_expect_success 'import/export-marks' '
git fast-export --export-marks=tmp-marks HEAD &&
test -s tmp-marks &&
test_line_count = 3 tmp-marks &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual &&
test $(
- git fast-export --import-marks=tmp-marks\
- --export-marks=tmp-marks HEAD |
- grep ^commit |
+ grep ^commit actual |
wc -l) \
-eq 0 &&
echo change > file &&
git commit -m "last commit" file &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual2 &&
test $(
- git fast-export --import-marks=tmp-marks \
- --export-marks=tmp-marks HEAD |
- grep ^commit\ |
+ grep ^commit\ actual2 |
wc -l) \
-eq 1 &&
test_line_count = 4 tmp-marks
@@ -184,7 +185,7 @@ test_expect_success 'submodule fast-export | fast-import' '
rm -rf new &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --signed-tags=strip --all |
+ git fast-export --signed-tags=strip --all >actual &&
(cd new &&
git fast-import &&
test "$SUBENT1" = "$(git ls-tree refs/heads/master^ sub)" &&
@@ -192,7 +193,7 @@ test_expect_success 'submodule fast-export | fast-import' '
git checkout master &&
git submodule init &&
git submodule update &&
- cmp sub/file ../sub/file)
+ cmp sub/file ../sub/file) <actual
'
@@ -361,18 +362,20 @@ test_expect_failure 'no exact-ref revisions included' '
)
'
-test_expect_success 'path limiting with import-marks does not lose unmodified files' '
+test_expect_success 'path limiting with import-marks does not lose unmodified files' '
git checkout -b simple marks~2 &&
git fast-export --export-marks=marks simple -- file > /dev/null &&
echo more content >> file &&
test_tick &&
git commit -mnext file &&
- git fast-export --import-marks=marks simple -- file file0 | grep file0
+ git fast-export --import-marks=marks simple -- file file0 >actual &&
+ grep file0 actual
'
-test_expect_success 'full-tree re-shows unmodified files' '
+test_expect_success 'full-tree re-shows unmodified files' '
git checkout -f simple &&
- test $(git fast-export --full-tree simple | grep -c file0) -eq 3
+ git fast-export --full-tree simple >actual &&
+ test $(grep -c file0 actual) -eq 3
'
test_expect_success 'set-up a few more tags for tag export tests' '
@@ -500,13 +503,13 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
git fast-export --import-marks=tmp-marks \
--export-marks=tmp-marks master > /dev/null &&
git fast-export --import-marks=tmp-marks \
- --export-marks=tmp-marks master > actual &&
+ --export-marks=tmp-marks master >actual &&
test_cmp expected actual
'
test_expect_success 'use refspec' '
- git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
- grep "^commit " | sort | uniq > actual &&
+ git fast-export --refspec refs/heads/master:refs/heads/foobar master >actual2 &&
+ grep "^commit " actual2 | sort | uniq >actual &&
echo "commit refs/heads/foobar" > expected &&
test_cmp expected actual
'
@@ -534,7 +537,8 @@ test_expect_success 'when using -C, do not declare copy when source of copy is a
git -C src commit -m 2nd_commit &&
test_create_repo dst &&
- git -C src fast-export --all -C | git -C dst fast-import &&
+ git -C src fast-export --all -C >actual &&
+ git -C dst fast-import <actual &&
git -C src show >expected &&
git -C dst show >actual &&
test_cmp expected actual
--
2.16.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH v3] test: avoid pipes in git related commands for test
2018-03-21 15:23 ` [GSoC][PATCH v3] test: avoid pipes in git related commands for test Pratik Karki
@ 2018-03-21 18:11 ` Junio C Hamano
2018-03-21 18:45 ` Eric Sunshine
2018-03-21 18:58 ` Eric Sunshine
0 siblings, 2 replies; 16+ messages in thread
From: Junio C Hamano @ 2018-03-21 18:11 UTC (permalink / raw)
To: Pratik Karki; +Cc: Git List, Eric Sunshine
Pratik Karki <predatoramigo@gmail.com> writes:
> Thank you Eric, for the review. This is follow on patch[1].
>
> The changes in patch increased from v1 to v2 because I
> got excited to work in Git codebase and I tried to
> fix the exisiting problems as much as possible.
> Hence, the large number of changes.
Eric understands why the scope was increased between the two; he
explained why it is not a good idea to increase the scope in the
middle, and I tend to agree with his reasoning. The reason why the
scope was increased does not matter.
>>> @@ -120,18 +122,20 @@ test_expect_success 'test another branch' '
>>> git config --add svn-remote.four.url "$svnrepo" &&
>>> git config --add svn-remote.four.fetch trunk:refs/remotes/four/trunk &&
>>> git config --add svn-remote.four.branches \
>>> - "branches/*/*:refs/remotes/four/branches/*/*" &&
>>> + "branches/*/*:refs/remotes/four/branches/*/*" &&
>>> git config --add svn-remote.four.tags \
>>> - "tags/*:refs/remotes/four/tags/*" &&
>>> + "tags/*:refs/remotes/four/tags/*" &&
>
>>I guess you sneaked in a whitespace change here which is unrelated to
>>the stated purpose of this patch, thus acted as a speed bump during
>>review....
>>Therefore, you should omit this change.
>
> I used tabify in Emacs and it must have messed up the whitespace
> change.
Then don't. Make sure the lines _you_ change are indented and
formatted correctly. Do not touch near-by (or far-away for that
matter) lines that you do not have to touch only to change the
formatting.
> I read SubmittingPatches guideline[2] for git where it
> is said that whitespace check must be done and git community is
> picky about it and 'git diff --check' must be done before commit.
Yes.
> If I change this change back to original the 'git diff --check'
> reports whitespace identation with space.
I do not think 'diff --check' would. Save the patch you sent to a
file, edit it so that these two lines Eric pointed out are not
changed, and then apply it with "git apply --whitespace=nowarn".
Then ask "git diff --check"---it should not complain about an
existing whitespace glitch that you did not introduce.
> So, isn't this
> supposedly a fix?
Unless it is a "here is a patch to reindent and fix whitespaces"
patch that does nothing else, it is an unwelcome noise that
distracts reviewers from the real changes.
> ------------------------------------- >8----------------------------------------
This is not wrong per se, but just a
-- >8 --
is sufficient ;-)
>
> Avoid using pipes downstream of Git commands since the exit
> codes of commands upstream of pipes get swallowed, thus potentially hiding
> failure of those commands. Instead, capture Git command output to a file and
> apply the downstream command(s) to that file.
Please do not indent the body of the log message by one space.
> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index 9c68b9925..707208284 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -311,9 +311,9 @@ test_expect_success 'unpacking with --strict' '
> rm -f .git/index &&
> tail -n 10 LIST | git update-index --index-info &&
> ST=$(git write-tree) &&
> - PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
> - git pack-objects test-5 ) &&
> - PACK6=$( (
> + git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
> + PACK5=$( git pack-objects test-5 <actual ) &&
> + PACK6=$((
I thought that Eric already pointed out and explained why this
change to PACK6 is wrong in the previous round?
> diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
> index 22b6e5ee7..a4225c9f6 100755
> --- a/t/t9111-git-svn-use-svnsync-props.sh
> +++ b/t/t9111-git-svn-use-svnsync-props.sh
> @@ -20,32 +20,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
>
> bar_url=http://mayonaise/svnrepo/bar
> test_expect_success 'verify metadata for /bar' "
> - git cat-file commit refs/remotes/bar | \
> - grep '^git-svn-id: $bar_url@12 $uuid$' &&
> - git cat-file commit refs/remotes/bar~1 | \
> - grep '^git-svn-id: $bar_url@11 $uuid$' &&
> - git cat-file commit refs/remotes/bar~2 | \
> - grep '^git-svn-id: $bar_url@10 $uuid$' &&
> - git cat-file commit refs/remotes/bar~3 | \
> - grep '^git-svn-id: $bar_url@9 $uuid$' &&
> - git cat-file commit refs/remotes/bar~4 | \
> - grep '^git-svn-id: $bar_url@6 $uuid$' &&
> - git cat-file commit refs/remotes/bar~5 | \
> - grep '^git-svn-id: $bar_url@1 $uuid$'
> + git cat-file commit refs/remotes/bar >actual &&
> + grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
> + git cat-file commit refs/remotes/bar~1 >actual1 &&
> + grep '^git-svn-id: $bar_url@11 $uuid$' actual1 &&
> + git cat-file commit refs/remotes/bar~2 >actual2 &&
> + grep '^git-svn-id: $bar_url@10 $uuid$' actual2 &&
> + git cat-file commit refs/remotes/bar~3 >actual3 &&
> + grep '^git-svn-id: $bar_url@9 $uuid$' actual3 &&
> + git cat-file commit refs/remotes/bar~4 >actual4 &&
> + grep '^git-svn-id: $bar_url@6 $uuid$' actual4 &&
> + git cat-file commit refs/remotes/bar~5 >actual5 &&
> + grep '^git-svn-id: $bar_url@1 $uuid$' actual5
> "
I also thought that Eric already pointed out that the above is not a
good idea because it forces readers to wonder if "actual[1-5]" need
to exist together with "actual" at the same time in the previous
round?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH v3] test: avoid pipes in git related commands for test
2018-03-21 18:11 ` Junio C Hamano
@ 2018-03-21 18:45 ` Eric Sunshine
2018-03-21 18:58 ` Eric Sunshine
1 sibling, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2018-03-21 18:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pratik Karki, Git List
On Wed, Mar 21, 2018 at 2:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pratik Karki <predatoramigo@gmail.com> writes:
>> The changes in patch increased from v1 to v2 because I
>> got excited to work in Git codebase and I tried to
>> fix the exisiting problems as much as possible.
>> Hence, the large number of changes.
>
> Eric understands why the scope was increased between the two; he
> explained why it is not a good idea to increase the scope in the
> middle, and I tend to agree with his reasoning. The reason why the
> scope was increased does not matter.
Thanks, Junio. I had just started writing a review of v3 when your
review arrived, and you covered every point I was going to make, thus
saved me the effort. I agree with everything in your review.
One additional comment, Pratik, is that this patch seems to be based
upon a slightly old version of the Git source code, thus does not
apply cleanly to present-day 'master'. Before re-rolling, update to
the latest Git and rebase your patch atop it.
>> - PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
>> - git pack-objects test-5 ) &&
>> - PACK6=$( (
>> + git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
>> + PACK5=$( git pack-objects test-5 <actual ) &&
>> + PACK6=$((
>
> I thought that Eric already pointed out and explained why this
> change to PACK6 is wrong in the previous round?
I probably should have been more explicit by naming PACK6 directly.
Comparing v3 against v2, I see that Pratik probably misunderstood my
comment, thinking that I was talking about losing the whitespace
inside PACK5=$(...); v2 dropped that whitespace and v3 restored it.
Pratik, dropping the unnecessary whitespace inside PACK5=$(...) is
fine (no complaint about that), but changing PACK6=$( (...) ) to
PACK6=$((...)) is outright incorrect as explained in [1].
[1]: https://public-inbox.org/git/CAPig+cTKkp6kpFcJfVV8W1ejCrCWQH33mHtgFUn+MpMgw5i1pA@mail.gmail.com/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH v3] test: avoid pipes in git related commands for test
2018-03-21 18:11 ` Junio C Hamano
2018-03-21 18:45 ` Eric Sunshine
@ 2018-03-21 18:58 ` Eric Sunshine
2018-03-23 15:01 ` [GSoC][PATCH v4] " Pratik Karki
1 sibling, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2018-03-21 18:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pratik Karki, Git List
On Wed, Mar 21, 2018 at 2:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pratik Karki <predatoramigo@gmail.com> writes:
>> Avoid using pipes downstream of Git commands since the exit
>> codes of commands upstream of pipes get swallowed, thus potentially hiding
>> failure of those commands. Instead, capture Git command output to a file and
>> apply the downstream command(s) to that file.
>
> Please do not indent the body of the log message by one space.
One other issue I forgot to mention is that the commit message in v3
started getting too wide again[1]; it was fine in v2. Pratik, try to
keep the commit message wrapped to about 70-72 characters or so.
[1]: https://public-inbox.org/git/CAPig+cRPzyw525ODC4=-E7w=zbpbhVN2eqxSYDSLij5wfW8S_A@mail.gmail.com/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [GSoC][PATCH v4] test: avoid pipes in git related commands for test
2018-03-21 18:58 ` Eric Sunshine
@ 2018-03-23 15:01 ` Pratik Karki
2018-03-25 8:37 ` Eric Sunshine
0 siblings, 1 reply; 16+ messages in thread
From: Pratik Karki @ 2018-03-23 15:01 UTC (permalink / raw)
To: Git List; +Cc: Pratik Karki, Eric Sunshine, Junio C Hamano
Thank you Eric and Junio for the review.
I hope this follow-on patch[1] is ready for merge.
[1]: https://public-inbox.org/git/20180321152356.10754-1-predatoramigo@gmail.com/
-- >8 --
Avoid using pipes downstream of Git commands since the exit codes
of commands upstream of pipes get swallowed, thus potentially
hiding failure of those commands. Instead, capture Git command
output to a file and apply the downstream command(s) to that file.
Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
t/t5300-pack-object.sh | 8 ++---
t/t5510-fetch.sh | 8 ++---
t/t7001-mv.sh | 22 +++++++-------
t/t7003-filter-branch.sh | 9 ++++--
t/t9104-git-svn-follow-parent.sh | 16 +++++-----
t/t9108-git-svn-glob.sh | 14 +++++----
t/t9109-git-svn-multi-glob.sh | 24 ++++++++-------
t/t9110-git-svn-use-svm-props.sh | 40 ++++++++++++-------------
t/t9111-git-svn-use-svnsync-props.sh | 36 +++++++++++-----------
t/t9114-git-svn-dcommit-merge.sh | 10 ++++---
t/t9130-git-svn-authors-file.sh | 28 +++++++++--------
t/t9138-git-svn-authors-prog.sh | 31 +++++++++----------
t/t9153-git-svn-rewrite-uuid.sh | 8 ++---
t/t9168-git-svn-partially-globbed-names.sh | 34 ++++++++++++---------
t/t9350-fast-export.sh | 48 ++++++++++++++++--------------
15 files changed, 180 insertions(+), 156 deletions(-)
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 9c68b9925..156beb2d5 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -311,8 +311,8 @@ test_expect_success 'unpacking with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$( git pack-objects test-5 <actual ) &&
PACK6=$( (
echo "$LIST"
echo "$LI"
@@ -358,8 +358,8 @@ test_expect_success 'index-pack with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$( git pack-objects test-5 <actual ) &&
PACK6=$( (
echo "$LIST"
echo "$LI"
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index da9ac0055..f6d28ed7f 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -840,8 +840,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
test_commit looooooooooooong-tag &&
(
cd full-output &&
- git -c fetch.output=full fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=full fetch origin >actual2 2>&1 &&
+ grep -e "->" actual2 | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/master
@@ -855,8 +855,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
test_commit extraaa &&
(
cd compact &&
- git -c fetch.output=compact fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=compact fetch origin >actual2 2>&1 &&
+ grep -e "->" actual2 | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/*
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index d4e6485a2..e96cbdb10 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -21,8 +21,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path1/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path1/COPYING" actual'
test_expect_success \
'moving the file back into subdirectory' \
@@ -35,8 +35,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path1/COPYING..*path0/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path1/COPYING..*path0/COPYING" actual'
test_expect_success \
'mv --dry-run does not move file' \
@@ -122,10 +122,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/README..*path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
+ grep "^R100..*path0/README..*path2/README" actual'
test_expect_success \
'succeed when source is a prefix of destination' \
@@ -141,10 +140,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/COPYING..*path1/path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/README..*path1/path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path2/COPYING..*path1/path2/COPYING" actual &&
+ grep "^R100..*path2/README..*path1/path2/README" actual'
test_expect_success \
'do not move directory over existing directory' \
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..6a28b6cce 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -187,7 +187,8 @@ test_expect_success 'author information is preserved' '
test \$GIT_COMMIT != $(git rev-parse master) || \
echo Hallo" \
preserved-author) &&
- test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l)
+ git rev-list --author="B V Uips" preserved-author >actual &&
+ test_line_count = 1 actual
'
test_expect_success "remove a certain author's commits" '
@@ -205,7 +206,8 @@ test_expect_success "remove a certain author's commits" '
cnt1=$(git rev-list master | wc -l) &&
cnt2=$(git rev-list removed-author | wc -l) &&
test $cnt1 -eq $(($cnt2 + 1)) &&
- test 0 = $(git rev-list --author="B V Uips" removed-author | wc -l)
+ git rev-list --author="B V Uips" removed-author >actual &&
+ test_line_count = 0 actual
'
test_expect_success 'barf on invalid name' '
@@ -258,7 +260,8 @@ test_expect_success 'Subdirectory filter with disappearing trees' '
git commit -m "Re-adding foo" &&
git filter-branch -f --subdirectory-filter foo &&
- test $(git rev-list master | wc -l) = 3
+ git rev-list master >actual &&
+ test_line_count = 3 actual
'
test_expect_success 'Tag name filtering retains tag message' '
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index cd480edf1..a735fa371 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -33,8 +33,8 @@ test_expect_success 'init and fetch a moved directory' '
git svn fetch -i thunk &&
test "$(git rev-parse --verify refs/remotes/thunk@2)" \
= "$(git rev-parse --verify refs/remotes/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/thunk:readme |\
- sed -n -e "3p")" = goodbye &&
+ git cat-file blob refs/remotes/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye &&
test -z "$(git config --get svn-remote.svn.fetch \
"^trunk:refs/remotes/thunk@2$")"
'
@@ -48,8 +48,8 @@ test_expect_success 'init and fetch from one svn-remote' '
git svn fetch -i svn/thunk &&
test "$(git rev-parse --verify refs/remotes/svn/trunk)" \
= "$(git rev-parse --verify refs/remotes/svn/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/svn/thunk:readme |\
- sed -n -e "3p")" = goodbye
+ git cat-file blob refs/remotes/svn/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye
'
test_expect_success 'follow deleted parent' '
@@ -107,7 +107,8 @@ test_expect_success 'follow deleted directory' '
git svn init --minimize-url -i glob "$svnrepo"/glob &&
git svn fetch -i glob &&
test "$(git cat-file blob refs/remotes/glob:blob/bye)" = hi &&
- test "$(git ls-tree refs/remotes/glob | wc -l )" -eq 1
+ git ls-tree refs/remotes/glob >actual &&
+ test_line_count = 1 actual
'
# ref: r9270 of the Subversion repository: (http://svn.collab.net/repos/svn)
@@ -204,8 +205,9 @@ test_expect_success "follow-parent is atomic" '
test_expect_success "track multi-parent paths" '
svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
git svn multi-fetch &&
- test $(git cat-file commit refs/remotes/glob | \
- grep "^parent " | wc -l) -eq 2
+ git cat-file commit refs/remotes/glob >actual &&
+ grep "^parent " actual >actual2 &&
+ test_line_count = 2 actual2
'
test_expect_success "multi-fetch continues to work" "
diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
index a94286c8e..6990f6436 100755
--- a/t/t9108-git-svn-glob.sh
+++ b/t/t9108-git-svn-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
index 8d99e848d..c1e7542a3 100755
--- a/t/t9109-git-svn-multi-glob.sh
+++ b/t/t9109-git-svn-multi-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/v1/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/v1/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/v1/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/v1/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/v1/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
cat > expect.four <<EOF
@@ -124,14 +126,16 @@ test_expect_success 'test another branch' '
git config --add svn-remote.four.tags \
"tags/*:refs/remotes/four/tags/*" &&
git svn fetch four &&
- test $(git rev-list refs/remotes/four/tags/next | wc -l) -eq 5 &&
- test $(git rev-list refs/remotes/four/branches/v2/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/four/tags/next >actual &&
+ test_line_count = 5 actual &&
+ git rev-list refs/remotes/four/branches/v2/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/four/branches/v2/start~2) = \
$(git rev-parse refs/remotes/four/trunk) &&
test $(git rev-parse refs/remotes/four/tags/next~2) = \
$(git rev-parse refs/remotes/four/branches/v2/start) &&
- git log --pretty=oneline refs/remotes/four/tags/next | \
- sed -e "s/^.\{41\}//" > output.four &&
+ git log --pretty=oneline refs/remotes/four/tags/next >actual &&
+ sed -e "s/^.\{41\}//" actual >output.four &&
test_cmp expect.four output.four
'
diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
index dde0a3c22..ad37d980c 100755
--- a/t/t9110-git-svn-use-svm-props.sh
+++ b/t/t9110-git-svn-use-svm-props.sh
@@ -21,37 +21,37 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_expect_success 'find commit based on SVN revision number' "
- git svn find-rev r12 |
- grep $(git rev-parse HEAD)
+ git svn find-rev r12 >actual &&
+ grep $(git rev-parse HEAD) actual
"
test_expect_success 'empty rebase' "
diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
index 22b6e5ee7..6c9307355 100755
--- a/t/t9111-git-svn-use-svnsync-props.sh
+++ b/t/t9111-git-svn-use-svnsync-props.sh
@@ -20,32 +20,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_done
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 50bca62de..32317d6bc 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -68,7 +68,8 @@ test_debug 'gitk --all & sleep 1'
test_expect_success 'verify pre-merge ancestry' "
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'git svn dcommit merges' "
@@ -82,12 +83,13 @@ test_expect_success 'verify post-merge ancestry' "
x\$(git rev-parse --verify refs/remotes/origin/trunk) &&
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'verify merge commit message' "
- git rev-list --pretty=raw -1 refs/heads/svn | \
- grep \" Merge branch 'merge' into svn\"
+ git rev-list --pretty=raw -1 refs/heads/svn >actual &&
+ grep \" Merge branch 'merge' into svn\" actual
"
test_done
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index 41264818c..7752a1fae 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -26,11 +26,12 @@ test_expect_success 'start import with incomplete authors file' '
test_expect_success 'imported 2 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 2 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author BBBBBBB BBBBBBB <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author AAAAAAA AAAAAAA <aa@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 2 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author BBBBBBB BBBBBBB <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author AAAAAAA AAAAAAA <aa@example\.com> " actual
)
'
@@ -43,11 +44,12 @@ test_expect_success 'continues to import once authors have been added' '
(
cd x
git svn fetch --authors-file=../svn-authors &&
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 4 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author DDDDDDD DDDDDDD <dd@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author CCCCCCC CCCCCCC <cc@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 4 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author DDDDDDD DDDDDDD <dd@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author CCCCCCC CCCCCCC <cc@example\.com> " actual
)
'
@@ -102,8 +104,10 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' '
test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
git svn clone "$svnrepo" gitconfig.clone &&
cd gitconfig.clone &&
- nr_ex=$(git log | grep "^Author:.*example.com" | wc -l) &&
- nr_rev=$(git rev-list HEAD | wc -l) &&
+ git log >actual &&
+ nr_ex=$(grep "^Author:.*example.com" actual | wc -l) &&
+ git rev-list HEAD >actual &&
+ nr_rev=$(wc -l <actual) &&
test $nr_rev -eq $nr_ex
)
'
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 7d7e9d46b..48109f949 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -37,31 +37,32 @@ test_expect_success 'import authors with prog and file' '
test_expect_success 'imported 6 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 6
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 6 actual
)
'
test_expect_success 'authors-prog ran correctly' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author ee-foo <ee-foo@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~2 | \
- grep "^author dd <dd@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~3 | \
- grep "^author cc <cc@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~4 | \
- grep "^author bb <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~5 | \
- grep "^author aa <aa@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author ee-foo <ee-foo@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~2 >actual &&
+ grep "^author dd <dd@sub\.example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~3 >actual &&
+ grep "^author cc <cc@sub\.example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~4 >actual &&
+ grep "^author bb <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~5 >actual &&
+ grep "^author aa <aa@example\.com> " actual
)
'
test_expect_success 'authors-file overrode authors-prog' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> " actual
)
'
@@ -73,8 +74,8 @@ test_expect_success 'authors-prog handled special characters in username' '
(
cd x &&
git svn --authors-prog=../svn-authors-prog fetch &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn |
- grep "^author xyz; touch evil <xyz; touch evil@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author xyz; touch evil <xyz; touch evil@example\.com> " actual &&
! test -f evil
)
'
diff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh
index 372ef1568..8cb2b5c69 100755
--- a/t/t9153-git-svn-rewrite-uuid.sh
+++ b/t/t9153-git-svn-rewrite-uuid.sh
@@ -16,10 +16,10 @@ test_expect_success 'load svn repo' "
"
test_expect_success 'verify uuid' "
- git cat-file commit refs/remotes/git-svn~0 | \
- grep '^git-svn-id: .*@2 $uuid$' &&
- git cat-file commit refs/remotes/git-svn~1 | \
- grep '^git-svn-id: .*@1 $uuid$'
+ git cat-file commit refs/remotes/git-svn~0 >actual &&
+ grep '^git-svn-id: .*@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/git-svn~1 >actual &&
+ grep '^git-svn-id: .*@1 $uuid$' actual
"
test_done
diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
index 8b22f2272..bdf6e8499 100755
--- a/t/t9168-git-svn-partially-globbed-names.sh
+++ b/t/t9168-git-svn-partially-globbed-names.sh
@@ -48,8 +48,8 @@ test_expect_success 'test refspec prefixed globbing' '
git config --add svn-remote.svn.tags\
"tags/t_*/src/a:refs/remotes/tags/t_*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.end &&
+ git log --pretty=oneline refs/remotes/tags/t_end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/t_end~1)" = \
"$(git rev-parse refs/remotes/branches/b_start)" &&
@@ -78,14 +78,16 @@ test_expect_success 'test left-hand-side only prefixed globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/t_end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/b_start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/t_end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/b_start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/b_start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/t_end~3) = \
$(git rev-parse refs/remotes/two/branches/b_start) &&
- git log --pretty=oneline refs/remotes/two/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/t_end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
@@ -118,14 +120,16 @@ test_expect_success 'test prefixed globs match just prefix' '
svn_cmd up
) &&
git svn fetch three &&
- test $(git rev-list refs/remotes/three/branches/b_ | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/three/tags/t_ | wc -l) -eq 3 &&
+ git rev-list refs/remotes/three/branches/b_ >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/three/tags/t_ >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/three/branches/b_~1) = \
$(git rev-parse refs/remotes/three/trunk) &&
test $(git rev-parse refs/remotes/three/tags/t_~1) = \
$(git rev-parse refs/remotes/three/branches/b_) &&
- git log --pretty=oneline refs/remotes/three/tags/t_ | \
- sed -e "s/^.\{41\}//" >output.three &&
+ git log --pretty=oneline refs/remotes/three/tags/t_ >actual &&
+ sed -e "s/^.\{41\}//" actual >output.three &&
test_cmp expect.three output.three
'
@@ -186,14 +190,16 @@ test_expect_success 'test globbing in the middle of the word' '
svn_cmd up
) &&
git svn fetch five &&
- test $(git rev-list refs/remotes/five/branches/abcde | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/five/tags/fghij | wc -l) -eq 3 &&
+ git rev-list refs/remotes/five/branches/abcde >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/five/tags/fghij >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/five/branches/abcde~1) = \
$(git rev-parse refs/remotes/five/trunk) &&
test $(git rev-parse refs/remotes/five/tags/fghij~1) = \
$(git rev-parse refs/remotes/five/branches/abcde) &&
- git log --pretty=oneline refs/remotes/five/tags/fghij | \
- sed -e "s/^.\{41\}//" >output.five &&
+ git log --pretty=oneline refs/remotes/five/tags/fghij >actual &&
+ sed -e "s/^.\{41\}//" actual >output.five &&
test_cmp expect.five output.five
'
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 866ddf605..b5f9ef6ff 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -43,20 +43,20 @@ test_expect_success 'fast-export | fast-import' '
MUSS=$(git rev-parse --verify muss) &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --all |
+ git fast-export --all >actual &&
(cd new &&
git fast-import &&
test $MASTER = $(git rev-parse --verify refs/heads/master) &&
test $REIN = $(git rev-parse --verify refs/tags/rein) &&
test $WER = $(git rev-parse --verify refs/heads/wer) &&
- test $MUSS = $(git rev-parse --verify refs/tags/muss))
+ test $MUSS = $(git rev-parse --verify refs/tags/muss)) <actual
'
test_expect_success 'fast-export master~2..master' '
- git fast-export master~2..master |
- sed "s/master/partial/" |
+ git fast-export master~2..master >actual &&
+ sed "s/master/partial/" actual |
(cd new &&
git fast-import &&
test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
@@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
test_tick &&
echo rosten >file &&
git commit -s -m den file &&
- git fast-export wer^..wer |
- sed "s/wer/i18n/" |
+ git fast-export wer^..wer >actual &&
+ sed "s/wer/i18n/" actual |
(cd new &&
git fast-import &&
- git cat-file commit i18n | grep "Áéí óú")
+ git cat-file commit i18n >actual &&
+ grep "Áéí óú" actual)
'
test_expect_success 'import/export-marks' '
@@ -87,18 +88,16 @@ test_expect_success 'import/export-marks' '
git fast-export --export-marks=tmp-marks HEAD &&
test -s tmp-marks &&
test_line_count = 3 tmp-marks &&
- test $(
- git fast-export --import-marks=tmp-marks\
- --export-marks=tmp-marks HEAD |
- grep ^commit |
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual &&
+ test $(grep ^commit actual |
wc -l) \
-eq 0 &&
echo change > file &&
git commit -m "last commit" file &&
- test $(
- git fast-export --import-marks=tmp-marks \
- --export-marks=tmp-marks HEAD |
- grep ^commit\ |
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual &&
+ test $(grep ^commit\ actual |
wc -l) \
-eq 1 &&
test_line_count = 4 tmp-marks
@@ -184,7 +183,7 @@ test_expect_success 'submodule fast-export | fast-import' '
rm -rf new &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --signed-tags=strip --all |
+ git fast-export --signed-tags=strip --all >actual &&
(cd new &&
git fast-import &&
test "$SUBENT1" = "$(git ls-tree refs/heads/master^ sub)" &&
@@ -192,7 +191,7 @@ test_expect_success 'submodule fast-export | fast-import' '
git checkout master &&
git submodule init &&
git submodule update &&
- cmp sub/file ../sub/file)
+ cmp sub/file ../sub/file) <actual
'
@@ -367,12 +366,14 @@ test_expect_success 'path limiting with import-marks does not lose unmodified fi
echo more content >> file &&
test_tick &&
git commit -mnext file &&
- git fast-export --import-marks=marks simple -- file file0 | grep file0
+ git fast-export --import-marks=marks simple -- file file0 >actual &&
+ grep file0 actual
'
test_expect_success 'full-tree re-shows unmodified files' '
git checkout -f simple &&
- test $(git fast-export --full-tree simple | grep -c file0) -eq 3
+ git fast-export --full-tree simple >actual &&
+ test $(grep -c file0 actual) -eq 3
'
test_expect_success 'set-up a few more tags for tag export tests' '
@@ -500,13 +501,13 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
git fast-export --import-marks=tmp-marks \
--export-marks=tmp-marks master > /dev/null &&
git fast-export --import-marks=tmp-marks \
- --export-marks=tmp-marks master > actual &&
+ --export-marks=tmp-marks master >actual &&
test_cmp expected actual
'
test_expect_success 'use refspec' '
- git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
- grep "^commit " | sort | uniq > actual &&
+ git fast-export --refspec refs/heads/master:refs/heads/foobar master >actual2 &&
+ grep "^commit " actual2 | sort | uniq >actual &&
echo "commit refs/heads/foobar" > expected &&
test_cmp expected actual
'
@@ -534,7 +535,8 @@ test_expect_success 'when using -C, do not declare copy when source of copy is a
git -C src commit -m 2nd_commit &&
test_create_repo dst &&
- git -C src fast-export --all -C | git -C dst fast-import &&
+ git -C src fast-export --all -C >actual &&
+ git -C dst fast-import <actual &&
git -C src show >expected &&
git -C dst show >actual &&
test_cmp expected actual
--
2.16.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH v4] test: avoid pipes in git related commands for test
2018-03-23 15:01 ` [GSoC][PATCH v4] " Pratik Karki
@ 2018-03-25 8:37 ` Eric Sunshine
2018-03-27 17:31 ` [GSoC][PATCH v5] " Pratik Karki
0 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2018-03-25 8:37 UTC (permalink / raw)
To: Pratik Karki; +Cc: Git List, Junio C Hamano
On Fri, Mar 23, 2018 at 11:01 AM, Pratik Karki <predatoramigo@gmail.com> wrote:
> I hope this follow-on patch[1] is ready for merge.
This iteration appears to address review comments from the last few
rounds, however, see below for a few new ones...
> Avoid using pipes downstream of Git commands since the exit codes
> of commands upstream of pipes get swallowed, thus potentially
> hiding failure of those commands. Instead, capture Git command
> output to a file and apply the downstream command(s) to that file.
>
> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
> ---
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> @@ -840,8 +840,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
> test_commit looooooooooooong-tag &&
> (
> cd full-output &&
> - git -c fetch.output=full fetch origin 2>&1 | \
> - grep -e "->" | cut -c 22- >../actual
> + git -c fetch.output=full fetch origin >actual2 2>&1 &&
> + grep -e "->" actual2 | cut -c 22- >../actual
The file "actual2" is clearly distinct from the file "../actual", so
inventing a name ("actual2") isn't particularly helping; you could
just as easily also name it "actual" without hurting comprehension.
(Not necessarily worth a re-roll.)
> ) &&
> @@ -855,8 +855,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
> test_commit extraaa &&
> (
> cd compact &&
> - git -c fetch.output=compact fetch origin 2>&1 | \
> - grep -e "->" | cut -c 22- >../actual
> + git -c fetch.output=compact fetch origin >actual2 2>&1 &&
> + grep -e "->" actual2 | cut -c 22- >../actual
Same comment.
> ) &&
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> @@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
> git commit -s -m den file &&
> - git fast-export wer^..wer |
> - sed "s/wer/i18n/" |
> + git fast-export wer^..wer >actual &&
> + sed "s/wer/i18n/" actual |
> (cd new &&
> git fast-import &&
> - git cat-file commit i18n | grep "Áéí óú")
> + git cat-file commit i18n >actual &&
> + grep "Áéí óú" actual)
It was a bit surprising to see a new "actual" file created inside the
subshell even as 'sed' is processing a file named "actual" outside the
subshell, and, as a reader, I was concerned about bad interaction
between the operations. However, the file in the subshell is really
"new/actual", thus is distinct from the other "actual", so it's okay.
This is one of those cases, however, in which it might make sense to
give the files different names to make the code easier to grok, so
future readers don't stumble over this as well. For instance, the
outer file could be named "iso8859-1.fi" (or something), and the file
in the subshell can remain "actual". Not itself worth a re-roll, but
probably a good idea.
(This differs in couple ways from my comment above about t5510 tests
naming files "actual2" and "../actual". In that case, it was quite
clear that, within the cd'd subshell, file "../actual" was distinct
from the file created within the cd'd directory, so no confusion.
Moreover, those files were not being accessed at the same time,
whereas in this t9350 test, the 'sed' is reading from the a file at
the same time as 'git cat-file' is outputting to a similarly named
file, which is potentially confusing and requires extra brain cycles
to sort out.)
> '
> @@ -87,18 +88,16 @@ test_expect_success 'import/export-marks' '
> test_line_count = 3 tmp-marks &&
> - test $(
> - git fast-export --import-marks=tmp-marks\
> - --export-marks=tmp-marks HEAD |
> - grep ^commit |
> + git fast-export --import-marks=tmp-marks \
> + --export-marks=tmp-marks HEAD >actual &&
> + test $(grep ^commit actual |
> wc -l) \
> -eq 0 &&
Since the git-fast-export invocation has been pulled out of the
$(...), the entire 'test' expression is now short enough to fit easily
on one line. Making such a change would improve readability
considering how hard it is to read split over three lines like that
(with inconsistent indentation, moreover):
test $(grep ^commit actual | wc -l) -eq 0 &&
> echo change > file &&
> git commit -m "last commit" file &&
> - test $(
> - git fast-export --import-marks=tmp-marks \
> - --export-marks=tmp-marks HEAD |
> - grep ^commit\ |
> + git fast-export --import-marks=tmp-marks \
> + --export-marks=tmp-marks HEAD >actual &&
> + test $(grep ^commit\ actual |
> wc -l) \
> -eq 1 &&
Same comment.
> test_line_count = 4 tmp-marks
> @@ -500,13 +501,13 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
> git fast-export --import-marks=tmp-marks \
> --export-marks=tmp-marks master > /dev/null &&
> git fast-export --import-marks=tmp-marks \
> - --export-marks=tmp-marks master > actual &&
> + --export-marks=tmp-marks master >actual &&
This change is unrelated to the purpose of this patch, thus is noise
which distracts reviewers from real changes. Fixing style problems in
code you're touching is fine (and usually recommended), however, this
code is outside the scope of what the patch should be touching (there
is no piping output of a git command here). Moreover, it doesn't make
sense to fix only "> actual" but not "> /dev/null" just above it.
Consequently, this change should be dropped from the patch.
> test_cmp expected actual
> '
^ permalink raw reply [flat|nested] 16+ messages in thread
* [GSoC][PATCH v5] test: avoid pipes in git related commands for test
2018-03-25 8:37 ` Eric Sunshine
@ 2018-03-27 17:31 ` Pratik Karki
2018-03-30 21:45 ` Eric Sunshine
0 siblings, 1 reply; 16+ messages in thread
From: Pratik Karki @ 2018-03-27 17:31 UTC (permalink / raw)
To: Git List; +Cc: Pratik Karki, Eric Sunshine
Thank you Eric, I made changes according to your review.
Cheers,
Pratik
-- >8 --
Avoid using pipes downstream of Git commands since the exit codes
of commands upstream of pipes get swallowed, thus potentially
hiding failure of those commands. Instead, capture Git command
output to a file and apply the downstream command(s) to that file.
Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
---
t/t5300-pack-object.sh | 8 ++---
t/t5510-fetch.sh | 8 ++---
t/t7001-mv.sh | 22 ++++++-------
t/t7003-filter-branch.sh | 9 ++++--
t/t9104-git-svn-follow-parent.sh | 16 +++++-----
t/t9108-git-svn-glob.sh | 14 +++++----
t/t9109-git-svn-multi-glob.sh | 24 ++++++++------
t/t9110-git-svn-use-svm-props.sh | 40 ++++++++++++------------
t/t9111-git-svn-use-svnsync-props.sh | 36 ++++++++++-----------
t/t9114-git-svn-dcommit-merge.sh | 10 +++---
t/t9130-git-svn-authors-file.sh | 28 ++++++++++-------
t/t9138-git-svn-authors-prog.sh | 31 +++++++++---------
t/t9153-git-svn-rewrite-uuid.sh | 8 ++---
t/t9168-git-svn-partially-globbed-names.sh | 34 +++++++++++---------
t/t9350-fast-export.sh | 50 ++++++++++++++----------------
15 files changed, 179 insertions(+), 159 deletions(-)
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 9c68b9925..156beb2d5 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -311,8 +311,8 @@ test_expect_success 'unpacking with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$( git pack-objects test-5 <actual ) &&
PACK6=$( (
echo "$LIST"
echo "$LI"
@@ -358,8 +358,8 @@ test_expect_success 'index-pack with --strict' '
rm -f .git/index &&
tail -n 10 LIST | git update-index --index-info &&
ST=$(git write-tree) &&
- PACK5=$( git rev-list --objects "$LIST" "$LI" "$ST" | \
- git pack-objects test-5 ) &&
+ git rev-list --objects "$LIST" "$LI" "$ST" >actual &&
+ PACK5=$( git pack-objects test-5 <actual ) &&
PACK6=$( (
echo "$LIST"
echo "$LI"
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index da9ac0055..ae5a530a2 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -840,8 +840,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch aligned output' '
test_commit looooooooooooong-tag &&
(
cd full-output &&
- git -c fetch.output=full fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=full fetch origin >actual 2>&1 &&
+ grep -e "->" actual | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/master
@@ -855,8 +855,8 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact output' '
test_commit extraaa &&
(
cd compact &&
- git -c fetch.output=compact fetch origin 2>&1 | \
- grep -e "->" | cut -c 22- >../actual
+ git -c fetch.output=compact fetch origin >actual 2>&1 &&
+ grep -e "->" actual | cut -c 22- >../actual
) &&
cat >expect <<-\EOF &&
master -> origin/*
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index d4e6485a2..e96cbdb10 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -21,8 +21,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path1/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path1/COPYING" actual'
test_expect_success \
'moving the file back into subdirectory' \
@@ -35,8 +35,8 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path1/COPYING..*path0/COPYING"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path1/COPYING..*path0/COPYING" actual'
test_expect_success \
'mv --dry-run does not move file' \
@@ -122,10 +122,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/COPYING..*path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path0/README..*path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
+ grep "^R100..*path0/README..*path2/README" actual'
test_expect_success \
'succeed when source is a prefix of destination' \
@@ -141,10 +140,9 @@ test_expect_success \
test_expect_success \
'checking the commit' \
- 'git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/COPYING..*path1/path2/COPYING" &&
- git diff-tree -r -M --name-status HEAD^ HEAD | \
- grep "^R100..*path2/README..*path1/path2/README"'
+ 'git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ grep "^R100..*path2/COPYING..*path1/path2/COPYING" actual &&
+ grep "^R100..*path2/README..*path1/path2/README" actual'
test_expect_success \
'do not move directory over existing directory' \
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 7cb60799b..6a28b6cce 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -187,7 +187,8 @@ test_expect_success 'author information is preserved' '
test \$GIT_COMMIT != $(git rev-parse master) || \
echo Hallo" \
preserved-author) &&
- test 1 = $(git rev-list --author="B V Uips" preserved-author | wc -l)
+ git rev-list --author="B V Uips" preserved-author >actual &&
+ test_line_count = 1 actual
'
test_expect_success "remove a certain author's commits" '
@@ -205,7 +206,8 @@ test_expect_success "remove a certain author's commits" '
cnt1=$(git rev-list master | wc -l) &&
cnt2=$(git rev-list removed-author | wc -l) &&
test $cnt1 -eq $(($cnt2 + 1)) &&
- test 0 = $(git rev-list --author="B V Uips" removed-author | wc -l)
+ git rev-list --author="B V Uips" removed-author >actual &&
+ test_line_count = 0 actual
'
test_expect_success 'barf on invalid name' '
@@ -258,7 +260,8 @@ test_expect_success 'Subdirectory filter with disappearing trees' '
git commit -m "Re-adding foo" &&
git filter-branch -f --subdirectory-filter foo &&
- test $(git rev-list master | wc -l) = 3
+ git rev-list master >actual &&
+ test_line_count = 3 actual
'
test_expect_success 'Tag name filtering retains tag message' '
diff --git a/t/t9104-git-svn-follow-parent.sh b/t/t9104-git-svn-follow-parent.sh
index cd480edf1..a735fa371 100755
--- a/t/t9104-git-svn-follow-parent.sh
+++ b/t/t9104-git-svn-follow-parent.sh
@@ -33,8 +33,8 @@ test_expect_success 'init and fetch a moved directory' '
git svn fetch -i thunk &&
test "$(git rev-parse --verify refs/remotes/thunk@2)" \
= "$(git rev-parse --verify refs/remotes/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/thunk:readme |\
- sed -n -e "3p")" = goodbye &&
+ git cat-file blob refs/remotes/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye &&
test -z "$(git config --get svn-remote.svn.fetch \
"^trunk:refs/remotes/thunk@2$")"
'
@@ -48,8 +48,8 @@ test_expect_success 'init and fetch from one svn-remote' '
git svn fetch -i svn/thunk &&
test "$(git rev-parse --verify refs/remotes/svn/trunk)" \
= "$(git rev-parse --verify refs/remotes/svn/thunk~1)" &&
- test "$(git cat-file blob refs/remotes/svn/thunk:readme |\
- sed -n -e "3p")" = goodbye
+ git cat-file blob refs/remotes/svn/thunk:readme >actual &&
+ test "$(sed -n -e "3p" actual)" = goodbye
'
test_expect_success 'follow deleted parent' '
@@ -107,7 +107,8 @@ test_expect_success 'follow deleted directory' '
git svn init --minimize-url -i glob "$svnrepo"/glob &&
git svn fetch -i glob &&
test "$(git cat-file blob refs/remotes/glob:blob/bye)" = hi &&
- test "$(git ls-tree refs/remotes/glob | wc -l )" -eq 1
+ git ls-tree refs/remotes/glob >actual &&
+ test_line_count = 1 actual
'
# ref: r9270 of the Subversion repository: (http://svn.collab.net/repos/svn)
@@ -204,8 +205,9 @@ test_expect_success "follow-parent is atomic" '
test_expect_success "track multi-parent paths" '
svn_cmd cp -m "resurrect /glob" "$svnrepo"/r9270 "$svnrepo"/glob &&
git svn multi-fetch &&
- test $(git cat-file commit refs/remotes/glob | \
- grep "^parent " | wc -l) -eq 2
+ git cat-file commit refs/remotes/glob >actual &&
+ grep "^parent " actual >actual2 &&
+ test_line_count = 2 actual2
'
test_expect_success "multi-fetch continues to work" "
diff --git a/t/t9108-git-svn-glob.sh b/t/t9108-git-svn-glob.sh
index a94286c8e..6990f6436 100755
--- a/t/t9108-git-svn-glob.sh
+++ b/t/t9108-git-svn-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
diff --git a/t/t9109-git-svn-multi-glob.sh b/t/t9109-git-svn-multi-glob.sh
index 8d99e848d..c1e7542a3 100755
--- a/t/t9109-git-svn-multi-glob.sh
+++ b/t/t9109-git-svn-multi-glob.sh
@@ -47,8 +47,8 @@ test_expect_success 'test refspec globbing' '
git config --add svn-remote.svn.tags\
"tags/*/src/a:refs/remotes/tags/*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/end | \
- sed -e "s/^.\{41\}//" > output.end &&
+ git log --pretty=oneline refs/remotes/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/end~1)" = \
"$(git rev-parse refs/remotes/branches/v1/start)" &&
@@ -75,14 +75,16 @@ test_expect_success 'test left-hand-side only globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/v1/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/v1/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/v1/start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/end~3) = \
$(git rev-parse refs/remotes/two/branches/v1/start) &&
- git log --pretty=oneline refs/remotes/two/tags/end | \
- sed -e "s/^.\{41\}//" > output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
cat > expect.four <<EOF
@@ -124,14 +126,16 @@ test_expect_success 'test another branch' '
git config --add svn-remote.four.tags \
"tags/*:refs/remotes/four/tags/*" &&
git svn fetch four &&
- test $(git rev-list refs/remotes/four/tags/next | wc -l) -eq 5 &&
- test $(git rev-list refs/remotes/four/branches/v2/start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/four/tags/next >actual &&
+ test_line_count = 5 actual &&
+ git rev-list refs/remotes/four/branches/v2/start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/four/branches/v2/start~2) = \
$(git rev-parse refs/remotes/four/trunk) &&
test $(git rev-parse refs/remotes/four/tags/next~2) = \
$(git rev-parse refs/remotes/four/branches/v2/start) &&
- git log --pretty=oneline refs/remotes/four/tags/next | \
- sed -e "s/^.\{41\}//" > output.four &&
+ git log --pretty=oneline refs/remotes/four/tags/next >actual &&
+ sed -e "s/^.\{41\}//" actual >output.four &&
test_cmp expect.four output.four
'
diff --git a/t/t9110-git-svn-use-svm-props.sh b/t/t9110-git-svn-use-svm-props.sh
index dde0a3c22..ad37d980c 100755
--- a/t/t9110-git-svn-use-svm-props.sh
+++ b/t/t9110-git-svn-use-svm-props.sh
@@ -21,37 +21,37 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_expect_success 'find commit based on SVN revision number' "
- git svn find-rev r12 |
- grep $(git rev-parse HEAD)
+ git svn find-rev r12 >actual &&
+ grep $(git rev-parse HEAD) actual
"
test_expect_success 'empty rebase' "
diff --git a/t/t9111-git-svn-use-svnsync-props.sh b/t/t9111-git-svn-use-svnsync-props.sh
index 22b6e5ee7..6c9307355 100755
--- a/t/t9111-git-svn-use-svnsync-props.sh
+++ b/t/t9111-git-svn-use-svnsync-props.sh
@@ -20,32 +20,32 @@ uuid=161ce429-a9dd-4828-af4a-52023f968c89
bar_url=http://mayonaise/svnrepo/bar
test_expect_success 'verify metadata for /bar' "
- git cat-file commit refs/remotes/bar | \
- grep '^git-svn-id: $bar_url@12 $uuid$' &&
- git cat-file commit refs/remotes/bar~1 | \
- grep '^git-svn-id: $bar_url@11 $uuid$' &&
- git cat-file commit refs/remotes/bar~2 | \
- grep '^git-svn-id: $bar_url@10 $uuid$' &&
- git cat-file commit refs/remotes/bar~3 | \
- grep '^git-svn-id: $bar_url@9 $uuid$' &&
- git cat-file commit refs/remotes/bar~4 | \
- grep '^git-svn-id: $bar_url@6 $uuid$' &&
- git cat-file commit refs/remotes/bar~5 | \
- grep '^git-svn-id: $bar_url@1 $uuid$'
+ git cat-file commit refs/remotes/bar >actual &&
+ grep '^git-svn-id: $bar_url@12 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~1 >actual &&
+ grep '^git-svn-id: $bar_url@11 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~2 >actual &&
+ grep '^git-svn-id: $bar_url@10 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~3 >actual &&
+ grep '^git-svn-id: $bar_url@9 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~4 >actual &&
+ grep '^git-svn-id: $bar_url@6 $uuid$' actual &&
+ git cat-file commit refs/remotes/bar~5 >actual &&
+ grep '^git-svn-id: $bar_url@1 $uuid$' actual
"
e_url=http://mayonaise/svnrepo/dir/a/b/c/d/e
test_expect_success 'verify metadata for /dir/a/b/c/d/e' "
- git cat-file commit refs/remotes/e | \
- grep '^git-svn-id: $e_url@1 $uuid$'
+ git cat-file commit refs/remotes/e >actual &&
+ grep '^git-svn-id: $e_url@1 $uuid$' actual
"
dir_url=http://mayonaise/svnrepo/dir
test_expect_success 'verify metadata for /dir' "
- git cat-file commit refs/remotes/dir | \
- grep '^git-svn-id: $dir_url@2 $uuid$' &&
- git cat-file commit refs/remotes/dir~1 | \
- grep '^git-svn-id: $dir_url@1 $uuid$'
+ git cat-file commit refs/remotes/dir >actual &&
+ grep '^git-svn-id: $dir_url@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/dir~1 >actual &&
+ grep '^git-svn-id: $dir_url@1 $uuid$' actual
"
test_done
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 50bca62de..32317d6bc 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -68,7 +68,8 @@ test_debug 'gitk --all & sleep 1'
test_expect_success 'verify pre-merge ancestry' "
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'git svn dcommit merges' "
@@ -82,12 +83,13 @@ test_expect_success 'verify post-merge ancestry' "
x\$(git rev-parse --verify refs/remotes/origin/trunk) &&
test x\$(git rev-parse --verify refs/heads/svn^2) = \
x\$(git rev-parse --verify refs/heads/merge) &&
- git cat-file commit refs/heads/svn^ | grep '^friend$'
+ git cat-file commit refs/heads/svn^ >actual &&
+ grep '^friend$' actual
"
test_expect_success 'verify merge commit message' "
- git rev-list --pretty=raw -1 refs/heads/svn | \
- grep \" Merge branch 'merge' into svn\"
+ git rev-list --pretty=raw -1 refs/heads/svn >actual &&
+ grep \" Merge branch 'merge' into svn\" actual
"
test_done
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index 41264818c..7752a1fae 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -26,11 +26,12 @@ test_expect_success 'start import with incomplete authors file' '
test_expect_success 'imported 2 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 2 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author BBBBBBB BBBBBBB <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author AAAAAAA AAAAAAA <aa@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 2 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author BBBBBBB BBBBBBB <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author AAAAAAA AAAAAAA <aa@example\.com> " actual
)
'
@@ -43,11 +44,12 @@ test_expect_success 'continues to import once authors have been added' '
(
cd x
git svn fetch --authors-file=../svn-authors &&
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 4 &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author DDDDDDD DDDDDDD <dd@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author CCCCCCC CCCCCCC <cc@example\.com> "
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 4 actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author DDDDDDD DDDDDDD <dd@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author CCCCCCC CCCCCCC <cc@example\.com> " actual
)
'
@@ -102,8 +104,10 @@ test_expect_success !MINGW 'fresh clone with svn.authors-file in config' '
test x"$HOME"/svn-authors = x"$(git config svn.authorsfile)" &&
git svn clone "$svnrepo" gitconfig.clone &&
cd gitconfig.clone &&
- nr_ex=$(git log | grep "^Author:.*example.com" | wc -l) &&
- nr_rev=$(git rev-list HEAD | wc -l) &&
+ git log >actual &&
+ nr_ex=$(grep "^Author:.*example.com" actual | wc -l) &&
+ git rev-list HEAD >actual &&
+ nr_rev=$(wc -l <actual) &&
test $nr_rev -eq $nr_ex
)
'
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 7d7e9d46b..48109f949 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -37,31 +37,32 @@ test_expect_success 'import authors with prog and file' '
test_expect_success 'imported 6 revisions successfully' '
(
cd x
- test "$(git rev-list refs/remotes/git-svn | wc -l)" -eq 6
+ git rev-list refs/remotes/git-svn >actual &&
+ test_line_count = 6 actual
)
'
test_expect_success 'authors-prog ran correctly' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
- grep "^author ee-foo <ee-foo@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~2 | \
- grep "^author dd <dd@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~3 | \
- grep "^author cc <cc@sub\.example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~4 | \
- grep "^author bb <bb@example\.com> " &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn~5 | \
- grep "^author aa <aa@example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
+ grep "^author ee-foo <ee-foo@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~2 >actual &&
+ grep "^author dd <dd@sub\.example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~3 >actual &&
+ grep "^author cc <cc@sub\.example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~4 >actual &&
+ grep "^author bb <bb@example\.com> " actual &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn~5 >actual &&
+ grep "^author aa <aa@example\.com> " actual
)
'
test_expect_success 'authors-file overrode authors-prog' '
(
cd x
- git rev-list -1 --pretty=raw refs/remotes/git-svn | \
- grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> "
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> " actual
)
'
@@ -73,8 +74,8 @@ test_expect_success 'authors-prog handled special characters in username' '
(
cd x &&
git svn --authors-prog=../svn-authors-prog fetch &&
- git rev-list -1 --pretty=raw refs/remotes/git-svn |
- grep "^author xyz; touch evil <xyz; touch evil@example\.com> " &&
+ git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
+ grep "^author xyz; touch evil <xyz; touch evil@example\.com> " actual &&
! test -f evil
)
'
diff --git a/t/t9153-git-svn-rewrite-uuid.sh b/t/t9153-git-svn-rewrite-uuid.sh
index 372ef1568..8cb2b5c69 100755
--- a/t/t9153-git-svn-rewrite-uuid.sh
+++ b/t/t9153-git-svn-rewrite-uuid.sh
@@ -16,10 +16,10 @@ test_expect_success 'load svn repo' "
"
test_expect_success 'verify uuid' "
- git cat-file commit refs/remotes/git-svn~0 | \
- grep '^git-svn-id: .*@2 $uuid$' &&
- git cat-file commit refs/remotes/git-svn~1 | \
- grep '^git-svn-id: .*@1 $uuid$'
+ git cat-file commit refs/remotes/git-svn~0 >actual &&
+ grep '^git-svn-id: .*@2 $uuid$' actual &&
+ git cat-file commit refs/remotes/git-svn~1 >actual &&
+ grep '^git-svn-id: .*@1 $uuid$' actual
"
test_done
diff --git a/t/t9168-git-svn-partially-globbed-names.sh b/t/t9168-git-svn-partially-globbed-names.sh
index 8b22f2272..bdf6e8499 100755
--- a/t/t9168-git-svn-partially-globbed-names.sh
+++ b/t/t9168-git-svn-partially-globbed-names.sh
@@ -48,8 +48,8 @@ test_expect_success 'test refspec prefixed globbing' '
git config --add svn-remote.svn.tags\
"tags/t_*/src/a:refs/remotes/tags/t_*" &&
git svn multi-fetch &&
- git log --pretty=oneline refs/remotes/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.end &&
+ git log --pretty=oneline refs/remotes/tags/t_end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.end &&
test_cmp expect.end output.end &&
test "$(git rev-parse refs/remotes/tags/t_end~1)" = \
"$(git rev-parse refs/remotes/branches/b_start)" &&
@@ -78,14 +78,16 @@ test_expect_success 'test left-hand-side only prefixed globbing' '
svn_cmd commit -m "try to try"
) &&
git svn fetch two &&
- test $(git rev-list refs/remotes/two/tags/t_end | wc -l) -eq 6 &&
- test $(git rev-list refs/remotes/two/branches/b_start | wc -l) -eq 3 &&
+ git rev-list refs/remotes/two/tags/t_end >actual &&
+ test_line_count = 6 actual &&
+ git rev-list refs/remotes/two/branches/b_start >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/two/branches/b_start~2) = \
$(git rev-parse refs/remotes/two/trunk) &&
test $(git rev-parse refs/remotes/two/tags/t_end~3) = \
$(git rev-parse refs/remotes/two/branches/b_start) &&
- git log --pretty=oneline refs/remotes/two/tags/t_end | \
- sed -e "s/^.\{41\}//" >output.two &&
+ git log --pretty=oneline refs/remotes/two/tags/t_end >actual &&
+ sed -e "s/^.\{41\}//" actual >output.two &&
test_cmp expect.two output.two
'
@@ -118,14 +120,16 @@ test_expect_success 'test prefixed globs match just prefix' '
svn_cmd up
) &&
git svn fetch three &&
- test $(git rev-list refs/remotes/three/branches/b_ | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/three/tags/t_ | wc -l) -eq 3 &&
+ git rev-list refs/remotes/three/branches/b_ >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/three/tags/t_ >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/three/branches/b_~1) = \
$(git rev-parse refs/remotes/three/trunk) &&
test $(git rev-parse refs/remotes/three/tags/t_~1) = \
$(git rev-parse refs/remotes/three/branches/b_) &&
- git log --pretty=oneline refs/remotes/three/tags/t_ | \
- sed -e "s/^.\{41\}//" >output.three &&
+ git log --pretty=oneline refs/remotes/three/tags/t_ >actual &&
+ sed -e "s/^.\{41\}//" actual >output.three &&
test_cmp expect.three output.three
'
@@ -186,14 +190,16 @@ test_expect_success 'test globbing in the middle of the word' '
svn_cmd up
) &&
git svn fetch five &&
- test $(git rev-list refs/remotes/five/branches/abcde | wc -l) -eq 2 &&
- test $(git rev-list refs/remotes/five/tags/fghij | wc -l) -eq 3 &&
+ git rev-list refs/remotes/five/branches/abcde >actual &&
+ test_line_count = 2 actual &&
+ git rev-list refs/remotes/five/tags/fghij >actual &&
+ test_line_count = 3 actual &&
test $(git rev-parse refs/remotes/five/branches/abcde~1) = \
$(git rev-parse refs/remotes/five/trunk) &&
test $(git rev-parse refs/remotes/five/tags/fghij~1) = \
$(git rev-parse refs/remotes/five/branches/abcde) &&
- git log --pretty=oneline refs/remotes/five/tags/fghij | \
- sed -e "s/^.\{41\}//" >output.five &&
+ git log --pretty=oneline refs/remotes/five/tags/fghij >actual &&
+ sed -e "s/^.\{41\}//" actual >output.five &&
test_cmp expect.five output.five
'
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 866ddf605..d5679ffb8 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -43,20 +43,20 @@ test_expect_success 'fast-export | fast-import' '
MUSS=$(git rev-parse --verify muss) &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --all |
+ git fast-export --all >actual &&
(cd new &&
git fast-import &&
test $MASTER = $(git rev-parse --verify refs/heads/master) &&
test $REIN = $(git rev-parse --verify refs/tags/rein) &&
test $WER = $(git rev-parse --verify refs/heads/wer) &&
- test $MUSS = $(git rev-parse --verify refs/tags/muss))
+ test $MUSS = $(git rev-parse --verify refs/tags/muss)) <actual
'
test_expect_success 'fast-export master~2..master' '
- git fast-export master~2..master |
- sed "s/master/partial/" |
+ git fast-export master~2..master >actual &&
+ sed "s/master/partial/" actual |
(cd new &&
git fast-import &&
test $MASTER != $(git rev-parse --verify refs/heads/partial) &&
@@ -74,11 +74,12 @@ test_expect_success 'iso-8859-1' '
test_tick &&
echo rosten >file &&
git commit -s -m den file &&
- git fast-export wer^..wer |
- sed "s/wer/i18n/" |
+ git fast-export wer^..wer >iso8859-1.fi &&
+ sed "s/wer/i18n/" iso8859-1.fi |
(cd new &&
git fast-import &&
- git cat-file commit i18n | grep "Áéí óú")
+ git cat-file commit i18n >actual &&
+ grep "Áéí óú" actual)
'
test_expect_success 'import/export-marks' '
@@ -87,20 +88,14 @@ test_expect_success 'import/export-marks' '
git fast-export --export-marks=tmp-marks HEAD &&
test -s tmp-marks &&
test_line_count = 3 tmp-marks &&
- test $(
- git fast-export --import-marks=tmp-marks\
- --export-marks=tmp-marks HEAD |
- grep ^commit |
- wc -l) \
- -eq 0 &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual &&
+ test $(grep ^commit actual | wc -l) -eq 0 &&
echo change > file &&
git commit -m "last commit" file &&
- test $(
- git fast-export --import-marks=tmp-marks \
- --export-marks=tmp-marks HEAD |
- grep ^commit\ |
- wc -l) \
- -eq 1 &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks HEAD >actual &&
+ test $(grep ^commit\ actual | wc -l) -eq 1 &&
test_line_count = 4 tmp-marks
'
@@ -184,7 +179,7 @@ test_expect_success 'submodule fast-export | fast-import' '
rm -rf new &&
mkdir new &&
git --git-dir=new/.git init &&
- git fast-export --signed-tags=strip --all |
+ git fast-export --signed-tags=strip --all >actual &&
(cd new &&
git fast-import &&
test "$SUBENT1" = "$(git ls-tree refs/heads/master^ sub)" &&
@@ -192,7 +187,7 @@ test_expect_success 'submodule fast-export | fast-import' '
git checkout master &&
git submodule init &&
git submodule update &&
- cmp sub/file ../sub/file)
+ cmp sub/file ../sub/file) <actual
'
@@ -367,12 +362,14 @@ test_expect_success 'path limiting with import-marks does not lose unmodified fi
echo more content >> file &&
test_tick &&
git commit -mnext file &&
- git fast-export --import-marks=marks simple -- file file0 | grep file0
+ git fast-export --import-marks=marks simple -- file file0 >actual &&
+ grep file0 actual
'
test_expect_success 'full-tree re-shows unmodified files' '
git checkout -f simple &&
- test $(git fast-export --full-tree simple | grep -c file0) -eq 3
+ git fast-export --full-tree simple >actual &&
+ test $(grep -c file0 actual) -eq 3
'
test_expect_success 'set-up a few more tags for tag export tests' '
@@ -505,8 +502,8 @@ test_expect_success 'refs are updated even if no commits need to be exported' '
'
test_expect_success 'use refspec' '
- git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
- grep "^commit " | sort | uniq > actual &&
+ git fast-export --refspec refs/heads/master:refs/heads/foobar master >actual2 &&
+ grep "^commit " actual2 | sort | uniq >actual &&
echo "commit refs/heads/foobar" > expected &&
test_cmp expected actual
'
@@ -534,7 +531,8 @@ test_expect_success 'when using -C, do not declare copy when source of copy is a
git -C src commit -m 2nd_commit &&
test_create_repo dst &&
- git -C src fast-export --all -C | git -C dst fast-import &&
+ git -C src fast-export --all -C >actual &&
+ git -C dst fast-import <actual &&
git -C src show >expected &&
git -C dst show >actual &&
test_cmp expected actual
--
2.16.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH v5] test: avoid pipes in git related commands for test
2018-03-27 17:31 ` [GSoC][PATCH v5] " Pratik Karki
@ 2018-03-30 21:45 ` Eric Sunshine
2018-03-30 22:08 ` Junio C Hamano
0 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2018-03-30 21:45 UTC (permalink / raw)
To: Pratik Karki; +Cc: Git List
On Tue, Mar 27, 2018 at 1:31 PM, Pratik Karki <predatoramigo@gmail.com> wrote:
> Avoid using pipes downstream of Git commands since the exit codes
> of commands upstream of pipes get swallowed, thus potentially
> hiding failure of those commands. Instead, capture Git command
> output to a file and apply the downstream command(s) to that file.
>
>
> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
Unnecessary double blank line above sign-off.
Aside from that minor hiccup (which Junio fixed when queuing), this
iteration addresses all my review comments[1] from the previous round
and does not seem to introduce any new issues.
Thanks.
[1]: https://public-inbox.org/git/CAPig+cS3GjYo+5C_W6WqzK3RP=W+918E6Cz=FSvHky6EWCEZPA@mail.gmail.com/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [GSoC][PATCH v5] test: avoid pipes in git related commands for test
2018-03-30 21:45 ` Eric Sunshine
@ 2018-03-30 22:08 ` Junio C Hamano
0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2018-03-30 22:08 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Pratik Karki, Git List
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Tue, Mar 27, 2018 at 1:31 PM, Pratik Karki <predatoramigo@gmail.com> wrote:
>> Avoid using pipes downstream of Git commands since the exit codes
>> of commands upstream of pipes get swallowed, thus potentially
>> hiding failure of those commands. Instead, capture Git command
>> output to a file and apply the downstream command(s) to that file.
>>
>>
>> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
>
> Unnecessary double blank line above sign-off.
"git am" would automatically trigger stripspace, which would eat the
extra blank line from that two-blank-line block.
> Aside from that minor hiccup (which Junio fixed when queuing), this
> iteration addresses all my review comments[1] from the previous round
> and does not seem to introduce any new issues.
Thanks for a review.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-03-30 22:08 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 20:19 [GSoC] [PATCH] test: avoid pipes in git related commands for test suite Pratik Karki
2018-03-14 7:30 ` Eric Sunshine
2018-03-14 9:57 ` Ævar Arnfjörð Bjarmason
2018-03-14 18:22 ` Eric Sunshine
2018-03-15 17:04 ` Junio C Hamano
2018-03-19 17:32 ` [GSoC][PATCH] " Pratik Karki
2018-03-21 11:02 ` Eric Sunshine
2018-03-21 15:23 ` [GSoC][PATCH v3] test: avoid pipes in git related commands for test Pratik Karki
2018-03-21 18:11 ` Junio C Hamano
2018-03-21 18:45 ` Eric Sunshine
2018-03-21 18:58 ` Eric Sunshine
2018-03-23 15:01 ` [GSoC][PATCH v4] " Pratik Karki
2018-03-25 8:37 ` Eric Sunshine
2018-03-27 17:31 ` [GSoC][PATCH v5] " Pratik Karki
2018-03-30 21:45 ` Eric Sunshine
2018-03-30 22:08 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).