{"thread":{"id":"47822","subject":"[PATCH] t6300-for-each-ref: fix \"more than one quoting style\" tests","startedAt":"2018-02-13T00:36:15Z","lastAt":"2018-02-15T17:41:50Z","messageCount":5,"participants":["SZEDER Gábor","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"339130","messageId":"20180213003601.7396-1-szeder.dev@gmail.com","threadId":"47822","inReplyTo":null,"subject":"[PATCH] t6300-for-each-ref: fix \"more than one quoting style\" tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-02-13T00:36:01Z","receivedAt":"2018-02-13T00:36:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"'git for-each-ref' should error out when invoked with more than one\nquoting style options.  The tests checking this have two issues:\n\n  - They run 'git for-each-ref' upstream of a pipe, hiding its exit\n    code, thus don't actually checking that 'git for-each-ref' exits\n    with error code.\n\n  - They check the error message in a rather roundabout way.\n\nEnsure that 'git for-each-ref' exits with an error code using the\n'test_must_fail' helper function, and check its error message by\ngrepping its saved standard error.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n\nThose tests were like this since ancient times, c9ecf4f12a\n(for-each-ref: Fix quoting style constants., 2007-12-06).\n\n\n t/t6300-for-each-ref.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex c128dfc579..295d1475bd 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -373,11 +373,8 @@ test_expect_success 'Quoting style: tcl' '\n \n for i in \"--perl --shell\" \"-s --python\" \"--python --tcl\" \"--tcl --perl\"; do\n \ttest_expect_success \"more than one quoting style: $i\" \"\n-\t\tgit for-each-ref $i 2>&1 | (read line &&\n-\t\tcase \\$line in\n-\t\t\\\"error: more than one quoting style\\\"*) : happy;;\n-\t\t*) false\n-\t\tesac)\n+\t\ttest_must_fail git for-each-ref $i 2>err &&\n+\t\tgrep '^error: more than one quoting style' err\n \t\"\n done\n \n-- \n2.16.1.181.g4b60b0bfb6\n\n"},{"id":"339235","messageId":"20180213222247.GA18401@sigill.intra.peff.net","threadId":"47822","inReplyTo":"20180213003601.7396-1-szeder.dev@gmail.com","subject":"Re: [PATCH] t6300-for-each-ref: fix \"more than one quoting style\" tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-13T22:22:47Z","receivedAt":"2018-02-13T22:22:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 13, 2018 at 01:36:01AM +0100, SZEDER Gábor wrote:\n\n> 'git for-each-ref' should error out when invoked with more than one\n> quoting style options.  The tests checking this have two issues:\n> \n>   - They run 'git for-each-ref' upstream of a pipe, hiding its exit\n>     code, thus don't actually checking that 'git for-each-ref' exits\n>     with error code.\n> \n>   - They check the error message in a rather roundabout way.\n> \n> Ensure that 'git for-each-ref' exits with an error code using the\n> 'test_must_fail' helper function, and check its error message by\n> grepping its saved standard error.\n\nYeah, this looks much nicer than the original.\n\n>  for i in \"--perl --shell\" \"-s --python\" \"--python --tcl\" \"--tcl --perl\"; do\n>  \ttest_expect_success \"more than one quoting style: $i\" \"\n> -\t\tgit for-each-ref $i 2>&1 | (read line &&\n> -\t\tcase \\$line in\n> -\t\t\\\"error: more than one quoting style\\\"*) : happy;;\n> -\t\t*) false\n> -\t\tesac)\n> +\t\ttest_must_fail git for-each-ref $i 2>err &&\n> +\t\tgrep '^error: more than one quoting style' err\n\nI suspect in the long run this ought to be test_i18ngrep, but since it's\nnot localized yet, it makes sense to stop here with this patch.\n\n-Peff\n"},{"id":"339472","messageId":"CAM0VKj=YPkS=YLtEz93RWS2=cvyy6LcyrBsCyBCg14ihxuKzjA@mail.gmail.com","threadId":"47822","inReplyTo":"20180213222247.GA18401@sigill.intra.peff.net","subject":"Re: [PATCH] t6300-for-each-ref: fix \"more than one quoting style\" tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-02-15T16:39:28Z","receivedAt":"2018-02-15T16:39:35Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Feb 13, 2018 at 11:22 PM, Jeff King <peff@peff.net> wrote:\n\n>>  for i in \"--perl --shell\" \"-s --python\" \"--python --tcl\" \"--tcl --perl\"; do\n>>       test_expect_success \"more than one quoting style: $i\" \"\n>> -             git for-each-ref $i 2>&1 | (read line &&\n>> -             case \\$line in\n>> -             \\\"error: more than one quoting style\\\"*) : happy;;\n>> -             *) false\n>> -             esac)\n>> +             test_must_fail git for-each-ref $i 2>err &&\n>> +             grep '^error: more than one quoting style' err\n>\n> I suspect in the long run this ought to be test_i18ngrep, but since it's\n> not localized yet, it makes sense to stop here with this patch.\n\nI thought 'git for-each-ref' is plumbing and that means that it\nshouldn't be localized.\n"},{"id":"339475","messageId":"20180215165218.GA7635@sigill.intra.peff.net","threadId":"47822","inReplyTo":"CAM0VKj=YPkS=YLtEz93RWS2=cvyy6LcyrBsCyBCg14ihxuKzjA@mail.gmail.com","subject":"Re: [PATCH] t6300-for-each-ref: fix \"more than one quoting style\" tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-15T16:52:18Z","receivedAt":"2018-02-15T16:53:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 15, 2018 at 05:39:28PM +0100, SZEDER Gábor wrote:\n\n> On Tue, Feb 13, 2018 at 11:22 PM, Jeff King <peff@peff.net> wrote:\n> \n> >>  for i in \"--perl --shell\" \"-s --python\" \"--python --tcl\" \"--tcl --perl\"; do\n> >>       test_expect_success \"more than one quoting style: $i\" \"\n> >> -             git for-each-ref $i 2>&1 | (read line &&\n> >> -             case \\$line in\n> >> -             \\\"error: more than one quoting style\\\"*) : happy;;\n> >> -             *) false\n> >> -             esac)\n> >> +             test_must_fail git for-each-ref $i 2>err &&\n> >> +             grep '^error: more than one quoting style' err\n> >\n> > I suspect in the long run this ought to be test_i18ngrep, but since it's\n> > not localized yet, it makes sense to stop here with this patch.\n> \n> I thought 'git for-each-ref' is plumbing and that means that it\n> shouldn't be localized.\n\nI always assumed stderr was mostly fair game, even for plumbing. At any\nrate, I'm willing to ignore the issue until somebody actually proposes a\npatch to translate it.\n\n-Peff\n"},{"id":"339477","messageId":"CAM0VKjmW6_3z49uVMFUpwsK5khZ_bNALDgkKkPUQdvu2PEogfQ@mail.gmail.com","threadId":"47822","inReplyTo":"20180215165218.GA7635@sigill.intra.peff.net","subject":"Re: [PATCH] t6300-for-each-ref: fix \"more than one quoting style\" tests","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-02-15T17:41:43Z","receivedAt":"2018-02-15T17:41:50Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Feb 15, 2018 at 5:52 PM, Jeff King <peff@peff.net> wrote:\n> On Thu, Feb 15, 2018 at 05:39:28PM +0100, SZEDER Gábor wrote:\n>\n>> On Tue, Feb 13, 2018 at 11:22 PM, Jeff King <peff@peff.net> wrote:\n>>\n>> >>  for i in \"--perl --shell\" \"-s --python\" \"--python --tcl\" \"--tcl --perl\"; do\n>> >>       test_expect_success \"more than one quoting style: $i\" \"\n>> >> -             git for-each-ref $i 2>&1 | (read line &&\n>> >> -             case \\$line in\n>> >> -             \\\"error: more than one quoting style\\\"*) : happy;;\n>> >> -             *) false\n>> >> -             esac)\n>> >> +             test_must_fail git for-each-ref $i 2>err &&\n>> >> +             grep '^error: more than one quoting style' err\n>> >\n>> > I suspect in the long run this ought to be test_i18ngrep, but since it's\n>> > not localized yet, it makes sense to stop here with this patch.\n>>\n>> I thought 'git for-each-ref' is plumbing and that means that it\n>> shouldn't be localized.\n>\n> I always assumed stderr was mostly fair game, even for plumbing. At any\n> rate, I'm willing to ignore the issue until somebody actually proposes a\n> patch to translate it.\n\nAh, OK, will keep that in mind.\n\nAnyway, the first GETTEXT_POISON build will fail if that error string\never gets translated but the test doesn't get updated.\n"}]}