Re: [PATCH] t/t3700: convert two uses of negation operator '!' to use test_must_fail

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/20/2010 10:55 AM, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Jul 20, 2010 at 15:24, Brandon Casey <casey@xxxxxxxxxxxxxxx> 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.

>> 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.

>     @@ -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
>      '
> 
> ?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]