{"thread":{"id":"54356","subject":"[PATCH] t5534: split stdout and stderr redirection","startedAt":"2020-10-06T15:08:48Z","lastAt":"2020-10-07T16:21:36Z","messageCount":4,"participants":["Đoàn Trần Công Danh","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"406992","messageId":"20201006150818.15698-1-congdanhqx@gmail.com","threadId":"54356","inReplyTo":null,"subject":"[PATCH] t5534: split stdout and stderr redirection","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-10-06T15:08:18Z","receivedAt":"2020-10-06T15:08:48Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On atomic pushing failure with GnuPG, we expect a very specific output\nin stdout due to `--porcelain` switch.\n\nOn such failure, we also write down some helpful hint into stderr\nin order to help user understand what happens and how to continue from\nthose failures.\n\nOn a lot of system, those hint (in stderr) will be flushed first,\nthen those messages in stdout will be flushed. In such systems, the\ncurrent test code is fine as is.\n\nHowever, we don't have such guarantee, (at least) there're some real\nsystems that writes those stream interleaved. On such systems, we may\nsee the stderr stream written in the middle of stdout stream.\n\nLet's split those stream redirection. By splitting those stream,\nthe output stream will contain exactly what we want to compare,\nthus, saving us a \"sed\" invocation.\n\nWhile we're at it, change the `test_i18ncmp` to `test_cmp` because we\nwill never translate those messages (because of `--porcelain`).\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n Arguably, I would say it's OK to change the:\n \t\n \ttest_i18ngrep ! \"gpg failed to sign\"\n\n to:\n\n \t! grep \"gpg failed to sign\"\n\n since the latter will be correct even if GIT_TEST_GETTEXT_POISON=true\n\n t/t5534-push-signed.sh | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh\nindex 7e928aff66..af0385fb89 100755\n--- a/t/t5534-push-signed.sh\n+++ b/t/t5534-push-signed.sh\n@@ -282,10 +282,9 @@ test_expect_success GPG 'failed atomic push does not execute GPG' '\n \tEOF\n \ttest_must_fail env PATH=\"$TRASH_DIRECTORY:$PATH\" git push \\\n \t\t\t--signed --atomic --porcelain \\\n-\t\t\tdst noop ff noff >out 2>&1 &&\n+\t\t\tdst noop ff noff >out 2>err &&\n \n-\ttest_i18ngrep ! \"gpg failed to sign\" out &&\n-\tsed -n -e \"/^To dst/,$ p\" out >actual &&\n+\ttest_i18ngrep ! \"gpg failed to sign\" err &&\n \tcat >expect <<-EOF &&\n \tTo dst\n \t=\trefs/heads/noop:refs/heads/noop\t[up to date]\n@@ -293,7 +292,7 @@ test_expect_success GPG 'failed atomic push does not execute GPG' '\n \t!\trefs/heads/noff:refs/heads/noff\t[rejected] (non-fast-forward)\n \tDone\n \tEOF\n-\ttest_i18ncmp expect actual\n+\ttest_cmp expect out\n '\n \n test_done\n-- \n2.28.0.849.g665beee440\n\n"},{"id":"407000","messageId":"xmqqft6r5fwb.fsf@gitster.c.googlers.com","threadId":"54356","inReplyTo":"20201006150818.15698-1-congdanhqx@gmail.com","subject":"Re: [PATCH] t5534: split stdout and stderr redirection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-06T19:11:00Z","receivedAt":"2020-10-06T19:11:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> On atomic pushing failure with GnuPG, we expect a very specific output\n> in stdout due to `--porcelain` switch.\n>\n> On such failure, we also write down some helpful hint into stderr\n> in order to help user understand what happens and how to continue from\n> those failures.\n>\n> On a lot of system, those hint (in stderr) will be flushed first,\n> then those messages in stdout will be flushed. In such systems, the\n> current test code is fine as is.\n>\n> However, we don't have such guarantee, (at least) there're some real\n> systems that writes those stream interleaved. On such systems, we may\n> see the stderr stream written in the middle of stdout stream.\n>\n> Let's split those stream redirection. By splitting those stream,\n> the output stream will contain exactly what we want to compare,\n> thus, saving us a \"sed\" invocation.\n\nMakes sense.\n\n> While we're at it, change the `test_i18ncmp` to `test_cmp` because we\n> will never translate those messages (because of `--porcelain`).\n\nGood thinking.  It would make sure that we will catch when we\naccidentally mark messages meant for --porcelain with the\ngettext-poison tests.\n\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n>  Arguably, I would say it's OK to change the:\n>  \t\n>  \ttest_i18ngrep ! \"gpg failed to sign\"\n>\n>  to:\n>\n>  \t! grep \"gpg failed to sign\"\n>\n>  since the latter will be correct even if GIT_TEST_GETTEXT_POISON=true\n\nIs it because we haven't managed to translate this particular\nmessage, or is it because we should never ever translate it perhaps\nbecause the message is meant for machine consumption?  If the\nlatter, yes, I agree with the reasoning, but I do not see a reason\nwhy this message should never be translated.\n\n>  t/t5534-push-signed.sh | 7 +++----\n>  1 file changed, 3 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh\n> index 7e928aff66..af0385fb89 100755\n> --- a/t/t5534-push-signed.sh\n> +++ b/t/t5534-push-signed.sh\n> @@ -282,10 +282,9 @@ test_expect_success GPG 'failed atomic push does not execute GPG' '\n>  \tEOF\n>  \ttest_must_fail env PATH=\"$TRASH_DIRECTORY:$PATH\" git push \\\n>  \t\t\t--signed --atomic --porcelain \\\n> -\t\t\tdst noop ff noff >out 2>&1 &&\n> +\t\t\tdst noop ff noff >out 2>err &&\n>  \n> -\ttest_i18ngrep ! \"gpg failed to sign\" out &&\n> -\tsed -n -e \"/^To dst/,$ p\" out >actual &&\n> +\ttest_i18ngrep ! \"gpg failed to sign\" err &&\n>  \tcat >expect <<-EOF &&\n>  \tTo dst\n>  \t=\trefs/heads/noop:refs/heads/noop\t[up to date]\n> @@ -293,7 +292,7 @@ test_expect_success GPG 'failed atomic push does not execute GPG' '\n>  \t!\trefs/heads/noff:refs/heads/noff\t[rejected] (non-fast-forward)\n>  \tDone\n>  \tEOF\n> -\ttest_i18ncmp expect actual\n> +\ttest_cmp expect out\n>  '\n>  \n>  test_done\n"},{"id":"407021","messageId":"20201007001551.GA10178@danh.dev","threadId":"54356","inReplyTo":"xmqqft6r5fwb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] t5534: split stdout and stderr redirection","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-10-07T00:15:51Z","receivedAt":"2020-10-07T00:15:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-10-06 12:11:00-0700, Junio C Hamano <gitster@pobox.com> wrote:\n> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n> \n> >  Arguably, I would say it's OK to change the:\n> >  \t\n> >  \ttest_i18ngrep ! \"gpg failed to sign\"\n> >\n> >  to:\n> >\n> >  \t! grep \"gpg failed to sign\"\n> >\n> >  since the latter will be correct even if GIT_TEST_GETTEXT_POISON=true\n> \n> Is it because we haven't managed to translate this particular\n> message, or is it because we should never ever translate it perhaps\n> because the message is meant for machine consumption?\n\nWe translated that message.\n\n> If the latter, yes, I agree with the reasoning,\n> but I do not see a reason why this message should never be translated.\n\nI was wrong, I thought test_i18ngrep will blindly \"return 0\", it does\ncall grep on \"test_i18ngrep !\"\n\nPlease ignore my non-sense in the foot-note.\nI guess the main part is still good, though.\n\nThanks,\n\n-- \nDanh\n"},{"id":"407069","messageId":"xmqq5z7m3t2t.fsf@gitster.c.googlers.com","threadId":"54356","inReplyTo":"20201007001551.GA10178@danh.dev","subject":"Re: [PATCH] t5534: split stdout and stderr redirection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-07T16:21:30Z","receivedAt":"2020-10-07T16:21:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> On 2020-10-06 12:11:00-0700, Junio C Hamano <gitster@pobox.com> wrote:\n>> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n>> \n>> >  Arguably, I would say it's OK to change the:\n>> >  \t\n>> >  \ttest_i18ngrep ! \"gpg failed to sign\"\n>> >\n>> >  to:\n>> >\n>> >  \t! grep \"gpg failed to sign\"\n>> >\n>> >  since the latter will be correct even if GIT_TEST_GETTEXT_POISON=true\n>> \n>> Is it because we haven't managed to translate this particular\n>> message, or is it because we should never ever translate it perhaps\n>> because the message is meant for machine consumption?\n>\n> We translated that message.\n\nIf so wouldn't the message shown under GIT_TEST_GETTEXT_POISON as\nsomething that does not even remotely resemble 'gpg failed to sign'?\n\n... Ahh, hmm, cute but true.  As long as our assertion is \"the\noutput should not have that message in it\", '! grep' would do the\njob.\n\nBut that probably is not a good example to leave for others to copy\nand paste without thinking.\n\nThanks.\n"}]}