threads / patch / 7805

patchFix tests when using bash_completion

Subject: [PATCH] Fix tests when using bash_completion

## tl;dr

8 messages between Apr 23, 2007 and Apr 24, 2007. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Ron Parker· Apr 23, 2007, 20:51 UTC · lore

When using bash_completion, 'cd' echos the target directory name to standard out. If 'cd' participates in a pipe whose output is compared against an expected value, this causes a false failure report. Pipe cd's stdout to /dev/null in these cases.

Signed-off-by: Ron Parker <ron.parker@mobot.org>
---
	I was going to look into some of the preliminary subproject
	support, since I have a need for it.  But a bunch of the tests
	failed because I run the bash_completion package.
	I don't think I've sent any patches for git before.  Please be
	gentle.  If there is a problem with this patch or the way I've
	sent it, I'll gladly redo it.
 t/t3040-subprojects-basic.sh |    2 +-
 t/t5000-tar-tree.sh          |   10 +++++-----
 t/t5500-fetch-pack.sh        |    8 ++++----
 t/t5502-quickfetch.sh        |    8 ++++----
 t/t7002-grep.sh              |    4 ++--
 5 files changed, 16 insertions(+), 16 deletions(-)
Show changes to 5 files +16 −16

t/t3040-subprojects-basic.sh, t/t5000-tar-tree.sh, t/t5500-fetch-pack.sh, t/t5502-quickfetch.sh, t/t7002-grep.sh

diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
index 79b9f23..9543921 100755
--- a/t/t3040-subprojects-basic.sh
+++ b/t/t3040-subprojects-basic.sh
@@ -61,7 +61,7 @@ test_expect_success 'check if git diff works for
subproject elements' \
 test_expect_success 'check if clone works' \
     'git ls-files -s >expected &&
     git clone -l -s . cloned &&
-    ( cd cloned && git ls-files -s ) >current &&
+    ( cd cloned >/dev/null && git ls-files -s ) >current &&
     git diff expected current'
 
 test_expect_success 'removing and adding subproject' \
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e223c07..a6654a3 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -38,7 +38,7 @@ test_expect_success \
      (p=long_path_to_a_file && cd a &&
       for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
       echo text >file_with_long_path) &&
-     (cd a && find .) | sort >a.lst'
+     (cd a >/dev/null && find .) | sort >a.lst'
 
 test_expect_success \
     'add files to repository' \
@@ -80,7 +80,7 @@ test_expect_success \
 
 test_expect_success \
     'validate filenames' \
-    '(cd b/a && find .) | sort >b.lst &&
+    '(cd b/a >/dev/null && find .) | sort >b.lst &&
      diff a.lst b.lst'
 
 test_expect_success \
@@ -97,7 +97,7 @@ test_expect_success \
 
 test_expect_success \
     'validate filenames with prefix' \
-    '(cd c/prefix/a && find .) | sort >c.lst &&
+    '(cd c/prefix/a >/dev/null && find .) | sort >c.lst &&
      diff a.lst c.lst'
 
 test_expect_success \
@@ -114,7 +114,7 @@ test_expect_success \
 
 test_expect_success \
     'validate filenames' \
-    '(cd d/a && find .) | sort >d.lst &&
+    '(cd d/a >/dev/null && find .) | sort >d.lst &&
      diff a.lst d.lst'
 
 test_expect_success \
@@ -131,7 +131,7 @@ test_expect_success \
 
 test_expect_success \
     'validate filenames with prefix' \
-    '(cd e/prefix/a && find .) | sort >e.lst &&
+    '(cd e/prefix/a >/dev/null && find .) | sort >e.lst &&
      diff a.lst e.lst'
 
 test_expect_success \
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 48e3d17..d8ed508 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -131,7 +131,7 @@ pull_to_client 3rd "A" $((1*3)) # old fails
 
 test_expect_success "clone shallow" "git-clone --depth 2 . shallow"
 
-(cd shallow; git-count-objects -v) > count.shallow
+(cd shallow >/dev/null; git-count-objects -v) > count.shallow
 
 test_expect_success "clone shallow object count" \
 	"test \"in-pack: 18\" = \"$(grep in-pack count.shallow)\""
@@ -155,7 +155,7 @@ add B67 $B66
 test_expect_success "pull in shallow repo" \
 	"(cd shallow; git pull .. B)"
 
-(cd shallow; git-count-objects -v) > count.shallow
+(cd shallow >/dev/null; git-count-objects -v) > count.shallow
 test_expect_success "clone shallow object count" \
 	"test \"count: 6\" = \"$(grep count count.shallow)\""
 
@@ -165,14 +165,14 @@ add B69 $B68
 test_expect_success "deepening pull in shallow repo" \
 	"(cd shallow; git pull --depth 4 .. B)"
 
-(cd shallow; git-count-objects -v) > count.shallow
+(cd shallow >/dev/null; git-count-objects -v) > count.shallow
 test_expect_success "clone shallow object count" \
 	"test \"count: 12\" = \"$(grep count count.shallow)\""
 
 test_expect_success "deepening fetch in shallow repo" \
 	"(cd shallow; git fetch --depth 4 .. A:A)"
 
-(cd shallow; git-count-objects -v) > count.shallow
+(cd shallow >/dev/null; git-count-objects -v) > count.shallow
 test_expect_success "clone shallow object count" \
 	"test \"count: 18\" = \"$(grep count count.shallow)\""
 
diff --git a/t/t5502-quickfetch.sh b/t/t5502-quickfetch.sh
index b4760f2..b33e414 100755
--- a/t/t5502-quickfetch.sh
+++ b/t/t5502-quickfetch.sh
@@ -26,7 +26,7 @@ test_expect_success 'clone without alternate' '
 		git remote add -f origin ..
 	) &&
 	cnt=$( (
-		cd cloned &&
+		cd cloned >/dev/null &&
 		git count-objects | sed -e "s/ *objects,.*//"
 	) ) &&
 	test $cnt -eq 3
@@ -54,7 +54,7 @@ test_expect_success 'copy commit and tree but not blob
by hand' '
 	) &&
 
 	cnt=$( (
-		cd cloned &&
+		cd cloned >/dev/null &&
 		git count-objects | sed -e "s/ *objects,.*//"
 	) ) &&
 	test $cnt -eq 6
@@ -64,7 +64,7 @@ test_expect_success 'copy commit and tree but not blob
by hand' '
 	rm -f "cloned/.git/objects/$blob" &&
 
 	cnt=$( (
-		cd cloned &&
+		cd cloned >/dev/null &&
 		git count-objects | sed -e "s/ *objects,.*//"
 	) ) &&
 	test $cnt -eq 5
@@ -79,7 +79,7 @@ test_expect_success 'quickfetch should not leave a
corrupted repository' '
 	) &&
 
 	cnt=$( (
-		cd cloned &&
+		cd cloned >/dev/null &&
 		git count-objects | sed -e "s/ *objects,.*//"
 	) ) &&
 	test $cnt -eq 6
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 6bfb899..5d2fc4a 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -92,7 +92,7 @@ do
 	test_expect_success "grep $L (t-2)" '
 		echo "${HC}t:1:test" >expected &&
 		(
-			cd t &&
+			cd t >/dev/null &&
 			git grep -n -e test $H
 		) >actual &&
 		diff expected actual
@@ -101,7 +101,7 @@ do
 	test_expect_success "grep $L (t-3)" '
 		echo "${HC}t/t:1:test" >expected &&
 		(
-			cd t &&
+			cd t >/dev/null &&
 			git grep --full-name -n -e test $H
 		) >actual &&
 		diff expected actual
-- 
1.5.2.rc0.1.g2cc31
Junio C Hamano· Apr 23, 2007, 21:15 UTC · re: Ron Parker · lore

Re: [PATCH] Fix tests when using bash_completion

"Ron Parker" <ron.parker@mobot.org> writes:
Show 5 quoted lines
> When using bash_completion, 'cd' echos the target directory name to
> standard
> out.  If 'cd' participates in a pipe whose output is compared against an
> expected value, this causes a false failure report.  Pipe cd's stdout to
> /dev/null in these cases.
This is way too ugly.
Are you sure bash-completion is the culprit, not your CDPATH?
Julian Phillips· Apr 23, 2007, 21:39 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fix tests when using bash_completion

On Mon, 23 Apr 2007, Junio C Hamano wrote:
Show 11 quoted lines
> "Ron Parker" <ron.parker@mobot.org> writes:
>
>> When using bash_completion, 'cd' echos the target directory name to
>> standard
>> out.  If 'cd' participates in a pipe whose output is compared against an
>> expected value, this causes a false failure report.  Pipe cd's stdout to
>> /dev/null in these cases.
>
> This is way too ugly.
>
> Are you sure bash-completion is the culprit, not your CDPATH?

even with bash-completion _and_ CDPATH I only get a print when changing to a directory in CDPATH not the current directory ... perhaps an alias set for cd?

(I'm not the only one who thinks CDPATH is ick am I? :S)
-- 
Julian

  ---
BOFH Excuse #350:

paradigm shift...without a clutch
Junio C Hamano· Apr 23, 2007, 21:56 UTC · re: Julian Phillips · lore

Re: [PATCH] Fix tests when using bash_completion

Julian Phillips <julian@quantumfyre.co.uk> writes:
> (I'm not the only one who thinks CDPATH is ick am I? :S)

CDPATH is Ok as long as it stays as an interactive shell variable.

Sadly, if you google for "CDPATH environment", you would see many many hits. That variable is not meant to be used as an environment variable.

Something like this in .bashrc is acceptable:
	case "$PS1" in
        '')
        	;; # do not do useless thing for noninteractive
	?*)
        	CDPATH=my:favorite:path
                . $HOME/.bash-completion
		;;
	esac

but people usually are way too careless than that and export CDPATH to the environment, and that is where breakages start.

Julian Phillips· Apr 23, 2007, 22:57 UTC · re: Junio C Hamano · lore

[OT] CDPATH (was: Re: [PATCH] Fix tests when using bash_completion)

On Mon, 23 Apr 2007, Junio C Hamano wrote:
Show 10 quoted lines
> Julian Phillips <julian@quantumfyre.co.uk> writes:
>
>> (I'm not the only one who thinks CDPATH is ick am I? :S)
>
> CDPATH is Ok as long as it stays as an interactive shell
> variable.
>
> Sadly, if you google for "CDPATH environment", you would see
> many many hits.  That variable is not meant to be used as an
> environment variable.

True - though personally I tend to use more discriminate shell functions anyway.

Show 14 quoted lines
>
> Something like this in .bashrc is acceptable:
>
> 	case "$PS1" in
>        '')
>        	;; # do not do useless thing for noninteractive
> 	?*)
>        	CDPATH=my:favorite:path
>                . $HOME/.bash-completion
> 		;;
> 	esac
>
> but people usually are way too careless than that and export
> CDPATH to the environment, and that is where breakages start.

I guess part of the problem is that a lot of these people don't actually understand the difference between shell variables and environment variables, or why the distinction is useful. Plenty of people seem to do "export FOO=..." by rote.

-- 
Julian

  ---
Anyone can hold the helm when the sea is calm.
 		-- Publius Syrus
Ron Parker· Apr 24, 2007, 13:34 UTC · re: Julian Phillips · lore

RE: [OT] CDPATH

Junio C Hamano <junkio@cox.net> wrote:
> Are you sure bash-completion is the culprit, not your CDPATH?

Sorry, I was half-asleep yesterday. Of course it was CDPATH and not bash-completion.

Julian Phillips <julian@quantumfyre.co.uk> wrote:
> CDPATH is Ok as long as it stays as an interactive shell
> variable.

Based on a shell I used years ago that had built-in functionality similar to CDPATH, I naïvely *assumed* CDPATH would only impact interactive shells.

In another message, Julian wrote:
> I guess part of the problem is that a lot of these people don't actually 
> understand the difference between shell variables and environment 
> variables, or why the distinction is useful.  Plenty of people seem to do 
> "export FOO=..." by rote.

Not quite my case, but assumptions are dangerous things. Based on my assumption, I intentionally exported CDPATH so it would propagate when I "xterm&". Grr, I guess I'll have to alias xterm instead.

Thanks for the enlightenment gentlemen.
Yet another reason I like "git reset --hard HEAD" for stupid patch removal.
Now if there was a "reset --hard HEAD" function for some people I've met...
Thanks,
Ron
Junio C Hamano· Apr 24, 2007, 16:57 UTC · re: Ron Parker · lore

Re: [OT] CDPATH

"Ron Parker" <ron.parker@mobot.org> writes:
Show 6 quoted lines
> Junio C Hamano <junkio@cox.net> wrote:
>
>> Are you sure bash-completion is the culprit, not your CDPATH?
>
> Sorry, I was half-asleep yesterday.  Of course it was CDPATH and not 
> bash-completion.

Thanks for confirmation. This suggests in t/test-lib.sh we'd better have "unset CDPATH" near the top to prevent surprises like this.

Ron Parker· Apr 24, 2007, 19:01 UTC · re: Junio C Hamano · lore

[PATCH] Unset CDPATH in the test library

Signed-off-by: Ron Parker <ron.parker@mobot.org>
---
	Junio,
	Your suggestion of unsetting CDPATH in
	t/test-lib.sh took care of the problem.
	Thanks.
 t/test-lib.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to t/test-lib.sh +1 −0
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c075474..8b99a76 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -11,6 +11,7 @@ TZ=UTC
 export LANG LC_ALL PAGER TZ
 EDITOR=:
 VISUAL=:
+unset CDPATH
 unset AUTHOR_DATE
 unset AUTHOR_EMAIL
 unset AUTHOR_NAME
-- 
1.5.0.5

← back to recent threads