All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Wong <e@80x24.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] t9700: ensure cat-file info isn't buffered by default
Date: Mon, 17 Jun 2024 13:50:41 -0700	[thread overview]
Message-ID: <xmqq1q4v5m5a.fsf@gitster.g> (raw)
In-Reply-To: <20240617104326.3522535-3-e@80x24.org> (Eric Wong's message of "Mon, 17 Jun 2024 10:43:26 +0000")

Eric Wong <e@80x24.org> writes:

> Buffering by default breaks some 3rd-party Perl scripts using
> cat-file, but this breakage was not detected anywhere in our
> test suite.  The easiest place to test this behavior is with
> Git.pm, since (AFAIK) other equivalent way to test this behavior
> from Bourne shell and/or awk would require racy sleeps,
> non-portable FIFOs or tedious C code.

Yes, using Perl is a good substitute for writing it in C in this
case.  I however question the choice to use t9700/test.pl here,
which is clearly stated that its purpose is to "test perl interface
which is Git.pm", and added tests are not testing anything in Git.pm
at all.

Using t9700/test.pl only because it happens to use "perl -MTest::More"
sounds a bit eh, suboptimal.

It seems that there are Perl snippets in other tests (including
t1006 that is specifically about cat-file).  How involved would it
be to implement these new tests without modifying unrelated test
scripts?

> Signed-off-by: Eric Wong <e@80x24.org>
> ---
>  t/t9700/test.pl | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index d8e85482ab..94a2e2c09d 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -154,6 +154,20 @@ sub adjust_dirsep {
>  		     "abc\"\\ \x07\x08\x09\x0a\x0b\x0c\x0d\x01 ",
>  		     'unquote escape sequences');
>  
> +# ensure --batch-check is unbuffered by default
> +my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe(qw(cat-file --batch-check));
> +print $out $file1hash, "\n" or die $!;
> +my $info = <$in>;
> +is $info, "$file1hash blob 15\n", 'command_bidi_pipe w/ --batch-check';
> +$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
> +
> +# ditto with `info' with --batch-command
> +($pid, $in, $out, $ctx) = $r->command_bidi_pipe(qw(cat-file --batch-command));
> +print $out 'info ', $file1hash, "\n" or die $!;
> +$info = <$in>;
> +is $info, "$file1hash blob 15\n", 'command_bidi_pipe w/ --batch-command=info';
> +$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
> +
>  printf "1..%d\n", Test::More->builder->current_test;
>  
>  my $is_passing = eval { Test::More->is_passing };

  reply	other threads:[~2024-06-17 20:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17 10:43 [PATCH 0/2] cat-file related doc and test Eric Wong
2024-06-17 10:43 ` [PATCH 1/2] Git.pm: use array in command_bidi_pipe example Eric Wong
2024-06-17 20:33   ` Junio C Hamano
2024-06-17 10:43 ` [PATCH 2/2] t9700: ensure cat-file info isn't buffered by default Eric Wong
2024-06-17 20:50   ` Junio C Hamano [this message]
2024-06-18 21:30     ` [PATCH v2 2/2] t1006: " Eric Wong
2024-06-18 23:37       ` Junio C Hamano
2024-06-19 17:56         ` Eric Wong
2024-06-20 17:28           ` Junio C Hamano
2024-06-21  7:16       ` Jeff King
2024-06-21 20:00         ` Eric Wong
2024-06-24 15:19           ` Jeff King
2024-06-17 23:08   ` [PATCH 2/2] t9700: " Junio C Hamano
2024-06-19  9:08   ` Phillip Wood
2024-06-19 18:08     ` Eric Wong
2024-06-21 13:03       ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1q4v5m5a.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.