All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH 08/10] t: forbid piping into 'test_i18ngrep'
Date: Fri, 26 Jan 2018 10:39:05 -0800	[thread overview]
Message-ID: <xmqqshas79di.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqq1sic8omp.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Fri, 26 Jan 2018 10:24:14 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> See two of the previous patches for the only such cases we had in our
>> test suite.  However, reliably preventing this antipattern is arguably
>> more important than supporting these cases, which can be worked around
>> by only minor inconveniences.
>
> I am not sure if that inconveniences will be minor.  Is this too
> contrived an example, for example?
>
>   check () {
>         pattern=$1 file=$2 script=./runme
>
>         test_i18ngrep "$pattern" "$file" &&
>         write_script "$script" &&
>         test_expect_success "check $pattern" '
>                 "$script"
>         '
>   }
>
>   check foo file <<-EOF
>   ... test script comes here ...
>   EOF

Is there a case where test_i18ngrep (after your clean-ups in this
series up to 06/10) needs to read from more than one file?

I actually think that the kind of inconveniences we *can* work with,
without risking breakage to legitimate test, would be to allow and
require test_i18ngrep to name and read only from one file that
appears at the end of its command line.  IOW, instead of doing a
probing "read" that you cannot undo and break legitimate test, I
think it is OK to see if the last token names a file that is on the
filesystem, e.g.

	test_i18ngrep () {
		eval test -f \"\${$#}\" ||
		error "bug in the test sript: test_i18ngrep must" \
		      "name a file to read as the last token on the command line"
		...


  reply	other threads:[~2018-01-26 18:39 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-26 12:36 [PATCH 00/10] 'test_i18ngrep'-related fixes and improvements SZEDER Gábor
2018-01-26 12:36 ` [PATCH 01/10] t5541: add 'test_i18ngrep's missing filename parameter SZEDER Gábor
2018-01-26 18:23   ` Jeff King
2018-01-26 18:23     ` Jeff King
2018-01-26 12:37 ` [PATCH 02/10] t5812: " SZEDER Gábor
2018-01-26 18:27   ` Jeff King
2018-02-07 13:53     ` SZEDER Gábor
2018-02-07 14:38       ` Jeff King
2018-01-30  9:50   ` Simon Ruderich
2018-01-26 12:37 ` [PATCH 03/10] t6022: don't run 'git merge' upstream of a pipe SZEDER Gábor
2018-01-26 12:37 ` [PATCH 04/10] t4001: don't run 'git status' " SZEDER Gábor
2018-01-26 12:37 ` [PATCH 05/10] t5510: consolidate 'grep' and 'test_i18ngrep' patterns SZEDER Gábor
2018-01-26 18:16   ` Junio C Hamano
2018-01-26 19:20     ` SZEDER Gábor
2018-01-26 19:23       ` Junio C Hamano
2018-01-26 12:37 ` [PATCH 06/10] t5536: let 'test_i18ngrep' read the file without redirection SZEDER Gábor
2018-01-26 12:37 ` [PATCH 07/10] t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh' SZEDER Gábor
2018-01-26 18:19   ` Junio C Hamano
2018-01-26 18:32     ` Jeff King
2018-01-26 19:08     ` SZEDER Gábor
2018-01-26 12:37 ` [PATCH 08/10] t: forbid piping into 'test_i18ngrep' SZEDER Gábor
2018-01-26 18:24   ` Junio C Hamano
2018-01-26 18:39     ` Junio C Hamano [this message]
2018-01-26 18:43       ` Jeff King
2018-01-26 18:51     ` SZEDER Gábor
2018-01-26 19:19       ` Junio C Hamano
2018-01-26 18:41   ` Jeff King
2018-01-26 12:37 ` [PATCH 09/10] t: make sure that 'test_i18ngrep' got enough parameters SZEDER Gábor
2018-01-26 18:47   ` Jeff King
2018-01-26 22:07   ` Eric Sunshine
2018-01-26 12:37 ` [PATCH 10/10] t: make 'test_i18ngrep' more informative on failure SZEDER Gábor
2018-01-26 18:50   ` Jeff King
2018-01-26 19:23     ` SZEDER Gábor
2018-01-26 19:25       ` Jeff King
2018-01-26 20:26         ` SZEDER Gábor
2018-01-26 20:32           ` Jeff King
2018-01-26 18:51 ` [PATCH 00/10] 'test_i18ngrep'-related fixes and improvements Jeff King
2018-02-08 15:56 ` [PATCH v2 0/9] " SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 1/9] t5541: add 'test_i18ngrep's missing filename parameter SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 2/9] t5812: " SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 3/9] t6022: don't run 'git merge' upstream of a pipe SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 4/9] t4001: don't run 'git status' " SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 5/9] t5510: consolidate 'grep' and 'test_i18ngrep' patterns SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 6/9] t5536: let 'test_i18ngrep' read the file without redirection SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 7/9] t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh' SZEDER Gábor
2018-02-08 15:56   ` [PATCH v2 8/9] t: validate 'test_i18ngrep's parameters SZEDER Gábor
2018-02-08 16:34     ` Jeff King
2018-02-08 15:56   ` [PATCH v2 9/9] t: make 'test_i18ngrep' more informative on failure SZEDER Gábor
2018-02-08 16:36   ` [PATCH v2 0/9] 'test_i18ngrep'-related fixes and improvements Jeff King

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=xmqqshas79di.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.com \
    /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.