<div dir="ltr"><div><br></div><div>Hi,<br><br></div><div>I have attached a patch that contains cmocka unittest for io.c . Kindly review it.<br><br></div><div>thanks <br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Tue, Mar 19, 2013 at 12:52 AM, Jakub Hrozek <span dir="ltr">&lt;<a href="mailto:jhrozek@redhat.com" target="_blank">jhrozek@redhat.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Tue, Mar 19, 2013 at 12:31:09AM +0530, Abhishek Singh wrote:<br>
&gt; Changes done.<br>
&gt;<br>
&gt; My public repo of sssd : <a href="https://github.com/AbhishekKumarSingh/sssd" target="_blank">https://github.com/AbhishekKumarSingh/sssd</a><br>
&gt;<br>
<br>
</div>Hi Abhishek,<br>
<br>
this is much better, thank you!<br>
<br>
I only have two minor style nitpicks now:<br>
1) void test_check_if_uid_is_active_success(void ** state)<br>
We usually don&#39;t put a space after the asterisk that denotes a pointer<br>
and the variable name. In other words, we would have used:<br>
&quot;void **state&quot;<br>
<br>
2) In main(), I think it would be more readable to put a tab (in my<br>
.vimrc a tab expands to 4 spaces) before names of the tests. So this<br>
block:<br>
 96 int main(void)<br>
 97 {<br>
 98     const UnitTest tests[] = {<br>
 99     unit_test(test_check_if_uid_is_active_success),<br>
100     unit_test(test_check_if_uid_is_active_fail),<br>
101     unit_test(test_get_uid_table)<br>
102     };<br>
<br>
Would become this:<br>
 96 int main(void)<br>
 97 {<br>
 98     const UnitTest tests[] = {<br>
 99         unit_test(test_check_if_uid_is_active_success),<br>
100         unit_test(test_check_if_uid_is_active_fail),<br>
101         unit_test(test_get_uid_table)<br>
102     };<br>
<br>
Otherwise the test looks very good to me. Compiles and runs just fine.<br>
There&#39;s no need to send the patches to the list if you fix these, just<br>
push the changes to your local repo and I&#39;ll check them out.<br>
<br>
For the record, on IRC we agreed that Abhishek&#39;s next step might be to<br>
take a look at one of the modules in the src/utils subdirectory that are<br>
lacking tests. The advantage is that those modules are just utility<br>
functions that are fully synchronous so he wouldn&#39;t have to worry about<br>
tevent (yet :-)). Two candidates that come to mind are<br>
src/util/backup_file.c and src/util/io.c<br>
<br>
Great work so far Abhishek!<br>
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
sssd-devel mailing list<br>
<a href="mailto:sssd-devel@lists.fedorahosted.org">sssd-devel@lists.fedorahosted.org</a><br>
<a href="https://lists.fedorahosted.org/mailman/listinfo/sssd-devel" target="_blank">https://lists.fedorahosted.org/mailman/listinfo/sssd-devel</a><br>
</div></div></blockquote></div><br></div>