[PATCH] t6050-replace: don't disable stdin for the whole test script

Subsystems: the rest

STALE3044d

3 messages, 2 authors, 2018-05-09 · open the first message on its own page

[PATCH] t6050-replace: don't disable stdin for the whole test script

From: SZEDER Gábor <hidden>
Date: 2018-05-07 12:04:13

The test script 't6050-replace.sh' starts off with redirecting the
whole test script's stdin from /dev/null.  This redirection has been
there since the test script was introduced in a3e8267225
(replace_object: add a test case, 2009-01-23), but the commit message
doesn't explain why it was deemed necessary.  AFAICT, it has never
been necessary, and t6050 runs just fine and succeeds even without it,
not only the current version but past versions as well.

Besides being unnecessary, this redirection is also harmful, as it
prevents the test helper functions 'test_pause' and 'debug' from
working properly in t6050, because we can't enter any commands to the
shell and the debugger, respectively.

So let's remove that redirection.

Signed-off-by: SZEDER Gábor <redacted>
---
 t/t6050-replace.sh | 2 --
 1 file changed, 2 deletions(-)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index c630aba657..199fbc78a3 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -4,8 +4,6 @@
 #
 test_description='Tests replace refs functionality'
 
-exec </dev/null
-
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-gpg.sh"
 
-- 
2.17.0.583.gcecc8b8e24

Re: [PATCH] t6050-replace: don't disable stdin for the whole test script

From: Johannes Schindelin <hidden>
Date: 2018-05-08 20:53:23

Hi Gábor,

On Mon, 7 May 2018, SZEDER Gábor wrote:
The test script 't6050-replace.sh' starts off with redirecting the whole
test script's stdin from /dev/null.  This redirection has been there
since the test script was introduced in a3e8267225 (replace_object: add
a test case, 2009-01-23), but the commit message doesn't explain why it
was deemed necessary.  AFAICT, it has never been necessary, and t6050
runs just fine and succeeds even without it, not only the current
version but past versions as well.

Besides being unnecessary, this redirection is also harmful, as it
prevents the test helper functions 'test_pause' and 'debug' from working
properly in t6050, because we can't enter any commands to the shell and
the debugger, respectively.
The redirection might have been necessary before 781f76b1582 (test-lib:
redirect stdin of tests, 2011-12-15), but it definitely is not necessary
now.

Thanks,
Dscho

Re: [PATCH] t6050-replace: don't disable stdin for the whole test script

From: SZEDER Gábor <hidden>
Date: 2018-05-09 20:40:16

On Tue, May 8, 2018 at 10:53 PM, Johannes Schindelin
[off-list ref] wrote:
On Mon, 7 May 2018, SZEDER Gábor wrote:
quoted
The test script 't6050-replace.sh' starts off with redirecting the whole
test script's stdin from /dev/null.  This redirection has been there
since the test script was introduced in a3e8267225 (replace_object: add
a test case, 2009-01-23), but the commit message doesn't explain why it
was deemed necessary.  AFAICT, it has never been necessary, and t6050
runs just fine and succeeds even without it, not only the current
version but past versions as well.

Besides being unnecessary, this redirection is also harmful, as it
prevents the test helper functions 'test_pause' and 'debug' from working
properly in t6050, because we can't enter any commands to the shell and
the debugger, respectively.
The redirection might have been necessary before 781f76b1582 (test-lib:
redirect stdin of tests, 2011-12-15), but it definitely is not necessary
now.
That doesn't seem to be an issue in a3e8267225 (or in any other
commits touching t6050 since):

  $ echo foobar | ( ./t6050-replace.sh ; read input ; echo $input )
  *   ok 1: set up buggy branch
  *   ok 2: replace the author
  * passed all 2 test(s)
  foobar
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help