On Mon, Dec 12, 2011 at 11:07:21AM -0800, Junio C Hamano wrote: > > Because of the latter, t7006.58ff cause unexpected results if you do: > > > > git rev-list <range> | > > while read sha; do > > git checkout sha > > make test > > done > > In the above, lack of dollar-sign in "git checkout $sha" is obvious ;-) > but I think it is a bug that you are not running make with its stdin > redirected from /dev/null in the first place. > > Perhaps "make test" should do that for all tests, not just this terminal > related one? Doing it this way we do not have to worry about other tests > reading from the standard input by mistake. That was my thought, as well. We want the test environment to be as sterile and predictable as possible, so connecting stdin to /dev/null seems like a sensible thing. > diff --git a/Makefile b/Makefile > index ed82320..7a85237 100644 > --- a/Makefile > +++ b/Makefile > @@ -2239,7 +2239,7 @@ export NO_SVN_TESTS > ### Testing rules > > test: all > - $(MAKE) -C t/ all > + $(MAKE) -C t/ all </dev/null Is this right place to do it? It doesn't catch "cd t && make". I would expect at the least for it to happen in t/Makefile. But I actually wonder if it should be in test-lib.sh, as it is as much about cleaning up the test script's environment as it is about protecting people running "make test" in a loop. I.e., something like: diff --git a/t/test-lib.sh b/t/test-lib.sh index bdd9513..5a38505 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -198,6 +198,9 @@ else exec 4>/dev/null 3>/dev/null fi +exec 6<&0 +exec 0</dev/null + test_failure=0 test_count=0 test_fixed=0 One downside of that approach is that it makes it harder to insert questionable debugging statements into test scripts. E.g., sometimes I will temporarily throw a "gdb" or even a "bash" invocation into a test script to investigate a failure. But that would still be possible by redirecting from "<6". -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html