Re: [PATCH] Add test-string-list.c

On Sun, Sep 5, 2010 at 2:02 AM, Jonathan Nieder [off-list ref] wrote:>

Git programs tend to start with

 #include "cache.h"

or

 #include "git-compat-util.h"

to get all the portability niceties.

Including it now.

quoted

+     print_string_list(&list, "");
+
+        int has_foo = string_list_has_string(&list, "foo");

Whitespace, declaration after statement... (see
Documentation/CodingGuidelines).

Fixed.

quoted

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

quoted

+     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?

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