[SSSD] [PATCH] Added unit test for sysdb_ssh.c

Jan Zelený jzeleny at redhat.com
Thu Jul 26 12:25:08 UTC 2012


Dne středa 25 července 2012 13:21:01, Michal Zidek napsal(a):
> Unit test for src/db/sysdb_ssh.c

Nack,
you are cleaning the directory TESTS_PATH but what if the directory existed 
(and maybe wasn't empty) before? The least you should do is to exclude the 
return code from ret variable.

Please use the SSSD_MAIN_OPTS macro in the variable long_options instead 
defining the debug-level manually there?

An important code-style comment: please declare all variables at the beginning 
of functions. There are only a handful exceptions of this rule in the code.

Nitpick: you are missing space before SYSDB_OBJECTCLASS in 
store_one_host_test()

Consider checking results of all functions (for example function like 
talloc_strdup(), talloc_zero(), sysdb_attrs_add_string(), ...) or at least 
those on which something else is depending (like allocation of structures) so 
we don't get segfaults. I'm not stating this as a hard requirement sice we 
don't have it in most of places in sysdb test suite, but it would be nice to 
make the coding style a little bit more defensive.

In delete_nonexisting_host_test():
nonexisting -> nonexistent
Deleting -> Deletion


Other than that the patch is ok.

Thanks
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120726/91882c25/attachment.sig>


More information about the sssd-devel mailing list