From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Jeff King" <peff@peff.net>,
git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH v2 8/9] t: validate 'test_i18ngrep's parameters
Date: Thu, 8 Feb 2018 16:56:55 +0100 [thread overview]
Message-ID: <20180208155656.9831-9-szeder.dev@gmail.com> (raw)
In-Reply-To: <20180208155656.9831-1-szeder.dev@gmail.com>
Some of the previous patches in this series fixed bogus
'test_i18ngrep' invocations:
- Two invocations where the tested git command's standard output is
directly piped into 'test_i18ngrep'. While convenient, this is an
antipattern, because the pipe hides the git command's exit code,
and the test could continue even if the command exited with error.
- Two invocations that had neither a filename parameter nor anything
piped into their standard input, yet both managed to remain
unnoticed for years. A third similarly bogus invocation is
currently lurking in 'pu' for a couple of weeks now.
Prevent similar mistakes in the future by validating 'test_i18ngrep's
parameters requiring that
- The last parameter names an existing file to be read, effectively
forbiding piping into 'test_i18ngrep'.
Note that this change will also forbid cases where 'test_i18ngrep'
would legitimately read its standard input, e.g. when its standard
input is redirected from a file, or when a git command's standard
output is first written to an intermediate file, which is then
preprocessed by a non-git command before the results are piped
into 'test_i18ngrep'. See two of the previous patches for the
only such cases we had in our test suite. However, reliably
preventing the piping antipattern is arguably more important than
supporting these cases, which can be easily worked around by
opening the file directly or using an intermediate file anyway.
- There are at least two parameters, not including the optional '!'
to negate the pattern. This ought to catch corner cases when
'test_i18ngrep' looks for the name of an existing file on its
standard input; the above check would miss this case becase the
filename as pattern would be the last parameter.
Note that this is not quite perfect, as it doesn't account for any
'grep --options' given as parameters. However, doing so would be
far too complicated, considering that patterns can start with
dashes as well, and in the majority of the cases we don't use any
such options anyway.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/test-lib-functions.sh | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 92ed029371..a1676e0386 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -719,6 +719,18 @@ test_i18ncmp () {
# under GETTEXT_POISON this pretends that the command produced expected
# results.
test_i18ngrep () {
+ eval "last_arg=\"\${$#}\""
+
+ test -f "$last_arg" ||
+ error "bug in the test script: test_i18ngrep requires a file" \
+ "to read as the last parameter"
+
+ if test $# -lt 2 ||
+ { test "x!" = "x$1" && test $# -lt 3 ; }
+ then
+ error "bug in the test script: too few parameters to test_i18ngrep"
+ fi
+
if test -n "$GETTEXT_POISON"
then
: # pretend success
--
2.16.1.158.ge6451079d
next prev parent reply other threads:[~2018-02-08 15:57 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
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 ` SZEDER Gábor [this message]
2018-02-08 16:34 ` [PATCH v2 8/9] t: validate 'test_i18ngrep's parameters 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180208155656.9831-9-szeder.dev@gmail.com \
--to=szeder.dev@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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.
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).