<div dir="ltr"><div>Hi,<br><br></div>Modification are made and the patch is attached with the mail. The required tests for getpwuid operations are passing. Kindly review the patch.<br><br><br><br><div><div><div><div class="gmail_extra">
<br><div class="gmail_quote">On 25 February 2014 20:40, Jakub Hrozek <span dir="ltr"><<a href="mailto:jhrozek@redhat.com" target="_blank">jhrozek@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Sat, Feb 15, 2014 at 01:04:36AM +0545, Pallavi Jha wrote:<br>
> Hi,<br>
><br>
> Attached is a patch with the additional test functions for getpwuid*<br>
> functions. I think I did not get the flow of the data in program properly.<br>
> That is why getting similar errors.<br>
><br>
> Please have a look into the patch and let me know how to improve upon it.<br>
><br>
><br>
><br>
> Thanks!<br>
> Pallavi<br>
<br>
</div></div>Hi,<br>
<br>
this test is mostly good after I did some local modifications. See a<br>
couple of comments inline.<br>
<br>
> From 5341e1331d984f48ee28118dba05d11dd4f068c4 Mon Sep 17 00:00:00 2001<br>
> From: Pallavi Jha <<a href="mailto:pallavikumarijha@gmail.com">pallavikumarijha@gmail.com</a>><br>
> Date: Mon, 27 Jan 2014 19:31:06 +0545<br>
> Subject: [PATCH] cmocka-unit-test-for-functions-getpwuid*-added<br>
<br>
[snip]<br>
<br>
> +static void mock_input_id(uint8_t *id)<br>
> +{<br>
> + will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);<br>
> + will_return(__wrap_sss_packet_get_body, id);<br>
> +}<br>
> +<br>
<br>
I had to change this function, what the responder expects is a body and<br>
body length which was previously only strlen. Now with this test we want<br>
to pass an uint32_t to SSSD, so we need to have a way to set the blen to<br>
sizeof(uint32_t). See the attached patch that applies on top of yours.<br>
<br>
> static void mock_fill_user(void)<br>
> {<br>
> /* One packet for the entry and one for num entries */<br>
> @@ -587,6 +606,343 @@ void test_nss_getpwnam_fqdn_fancy(void **state)<br>
> assert_int_equal(ret, EOK);<br>
> }<br>
><br>
> +/* Check getting cached and valid id from cache. Account callback will<br>
> + * not be called and test_nss_getpwuid_check will make sure the id is<br>
> + * the same as the test entered before starting<br>
> + */<br>
> +static int test_nss_getpwuid_check(uint32_t status, uint8_t *body, size_t blen)<br>
> +{<br>
> + struct passwd pwd;<br>
> + errno_t ret;<br>
> +<br>
> + assert_int_equal(status, EOK);<br>
> +<br>
> + ret = parse_user_packet(body, blen, &pwd);<br>
> + assert_int_equal(ret, EOK);<br>
> +<br>
> + assert_int_equal(pwd.pw_uid, 101);<br>
> + assert_int_equal(pwd.pw_gid, 401);<br>
> + assert_string_equal(pwd.pw_name, "testuser1");<br>
> + assert_string_equal(pwd.pw_shell, "/bin/sh");<br>
> + assert_string_equal(pwd.pw_passwd, "*");<br>
> + return EOK;<br>
> +}<br>
> +<br>
> +void test_nss_getpwuid(void **state)<br>
> +{<br>
> + errno_t ret;<br>
> +<br>
> + /* Prime the cache with a valid user */<br>
> + ret = sysdb_add_user(nss_test_ctx->tctx->dom,<br>
> + "testuser1", 101, 401, "test user1",<br>
> + "/home/testuser1", "/bin/sh", NULL,<br>
> + NULL, 300, 0);<br>
> + assert_int_equal(ret, EOK);<br>
> +<br>
> + uint8_t id = 101;<br>
<br>
IDs in Linux are larger, typically uid_t is used, but inside SSSD we use<br>
uint32_t.<br>
<br>
> + mock_input_id(&id);<br>
> + will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWUID);<br>
> + mock_fill_user();<br>
> +<br>
> + /* Query for that id, call a callback when command finishes */<br>
> + set_cmd_cb(test_nss_getpwuid_check);<br>
> + ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWUID,<br>
> + nss_test_ctx->nss_cmds);<br>
> + assert_int_equal(ret, EOK);<br>
> +<br>
> + /* Wait until the test finishes with EOK */<br>
> + ret = test_ev_loop(nss_test_ctx->tctx);<br>
> + assert_int_equal(ret, EOK);<br>
> +}<br>
> +<br>
<br>
The other tests were mostly good, so I'm going to omit those.<br>
<br>
> +static int test_nss_getpwuid_check_fqdn(uint32_t status,<br>
> + uint8_t *body, size_t blen)<br>
<br>
Unlike get-<something>-by-name, get-by-id doesn't support fully<br>
qualified lookups. So you can just remove the getpwuid_fqdn tests.<br>
<br>
> +{<br>
> + struct passwd pwd;<br>
> + errno_t ret;<br>
> +<br>
> + assert_int_equal(status, EOK);<br>
> +<br>
> + nss_test_ctx->cctx->rctx->domains[0].fqnames = false;<br>
> +<br>
> + ret = parse_user_packet(body, blen, &pwd);<br>
> + assert_int_equal(ret, EOK);<br>
> +<br>
> + assert_int_equal(pwd.pw_uid, 301);<br>
> + assert_int_equal(pwd.pw_gid, 755);<br>
> + assert_string_equal(pwd.pw_name, "exampleuser_fqdn@"TEST_DOM_NAME);<br>
> + assert_string_equal(pwd.pw_shell, "/bin/sh");<br>
> + return EOK;<br>
> +}<br>
> +<br>
> +void test_nss_getpwuid_fqdn(void **state)<br>
> +{<br>
> + errno_t ret;<br>
> +<br>
> + /* Prime the cache with a valid user */<br>
> + ret = sysdb_add_user(nss_test_ctx->tctx->dom,<br>
> + "exampleuser_fqdn", 301, 755, "example user",<br>
> + "/home/exampleuser", "/bin/sh", NULL,<br>
> + NULL, 300, 0);<br>
> + assert_int_equal(ret, EOK);<br>
> +<br>
> + uint8_t id = 301;<br>
<br>
btw 301 doesn't fit uinto uint8_t. But that's not really important<br>
anymore since we're changing the type to uint32_t.<br>
<br>
Good work in overall! Please check out the attached patch and squash it<br>
in if you agree. With the patch, all tests except the fqdn ones pass,<br>
but the fqdn test should be removed anyway.<br>
</blockquote></div><br></div></div></div></div></div>