Hi,

Modification are made and the patch is attached with the mail. The required tests for getpwuid operations are passing. Kindly review the patch.




On 25 February 2014 20:40, Jakub Hrozek <jhrozek@redhat.com> wrote:
On Sat, Feb 15, 2014 at 01:04:36AM +0545, Pallavi Jha wrote:
> Hi,
>
> Attached is a  patch with the additional test functions for getpwuid*
> functions. I think I did not get the flow of the data in program properly.
> That is why getting similar errors.
>
> Please have a look into the patch and let me know how to improve upon it.
>
>
>
> Thanks!
> Pallavi

Hi,

this test is mostly good after I did some local modifications. See a
couple of comments inline.

> From 5341e1331d984f48ee28118dba05d11dd4f068c4 Mon Sep 17 00:00:00 2001
> From: Pallavi Jha <pallavikumarijha@gmail.com>
> Date: Mon, 27 Jan 2014 19:31:06 +0545
> Subject: [PATCH] cmocka-unit-test-for-functions-getpwuid*-added

[snip]

> +static void mock_input_id(uint8_t *id)
> +{
> +    will_return(__wrap_sss_packet_get_body, WRAP_CALL_WRAPPER);
> +    will_return(__wrap_sss_packet_get_body, id);
> +}
> +

I had to change this function, what the responder expects is a body and
body length which was previously only strlen. Now with this test we want
to pass an uint32_t to SSSD, so we need to have a way to set the blen to
sizeof(uint32_t). See the attached patch that applies on top of yours.

>  static void mock_fill_user(void)
>  {
>      /* One packet for the entry and one for num entries */
> @@ -587,6 +606,343 @@ void test_nss_getpwnam_fqdn_fancy(void **state)
>      assert_int_equal(ret, EOK);
>  }
>
> +/* Check getting cached and valid id from cache. Account callback will
> + * not be called and test_nss_getpwuid_check will make sure the id is
> + * the same as the test entered before starting
> + */
> +static int test_nss_getpwuid_check(uint32_t status, uint8_t *body, size_t blen)
> +{
> +    struct passwd pwd;
> +    errno_t ret;
> +
> +    assert_int_equal(status, EOK);
> +
> +    ret = parse_user_packet(body, blen, &pwd);
> +    assert_int_equal(ret, EOK);
> +
> +    assert_int_equal(pwd.pw_uid, 101);
> +    assert_int_equal(pwd.pw_gid, 401);
> +    assert_string_equal(pwd.pw_name, "testuser1");
> +    assert_string_equal(pwd.pw_shell, "/bin/sh");
> +    assert_string_equal(pwd.pw_passwd, "*");
> +    return EOK;
> +}
> +
> +void test_nss_getpwuid(void **state)
> +{
> +    errno_t ret;
> +
> +    /* Prime the cache with a valid user */
> +    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
> +                         "testuser1", 101, 401, "test user1",
> +                         "/home/testuser1", "/bin/sh", NULL,
> +                         NULL, 300, 0);
> +    assert_int_equal(ret, EOK);
> +
> +    uint8_t id = 101;

IDs in Linux are larger, typically uid_t is used, but inside SSSD we use
uint32_t.

> +    mock_input_id(&id);
> +    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETPWUID);
> +    mock_fill_user();
> +
> +    /* Query for that id, call a callback when command finishes */
> +    set_cmd_cb(test_nss_getpwuid_check);
> +    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETPWUID,
> +                          nss_test_ctx->nss_cmds);
> +    assert_int_equal(ret, EOK);
> +
> +    /* Wait until the test finishes with EOK */
> +    ret = test_ev_loop(nss_test_ctx->tctx);
> +    assert_int_equal(ret, EOK);
> +}
> +

The other tests were mostly good, so I'm going to omit those.

> +static int test_nss_getpwuid_check_fqdn(uint32_t status,
> +                                        uint8_t *body, size_t blen)

Unlike get-<something>-by-name, get-by-id doesn't support fully
qualified lookups. So you can just remove the getpwuid_fqdn tests.

> +{
> +    struct passwd pwd;
> +    errno_t ret;
> +
> +    assert_int_equal(status, EOK);
> +
> +    nss_test_ctx->cctx->rctx->domains[0].fqnames = false;
> +
> +    ret = parse_user_packet(body, blen, &pwd);
> +    assert_int_equal(ret, EOK);
> +
> +    assert_int_equal(pwd.pw_uid, 301);
> +    assert_int_equal(pwd.pw_gid, 755);
> +    assert_string_equal(pwd.pw_name, "exampleuser_fqdn@"TEST_DOM_NAME);
> +    assert_string_equal(pwd.pw_shell, "/bin/sh");
> +    return EOK;
> +}
> +
> +void test_nss_getpwuid_fqdn(void **state)
> +{
> +    errno_t ret;
> +
> +    /* Prime the cache with a valid user */
> +    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
> +                         "exampleuser_fqdn", 301, 755, "example user",
> +                         "/home/exampleuser", "/bin/sh", NULL,
> +                         NULL, 300, 0);
> +    assert_int_equal(ret, EOK);
> +
> +    uint8_t id = 301;

btw 301 doesn't fit uinto uint8_t. But that's not really important
anymore since we're changing the type to uint32_t.

Good work in overall! Please check out the attached patch and squash it
in if you agree. With the patch, all tests except the fqdn ones pass,
but the fqdn test should be removed anyway.