[SSSD] [WIP] [TEST]: Observation patch

Petr Cech pcech at redhat.com
Mon Aug 17 08:17:54 UTC 2015


On 08/17/2015 08:52 AM, Lukas Slebodnik wrote:
> >From c871c97862997df4e724647f1a0ce7297f2f059b Mon Sep 17 00:00:00 2001
>> >From: Petr Cech<pcech at redhat.com>
>> >Date: Fri, 14 Aug 2015 13:17:22 +0200
>> >Subject: [PATCH] TEST: Fix for responder_cache_req-tests
>> >
>> >Tests, that do not pass, have a problem with time. Time for writing
>> >records into database varied from time of creating a request,  that is
>> >used for filtering records internally.
>> >
>> >The patch modifies the time of creation record (adds one second to
>> >now()), so it should not be different times there.
>> >
>> >Resolves:
>> >https://fedorahosted.org/sssd/ticket/2730
>> >---
>> >src/tests/cmocka/test_responder_cache_req.c |   18 ++++++++++++------
>> >1 files changed, 12 insertions(+), 6 deletions(-)
>> >
>> >diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c
>> >index 032fe429ac88b8cc9113976329ea04837f287276..4f77fe767e016496652a97c7a73fc9e29ba7faf0 100644
>> >--- a/src/tests/cmocka/test_responder_cache_req.c
>> >+++ b/src/tests/cmocka/test_responder_cache_req.c
>> >@@ -1721,9 +1721,10 @@ void test_users_by_filter_valid(void **state)
>> >     test_ctx = talloc_get_type_abort(*state, struct cache_req_test_ctx);
>> >     test_ctx->create_user = true;
>> >
>> >+    /* set (time+1) to avoid failure request time filter */
>> >     ret = sysdb_store_user(test_ctx->tctx->dom, TEST_USER_NAME2, "pwd", 1001, 1001,
>> >                            NULL, NULL, NULL, "cn="TEST_USER_NAME2",dc=test", NULL,
>> >-                           NULL, 1000, time(NULL));
>> >+                           NULL, 1000, time(NULL)+1);
>> >     assert_int_equal(ret, EOK);
> Although, this patch fix intermitent failures
> there are few problems.
>
> The protopype of function sysdb_store_user is:
> /* this function does not check that all user members are actually present */
>
> /* if one of the basic attributes is empty ("") as opposed to NULL,
>   * this will just remove it */
>
> int sysdb_store_user(struct sss_domain_info *domain,
>                       const char *name,
>                       const char *pwd,
>                       uid_t uid, gid_t gid,
>                       const char *gecos,
>                       const char *homedir,
>                       const char *shell,
>                       const char *orig_dn,
>                       struct sysdb_attrs *attrs,
>                       char **remove_attrs,
>                       uint64_t cache_timeout,
>                       time_t now);
>
>
> and if now is 0 then we will get the current time.
> 1912     /* get transaction timestamp */
> 1913     if (!now) {
> 1914         now = time(NULL);
> 1915     }
>
> I do not understand why we shoudl set current time (now)
> to future "time(NULL)+1". I didn't check it properly, but
> if now is used as transaction timestamp (according to comment)
> it should not be from futire.
>
> LS
The initial value was time(now) and it could be simply 0, I agree with that.
(I've tried time(now) -> 0, but unfortunately it was not enough. The 
problem is elsewhere.)

The problem is reading the data. There is a filter from a certain time, 
internally used time is set to time of creating request for reading 
data. But this request is creating after inserting data. Therefore, you 
can insert a timestamp data and timestamp of request creation vary, 
especially if the machine is busy.

Completely correct solution (meaning clear) would be create a request to 
read data in the beginning of the test, then insert data and then try to 
read it. I tried this, I had complication with mock then.

Petr



More information about the sssd-devel mailing list