# [PATCH] t6300-for-each-ref: fix "more than one quoting style" tests 5 messages from 2018-02-13 to 2018-02-15. Participants: SZEDER Gábor, Jeff King. Thread: https://gitlist.dev/t/47822 ## SZEDER Gábor, 2018-02-13 00:36 Subject: [PATCH] t6300-for-each-ref: fix "more than one quoting style" tests Message-ID: <20180213003601.7396-1-szeder.dev@gmail.com> URL: https://gitlist.dev/e/20180213003601.7396-1-szeder.dev%40gmail.com ``` 'git for-each-ref' should error out when invoked with more than one quoting style options. The tests checking this have two issues: - They run 'git for-each-ref' upstream of a pipe, hiding its exit code, thus don't actually checking that 'git for-each-ref' exits with error code. - They check the error message in a rather roundabout way. Ensure that 'git for-each-ref' exits with an error code using the 'test_must_fail' helper function, and check its error message by grepping its saved standard error. Signed-off-by: SZEDER Gábor --- Those tests were like this since ancient times, c9ecf4f12a (for-each-ref: Fix quoting style constants., 2007-12-06). t/t6300-for-each-ref.sh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index c128dfc579..295d1475bd 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -373,11 +373,8 @@ test_expect_success 'Quoting style: tcl' ' for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do test_expect_success "more than one quoting style: $i" " - git for-each-ref $i 2>&1 | (read line && - case \$line in - \"error: more than one quoting style\"*) : happy;; - *) false - esac) + test_must_fail git for-each-ref $i 2>err && + grep '^error: more than one quoting style' err " done -- 2.16.1.181.g4b60b0bfb6 ``` ## Jeff King, 2018-02-13 22:22 Subject: Re: [PATCH] t6300-for-each-ref: fix "more than one quoting style" tests Message-ID: <20180213222247.GA18401@sigill.intra.peff.net> URL: https://gitlist.dev/e/20180213222247.GA18401%40sigill.intra.peff.net In-Reply-To: <20180213003601.7396-1-szeder.dev@gmail.com> ``` On Tue, Feb 13, 2018 at 01:36:01AM +0100, SZEDER Gábor wrote: > 'git for-each-ref' should error out when invoked with more than one > quoting style options. The tests checking this have two issues: > > - They run 'git for-each-ref' upstream of a pipe, hiding its exit > code, thus don't actually checking that 'git for-each-ref' exits > with error code. > > - They check the error message in a rather roundabout way. > > Ensure that 'git for-each-ref' exits with an error code using the > 'test_must_fail' helper function, and check its error message by > grepping its saved standard error. Yeah, this looks much nicer than the original. > for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do > test_expect_success "more than one quoting style: $i" " > - git for-each-ref $i 2>&1 | (read line && > - case \$line in > - \"error: more than one quoting style\"*) : happy;; > - *) false > - esac) > + test_must_fail git for-each-ref $i 2>err && > + grep '^error: more than one quoting style' err I suspect in the long run this ought to be test_i18ngrep, but since it's not localized yet, it makes sense to stop here with this patch. -Peff ``` ## SZEDER Gábor, 2018-02-15 16:39 Subject: Re: [PATCH] t6300-for-each-ref: fix "more than one quoting style" tests Message-ID: URL: https://gitlist.dev/e/CAM0VKj%3DYPkS%3DYLtEz93RWS2%3Dcvyy6LcyrBsCyBCg14ihxuKzjA%40mail.gmail.com In-Reply-To: <20180213222247.GA18401@sigill.intra.peff.net> ``` On Tue, Feb 13, 2018 at 11:22 PM, Jeff King wrote: >> for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do >> test_expect_success "more than one quoting style: $i" " >> - git for-each-ref $i 2>&1 | (read line && >> - case \$line in >> - \"error: more than one quoting style\"*) : happy;; >> - *) false >> - esac) >> + test_must_fail git for-each-ref $i 2>err && >> + grep '^error: more than one quoting style' err > > I suspect in the long run this ought to be test_i18ngrep, but since it's > not localized yet, it makes sense to stop here with this patch. I thought 'git for-each-ref' is plumbing and that means that it shouldn't be localized. ``` ## Jeff King, 2018-02-15 16:52 Subject: Re: [PATCH] t6300-for-each-ref: fix "more than one quoting style" tests Message-ID: <20180215165218.GA7635@sigill.intra.peff.net> URL: https://gitlist.dev/e/20180215165218.GA7635%40sigill.intra.peff.net In-Reply-To: ``` On Thu, Feb 15, 2018 at 05:39:28PM +0100, SZEDER Gábor wrote: > On Tue, Feb 13, 2018 at 11:22 PM, Jeff King wrote: > > >> for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do > >> test_expect_success "more than one quoting style: $i" " > >> - git for-each-ref $i 2>&1 | (read line && > >> - case \$line in > >> - \"error: more than one quoting style\"*) : happy;; > >> - *) false > >> - esac) > >> + test_must_fail git for-each-ref $i 2>err && > >> + grep '^error: more than one quoting style' err > > > > I suspect in the long run this ought to be test_i18ngrep, but since it's > > not localized yet, it makes sense to stop here with this patch. > > I thought 'git for-each-ref' is plumbing and that means that it > shouldn't be localized. I always assumed stderr was mostly fair game, even for plumbing. At any rate, I'm willing to ignore the issue until somebody actually proposes a patch to translate it. -Peff ``` ## SZEDER Gábor, 2018-02-15 17:41 Subject: Re: [PATCH] t6300-for-each-ref: fix "more than one quoting style" tests Message-ID: URL: https://gitlist.dev/e/CAM0VKjmW6_3z49uVMFUpwsK5khZ_bNALDgkKkPUQdvu2PEogfQ%40mail.gmail.com In-Reply-To: <20180215165218.GA7635@sigill.intra.peff.net> ``` On Thu, Feb 15, 2018 at 5:52 PM, Jeff King wrote: > On Thu, Feb 15, 2018 at 05:39:28PM +0100, SZEDER Gábor wrote: > >> On Tue, Feb 13, 2018 at 11:22 PM, Jeff King wrote: >> >> >> for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do >> >> test_expect_success "more than one quoting style: $i" " >> >> - git for-each-ref $i 2>&1 | (read line && >> >> - case \$line in >> >> - \"error: more than one quoting style\"*) : happy;; >> >> - *) false >> >> - esac) >> >> + test_must_fail git for-each-ref $i 2>err && >> >> + grep '^error: more than one quoting style' err >> > >> > I suspect in the long run this ought to be test_i18ngrep, but since it's >> > not localized yet, it makes sense to stop here with this patch. >> >> I thought 'git for-each-ref' is plumbing and that means that it >> shouldn't be localized. > > I always assumed stderr was mostly fair game, even for plumbing. At any > rate, I'm willing to ignore the issue until somebody actually proposes a > patch to translate it. Ah, OK, will keep that in mind. Anyway, the first GETTEXT_POISON build will fail if that error string ever gets translated but the test doesn't get updated. ```