[SSSD] SSSD Test Suite Coverage

Abhishek Singh abhishekkumarsingh.cse at gmail.com
Fri Apr 5 13:04:45 UTC 2013


Hi Jakub,

I have made necessary changes, please review it.

thanks,


On Fri, Apr 5, 2013 at 4:09 AM, Jakub Hrozek <jhrozek at redhat.com> wrote:

> 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.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130405/3c9a1855/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cmocka-unittest-for-find_uid-added.patch
Type: application/octet-stream
Size: 4242 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130405/3c9a1855/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cmocka-unittest-for-io-added.patch
Type: application/octet-stream
Size: 5088 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130405/3c9a1855/attachment-0001.obj>


More information about the sssd-devel mailing list