Re: [PATCH] Add test-string-list.c
- From
Thiago Farina <tfransosi@gmail.com>
- Date
- Sep 6, 2010, 23:48 UTC
- Message-ID
- <AANLkTi=J_0XaVYUt8Umu3uW15W86wbWv-xCMuVGA7cUE@mail.gmail.com>
- In-Reply-To
- <20100905050254.GA6134@burratino>
On Sun, Sep 5, 2010 at 2:02 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:>
Show 11 quoted lines
> > Git programs tend to start with > > #include "cache.h" > > or > > #include "git-compat-util.h" > > to get all the portability niceties. >
Including it now.
Show 7 quoted lines
>> + print_string_list(&list, ""); >> + >> + int has_foo = string_list_has_string(&list, "foo"); > > Whitespace, declaration after statement... (see > Documentation/CodingGuidelines). >
Fixed.
Show 9 quoted lines
>> + if (has_foo != 1)
>> + error("List doesn't have foo.");
>
> This does not exit with nonzero status when it fails. You probably
> wanted
>
> if (bad things)
> return error("problems!");
>Fixed.
Show 8 quoted lines
>> + string_list_clear(&list, 0);
>> + if (list.nr > 0)
>> + error("List is not clear.");
>
> To make sure this example remains valid, wouldn't you want to include
> a caller in the t/ directory so it can be automatically run? (See
> t/README.)
>I read it, but I'm not sure how to do this. Maybe you could point me to an example?
Show 7 quoted lines
> Thoughts separate from the code: > > * for this to be useful as a test I think one has to sort of believe > that it can break. That is, a test of something this basic (which > is already demonstrated and exercised by code throughout git, after > all) would tend to be especially devious. >
It is basic, so anyone can read it, and say "Oh, I can do this.". Looking through the code maybe not so easy.
It can be expanded later by anyone to test many other things though. So, why not? (Is it so bad to not have it at all?).