[SSSD] SSSD Test Suite Coverage

Jakub Hrozek jhrozek at redhat.com
Thu Apr 4 22:39:45 UTC 2013


On Thu, Apr 04, 2013 at 02:44:47AM +0530, Abhishek Singh wrote:
> Hi Jakub,
> 
> I have done requested changes and have attached the patch with this mail.
> Kindly review it.
> 
> thanks,
> 
> 

Hi Abhishek,

this is much better and we're almost there. I only have a couple of
comments before I push the patch:

1) I couldn't apply the patch cleanly. Can you make sure that the
patches you send to the list can be applied on top of origin/master? In
most cases, all you have to do is continuously rebase using "git rebase
origin/master" and fix the conflicts. 

If there are any patches that needs to be applied along with this one,
feel free to send them along in the same e-mail as another attachment.

2) because I couldn't apply the patch I only did visual review. I have
one nitpick:

+static char *get_filepath(char path[23])
+{
+    int ret;
+
+    strncpy(path, FILE_PATH, 23);

I'm not fond of the magic constant "23" used here.  I think it would be
better to either declare that path is always PATH_MAX long or pass its
size as additional parameter and declare path as "char *".

3) The get_dirfd() function assigns a DIR pointer but never calls
closedir() to free this resource. It's not a big deal in tests, but we
should still try to make the code as good as possible.

I think in this case actually a "fixture" might be a good idea. A fixture
in unit tests is a function that gets run before the test is executed
and another after the test is done. I think for this test it might be a
good idea to include a setup fixture to open the directory with opendir()
and a teardown fixture to close the handle with closedir().

Feel free to use either a global variable or the **state pointer for
storing the dir handle, both would work for me. If you need more
information on using the cmocka fixtures, some documentation is
available at:
https://cmockery.googlecode.com/svn/trunk/doc/index.html#Test_State

Feel free to ping me on #sssd if you need any help.



More information about the sssd-devel mailing list