git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] t6050-replace: don't disable stdin for the whole test script
@ 2018-05-07 12:04 SZEDER Gábor
  2018-05-08 20:53 ` Johannes Schindelin
  0 siblings, 1 reply; 3+ messages in thread
From: SZEDER Gábor @ 2018-05-07 12:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, git, SZEDER Gábor

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 <szeder.dev@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] t6050-replace: don't disable stdin for the whole test script
  2018-05-07 12:04 [PATCH] t6050-replace: don't disable stdin for the whole test script SZEDER Gábor
@ 2018-05-08 20:53 ` Johannes Schindelin
  2018-05-09 20:40   ` SZEDER Gábor
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Schindelin @ 2018-05-08 20:53 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, Christian Couder, git

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] t6050-replace: don't disable stdin for the whole test script
  2018-05-08 20:53 ` Johannes Schindelin
@ 2018-05-09 20:40   ` SZEDER Gábor
  0 siblings, 0 replies; 3+ messages in thread
From: SZEDER Gábor @ 2018-05-09 20:40 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Christian Couder, Git mailing list

On Tue, May 8, 2018 at 10:53 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> 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.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-09 20:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 12:04 [PATCH] t6050-replace: don't disable stdin for the whole test script SZEDER Gábor
2018-05-08 20:53 ` Johannes Schindelin
2018-05-09 20:40   ` SZEDER Gábor

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).