[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