From: Brandon Casey <casey@nrlssc.navy.mil>
Date: Tue, 17 Feb 2009 16:56:36 GMT
Subject: Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !
Message-ID: <tJ6pp2ZnBjKVBLynGfYewBaGmSehbwZNLOShEvpXmtkD3IMXRugEKg@cipher.nrlssc.navy.mil>
In-Reply-To: <200902170946.52093.trast@student.ethz.ch>

Thomas Rast wrote:
> Thomas Rast wrote:
>> Junio C Hamano wrote:
>>> Thomas Rast <trast@student.ethz.ch> writes:
>>>
>>>> +	MSG=3 test_must_fail git notes edit
>>> test_must_fail is a shell function, and we have precedence 2d60615 (tests:
>>> Avoid single-shot environment export for shell function invocation,
>>> 2009-01-26) to avoid this construct.
>> I see you took this into the pu branch (currently as 891840b).  I
>> assumed you would simply drop it, given the incompatibility?  I'd
>> rather have the slight chance of missing a segfault while git is
>> trying to execute a shell script (what are the odds that it only does
>> so for git-notes?) than be killed by an angry mob of ash users ;-)
> 
> Oh, you fixed it up locally, and the next one in the series too.
> Thanks, and sorry for the noise!

Yeah, but what's this, introduced by 94859732?

t/t3301-notes.sh line 40:
test_expect_success 'refusing to edit in refs/heads/' '
        (MSG=1 GIT_NOTES_REF=refs/heads/bogus &&
         export MSG= GIT_NOTES_REF=refs/heads/bogus &&
         test_must_fail git notes edit)
'

test_expect_success 'refusing to edit in refs/remotes/' '
        (MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&
         export MSG= GIT_NOTES_REF=refs/heads/bogus &&
         test_must_fail git notes edit)
'


Notice the 'export' lines.  Comparing it to Thomas's v2 5/5 patch, it
looks like the first line which sets the variables is correct and the
export line should just be 'export MSG GIT_NOTES_REF' in both tests.

-brandon