{"thread":{"id":"14042","subject":"[PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","startedAt":"2008-06-19T17:32:02Z","lastAt":"2008-06-19T20:39:14Z","messageCount":8,"participants":["Brandon Casey","Mike Hommey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"80336","messageId":"JvISWq5uUteLVQOl-3QkroLV8SsQufGuWrfseMUjpYY@cipher.nrlssc.navy.mil","threadId":"14042","inReplyTo":null,"subject":"[PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-06-19T17:32:02Z","receivedAt":"2008-06-19T17:32:02Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"When the arguments to test_must_fail() begin with a variable assignment,\ntest_must_fail() attempts to execute the variable assignment as a command.\nThis fails, and so test_must_fail returns with a successful status value\nwithout running the command it was intended to test.\n\nFor example, the following script:\n\n\t#!/bin/sh\n\ttest_must_fail () {\n\t\t\"$@\"\n\t\ttest $? -gt 0 -a $? -le 129\n\t}\n\tfoo='wo adrian'\n\ttest_must_fail foo='yo adrian' sh -c 'echo foo: $foo'\n\nalways exits zero and prints the message:\n\n\ttest.sh: line 3: foo=yo adrian: command not found\n\nTest 16 calls test_must_fail in such a way and therefore has not been\ntesting whether git 'do[es] not fire editor in the presence of conflicts'.\nFix this by reverting to the traditional negation operator '!' and accept\nthe caveat that segfault will not be detected.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nUnless you have a better work around...\n\n-brandon\n\n\n t/t7502-commit.sh |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex ed871a6..e3469e0 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -212,7 +212,9 @@ test_expect_success 'do not fire editor in the presence of conflicts' '\n \t# Must fail due to conflict\n \ttest_must_fail git cherry-pick -n master &&\n \techo \"editor not started\" >.git/result &&\n-\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n+\t# We intentionally do not use test_must_fail on the next line since the\n+\t# mechanism does not work when setting environment variables inline\n+\t! GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n \ttest \"$(cat .git/result)\" = \"editor not started\"\n '\n \n-- \n1.5.5.3\n"},{"id":"80337","messageId":"20080619173932.GA32593@glandium.org","threadId":"14042","inReplyTo":"JvISWq5uUteLVQOl-3QkroLV8SsQufGuWrfseMUjpYY@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-06-19T17:39:32Z","receivedAt":"2008-06-19T17:39:32Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote:\n> -\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n> +\t# We intentionally do not use test_must_fail on the next line since the\n> +\t# mechanism does not work when setting environment variables inline\n> +\t! GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n\nDoesn't GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" test_must_fail git commit\nwork ?\n\nMike\n"},{"id":"80338","messageId":"gASAIyn4TvarEmVo9rWtEKa6eROfKXwowHxH6j05FzPARJ-CDBCHLw@cipher.nrlssc.navy.mil","threadId":"14042","inReplyTo":"20080619173932.GA32593@glandium.org","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-06-19T17:44:23Z","receivedAt":"2008-06-19T17:44:23Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Mike Hommey wrote:\n> On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote:\n>> -\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n>> +\t# We intentionally do not use test_must_fail on the next line since the\n>> +\t# mechanism does not work when setting environment variables inline\n>> +\t! GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n> \n> Doesn't GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" test_must_fail git commit\n> work ?\n\nThat leaves GIT_EDITOR set to the new value after the command completes.\n\n-brandon\n"},{"id":"80339","messageId":"20080619175639.GA1604@glandium.org","threadId":"14042","inReplyTo":"gASAIyn4TvarEmVo9rWtEKa6eROfKXwowHxH6j05FzPARJ-CDBCHLw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-06-19T17:56:39Z","receivedAt":"2008-06-19T17:56:39Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Jun 19, 2008 at 12:44:23PM -0500, Brandon Casey wrote:\n> Mike Hommey wrote:\n> > On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote:\n> >> -\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n> >> +\t# We intentionally do not use test_must_fail on the next line since the\n> >> +\t# mechanism does not work when setting environment variables inline\n> >> +\t! GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n> > \n> > Doesn't GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" test_must_fail git commit\n> > work ?\n> \n> That leaves GIT_EDITOR set to the new value after the command completes.\n\nIt really shouldn't.\n\nMike\n"},{"id":"80341","messageId":"frLVfPcFTL8gWg3cEzx3q-MMBWxW3Ts2EnBH2U39k5_QFQTfThtN-A@cipher.nrlssc.navy.mil","threadId":"14042","inReplyTo":"20080619175639.GA1604@glandium.org","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-06-19T18:07:09Z","receivedAt":"2008-06-19T18:07:09Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Mike Hommey wrote:\n> On Thu, Jun 19, 2008 at 12:44:23PM -0500, Brandon Casey wrote:\n>> Mike Hommey wrote:\n>>> On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote:\n>>>> -\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n>>>> +\t# We intentionally do not use test_must_fail on the next line since the\n>>>> +\t# mechanism does not work when setting environment variables inline\n>>>> +\t! GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n>>> Doesn't GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" test_must_fail git commit\n>>> work ?\n>> That leaves GIT_EDITOR set to the new value after the command completes.\n> \n> It really shouldn't.\n\nThe change you suggested is also what I tried first. Looks like it depends on the\nshell. On my system /bin/sh is a symlink to /bin/bash and that is the behavior\nI observed. On other shells the assignment is not passed to the environment of\nthe called procedure at all.\n\nHere is an example script:\n\n\t#!/bin/sh\n\ttest_must_fail () {\n\t\t\"$@\"\n\t\ttest $? -gt 0 -a $? -le 129\n\t}\n\tfoo='wo adrian'\n\tfoo='yo adrian' test_must_fail sh -c 'echo foo: $foo'\n\techo \"foo: $foo\"\n\nWhen /bin/sh is a symlink to bash, I get:\n\nfoo: yo adrian\nfoo: yo adrian\n\nIf I change #!/bin/sh to #!/bin/bash I get what is expected:\n\nfoo: yo adrian\nfoo: wo adrian\n\n#!/bin/ksh\nfoo:\nfoo: yo adrian\n\n#!/bin/dash\n\nfoo:\nfoo: yo adrian\n\n\n-brandon\n"},{"id":"80342","messageId":"-l_oe9PTh99x_C1EHrrbpmJc3l75joqJVNOzSICatedVnoX0-mlWkw@cipher.nrlssc.navy.mil","threadId":"14042","inReplyTo":"frLVfPcFTL8gWg3cEzx3q-MMBWxW3Ts2EnBH2U39k5_QFQTfThtN-A@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-06-19T18:12:02Z","receivedAt":"2008-06-19T18:12:02Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Brandon Casey wrote:\n\n> If I change #!/bin/sh to #!/bin/bash I get what is expected:\n> \n> foo: yo adrian\n> foo: wo adrian\n> \n> #!/bin/ksh\n> foo:\n> foo: yo adrian\n\nUnless I'm on solaris, then #!/bin/ksh gives me the same result\nas bash:\n\nfoo: yo adrian\nfoo: wo adrian\n\n-brandon\n"},{"id":"80349","messageId":"7vd4mduv3t.fsf@gitster.siamese.dyndns.org","threadId":"14042","inReplyTo":"gASAIyn4TvarEmVo9rWtEKa6eROfKXwowHxH6j05FzPARJ-CDBCHLw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-19T18:40:06Z","receivedAt":"2008-06-19T18:40:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> Mike Hommey wrote:\n>> On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote:\n>>> -\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n>>> +\t# We intentionally do not use test_must_fail on the next line since the\n>>> +\t# mechanism does not work when setting environment variables inline\n>>> +\t! GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n>> \n>> Doesn't GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" test_must_fail git commit\n>> work ?\n>\n> That leaves GIT_EDITOR set to the new value after the command completes.\n>\n> -brandon\n\nA Subshell?\n\n@@ -212,6 +212,7 @@ test_expect_success 'do not fire\n \t# Must fail due to conflict\n \ttest_must_fail git cherry-pick -n master &&\n \techo \"editor not started\" >.git/result &&\n-\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n+\t( GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" && export GIT_EDITOR &&\n+\t  test_must_fail git commit ) &&\n \ttest \"$(cat .git/result)\" = \"editor not started\"\n '\n"},{"id":"80365","messageId":"eL0AGIYbdnvG_hyKGyAA6ogzMbixNrhYwemCgYnHo-tYJkBD8ad0kQ@cipher.nrlssc.navy.mil","threadId":"14042","inReplyTo":"7vd4mduv3t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-06-19T20:39:14Z","receivedAt":"2008-06-19T20:39:14Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\nJunio C Hamano wrote:\n\n> A Subshell?\n> \n> @@ -212,6 +212,7 @@ test_expect_success 'do not fire\n>  \t# Must fail due to conflict\n>  \ttest_must_fail git cherry-pick -n master &&\n>  \techo \"editor not started\" >.git/result &&\n> -\ttest_must_fail GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" git commit &&\n> +\t( GIT_EDITOR=\"$(pwd)/.git/FAKE_EDITOR\" && export GIT_EDITOR &&\n> +\t  test_must_fail git commit ) &&\n>  \ttest \"$(cat .git/result)\" = \"editor not started\"\n>  '\n\nThat works and it's better.\n\n-brandon\n"}]}