# [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables 8 messages from 2008-06-19 to 2008-06-19. Participants: Brandon Casey, Mike Hommey, Junio C Hamano. Thread: https://gitlist.dev/t/14042 ## Brandon Casey, 2008-06-19 17:32 Subject: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: URL: https://gitlist.dev/e/JvISWq5uUteLVQOl-3QkroLV8SsQufGuWrfseMUjpYY%40cipher.nrlssc.navy.mil ``` When the arguments to test_must_fail() begin with a variable assignment, test_must_fail() attempts to execute the variable assignment as a command. This fails, and so test_must_fail returns with a successful status value without running the command it was intended to test. For example, the following script: #!/bin/sh test_must_fail () { "$@" test $? -gt 0 -a $? -le 129 } foo='wo adrian' test_must_fail foo='yo adrian' sh -c 'echo foo: $foo' always exits zero and prints the message: test.sh: line 3: foo=yo adrian: command not found Test 16 calls test_must_fail in such a way and therefore has not been testing whether git 'do[es] not fire editor in the presence of conflicts'. Fix this by reverting to the traditional negation operator '!' and accept the caveat that segfault will not be detected. Signed-off-by: Brandon Casey --- Unless you have a better work around... -brandon t/t7502-commit.sh | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index ed871a6..e3469e0 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -212,7 +212,9 @@ test_expect_success 'do not fire editor in the presence of conflicts' ' # Must fail due to conflict test_must_fail git cherry-pick -n master && echo "editor not started" >.git/result && - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && + # We intentionally do not use test_must_fail on the next line since the + # mechanism does not work when setting environment variables inline + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && test "$(cat .git/result)" = "editor not started" ' -- 1.5.5.3 ``` ## Mike Hommey, 2008-06-19 17:39 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: <20080619173932.GA32593@glandium.org> URL: https://gitlist.dev/e/20080619173932.GA32593%40glandium.org In-Reply-To: ``` On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote: > - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && > + # We intentionally do not use test_must_fail on the next line since the > + # mechanism does not work when setting environment variables inline > + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && Doesn't GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" test_must_fail git commit work ? Mike ``` ## Brandon Casey, 2008-06-19 17:44 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: URL: https://gitlist.dev/e/gASAIyn4TvarEmVo9rWtEKa6eROfKXwowHxH6j05FzPARJ-CDBCHLw%40cipher.nrlssc.navy.mil In-Reply-To: <20080619173932.GA32593@glandium.org> ``` Mike Hommey wrote: > On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote: >> - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && >> + # We intentionally do not use test_must_fail on the next line since the >> + # mechanism does not work when setting environment variables inline >> + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && > > Doesn't GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" test_must_fail git commit > work ? That leaves GIT_EDITOR set to the new value after the command completes. -brandon ``` ## Mike Hommey, 2008-06-19 17:56 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: <20080619175639.GA1604@glandium.org> URL: https://gitlist.dev/e/20080619175639.GA1604%40glandium.org In-Reply-To: ``` On Thu, Jun 19, 2008 at 12:44:23PM -0500, Brandon Casey wrote: > Mike Hommey wrote: > > On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote: > >> - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && > >> + # We intentionally do not use test_must_fail on the next line since the > >> + # mechanism does not work when setting environment variables inline > >> + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && > > > > Doesn't GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" test_must_fail git commit > > work ? > > That leaves GIT_EDITOR set to the new value after the command completes. It really shouldn't. Mike ``` ## Brandon Casey, 2008-06-19 18:07 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: URL: https://gitlist.dev/e/frLVfPcFTL8gWg3cEzx3q-MMBWxW3Ts2EnBH2U39k5_QFQTfThtN-A%40cipher.nrlssc.navy.mil In-Reply-To: <20080619175639.GA1604@glandium.org> ``` Mike Hommey wrote: > On Thu, Jun 19, 2008 at 12:44:23PM -0500, Brandon Casey wrote: >> Mike Hommey wrote: >>> On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote: >>>> - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && >>>> + # We intentionally do not use test_must_fail on the next line since the >>>> + # mechanism does not work when setting environment variables inline >>>> + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && >>> Doesn't GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" test_must_fail git commit >>> work ? >> That leaves GIT_EDITOR set to the new value after the command completes. > > It really shouldn't. The change you suggested is also what I tried first. Looks like it depends on the shell. On my system /bin/sh is a symlink to /bin/bash and that is the behavior I observed. On other shells the assignment is not passed to the environment of the called procedure at all. Here is an example script: #!/bin/sh test_must_fail () { "$@" test $? -gt 0 -a $? -le 129 } foo='wo adrian' foo='yo adrian' test_must_fail sh -c 'echo foo: $foo' echo "foo: $foo" When /bin/sh is a symlink to bash, I get: foo: yo adrian foo: yo adrian If I change #!/bin/sh to #!/bin/bash I get what is expected: foo: yo adrian foo: wo adrian #!/bin/ksh foo: foo: yo adrian #!/bin/dash foo: foo: yo adrian -brandon ``` ## Brandon Casey, 2008-06-19 18:12 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: <-l_oe9PTh99x_C1EHrrbpmJc3l75joqJVNOzSICatedVnoX0-mlWkw@cipher.nrlssc.navy.mil> URL: https://gitlist.dev/e/-l_oe9PTh99x_C1EHrrbpmJc3l75joqJVNOzSICatedVnoX0-mlWkw%40cipher.nrlssc.navy.mil In-Reply-To: ``` Brandon Casey wrote: > If I change #!/bin/sh to #!/bin/bash I get what is expected: > > foo: yo adrian > foo: wo adrian > > #!/bin/ksh > foo: > foo: yo adrian Unless I'm on solaris, then #!/bin/ksh gives me the same result as bash: foo: yo adrian foo: wo adrian -brandon ``` ## Junio C Hamano, 2008-06-19 18:40 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: <7vd4mduv3t.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd4mduv3t.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Brandon Casey writes: > Mike Hommey wrote: >> On Thu, Jun 19, 2008 at 12:32:02PM -0500, Brandon Casey wrote: >>> - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && >>> + # We intentionally do not use test_must_fail on the next line since the >>> + # mechanism does not work when setting environment variables inline >>> + ! GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && >> >> Doesn't GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" test_must_fail git commit >> work ? > > That leaves GIT_EDITOR set to the new value after the command completes. > > -brandon A Subshell? @@ -212,6 +212,7 @@ test_expect_success 'do not fire # Must fail due to conflict test_must_fail git cherry-pick -n master && echo "editor not started" >.git/result && - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && + ( GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" && export GIT_EDITOR && + test_must_fail git commit ) && test "$(cat .git/result)" = "editor not started" ' ``` ## Brandon Casey, 2008-06-19 20:39 Subject: Re: [PATCH] t7502-commit.sh: test_must_fail doesn't work with inline environment variables Message-ID: URL: https://gitlist.dev/e/eL0AGIYbdnvG_hyKGyAA6ogzMbixNrhYwemCgYnHo-tYJkBD8ad0kQ%40cipher.nrlssc.navy.mil In-Reply-To: <7vd4mduv3t.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano wrote: > A Subshell? > > @@ -212,6 +212,7 @@ test_expect_success 'do not fire > # Must fail due to conflict > test_must_fail git cherry-pick -n master && > echo "editor not started" >.git/result && > - test_must_fail GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" git commit && > + ( GIT_EDITOR="$(pwd)/.git/FAKE_EDITOR" && export GIT_EDITOR && > + test_must_fail git commit ) && > test "$(cat .git/result)" = "editor not started" > ' That works and it's better. -brandon ```