Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail
- From
Brandon Casey <casey@nrlssc.navy.mil>
- Date
- Jul 20, 2010, 16:32 UTC
- Message-ID
- <fVE942wHC3SFihkQG8AthPTKiTZtYJ9zmR2TT7F5OlkGD4IA9xPcMA@cipher.nrlssc.navy.mil>
- In-Reply-To
- <AANLkTil9jA8Dva_KqW67c1ZgWk9_a5S1rBViui8Jn0Os@mail.gmail.com>
On 07/20/2010 10:55 AM, Ævar Arnfjörð Bjarmason wrote:
Show 16 quoted lines
> On Tue, Jul 20, 2010 at 15:24, Brandon Casey <casey@nrlssc.navy.mil> wrote: > >> These two lines use the negation '!' operator to negate the result of a >> simple command. Since these commands do not contain any pipes or other >> complexities, the test_must_fail function can be used and is preferred >> since it will additionally detect termination due to a signal. > > Maybe I'm missing something, but unless `git add --dry-run` is special > in being killed due to a signal this seems misguided. We actually > prefer to use !, from t/README: > > - test_must_fail <git-command> > > Run a git command and ensure it fails in a controlled way. Use > this instead of "! <git-command>" to fail when git commands > segfault.
I think you have misunderstood the explanation of test_must_fail. The paragraph you quoted actually recommends using test_must_fail instead of "! <git-command>".
It says:
Use this instead of "! <git-command>" to fail when git commands segfault.
Or with a slight rewording:
Use test_must_fail instead of "! <git-command>" since test_must_fail will fail when <git-command> segfaults.
See, if "! <git-command>" is used, then if "<git-command>" is terminated due to some flaw in git (like a segfault), then the statement will still be interpreted as a success. When test_must_fail is used, termination due to segfault or other signal is detected, and the statement will fail.
Show 13 quoted lines
>> This was noticed because the second use of '!' does not include a space >> between the '!' and the opening parens. Ksh interprets this as follows: >> >> !(pattern-list) >> Matches anything except one of the given patterns. >> >> Ksh performs a file glob using the pattern-list and then tries to execute >> the first file in the list. If a space is added between the '!' and the >> open parens, then Ksh will not interpret it as a pattern list, but in this >> case, it is preferred to use test_must_fail, so lets do so. > > Isn't this a completely seperate thing? Was this test really the only > bit in the test suite that did "!foo" instead of "! foo" ?
This was the only instance of "!()" that was failing for me. I didn't look before, but now that I have, there is another instance of "!()" in t5541 that should be fixed. t5541 hasn't caused a problem for me because GIT_TEST_HTTPD must be set in order to enable it, and I haven't done so.
> Does the test pass for you if you just:
Yes.
Show 13 quoted lines
> @@ -281,7 +281,7 @@ add 'track-this' > EOF > > test_expect_success 'git add --dry-run --ignore-missing of > non-existing file' ' > - !(git add --dry-run --ignore-missing track-this > ignored-file >actual 2>&1) && > + ! (git add --dry-run --ignore-missing track-this > ignored-file >actual 2>&1) && > test_cmp expect actual > ' > > ?