[SSSD] [PATCH] tests: be_ptask

Jakub Hrozek jhrozek at redhat.com
Fri Nov 28 15:18:48 UTC 2014


On Thu, Nov 27, 2014 at 06:45:11PM +0100, Lukas Slebodnik wrote:
> On (20/11/14 17:41), Pavel Březina wrote:
> >On 11/18/2014 06:32 PM, Lukas Slebodnik wrote:
> >>On (18/11/14 18:26), Lukas Slebodnik wrote:
> >
> >All above issues where addressed.
> >
> >>>Very nice unit test.
> >
> >Thank you.
> >
> >>>It would be good to add few more tests with corner cases.
> >>>There aren't test for failed functions (_send, _recv)
> >>>(Attached is diff with test for error occured in be_ptask_sync_t.)
> >
> >Thank you for the diff, I have rewritten it slightly. We don't really care
> >that the tevent_req error was set to the return code of yours - that is
> >hidden to the caller. What we want to check is that the task will be
> >correctly rescheduled according to documentation.
> >
> >This test also revealed a huge bug that we are quite lucky we haven't been
> >punished for it so far. So cudos. (current patch #4).
> >
> >I added new tests for corner cases (_recv returns error is already there as
> >*reschedule_error):
> >- test_be_ptask_reschedule_null - _send returns NULL
> >- test_be_ptask_reschedule_timeout - req timed out
> >- test_be_ptask_sync_reschedule_ok - sync task returns EOK
> >- test_be_ptask_sync_reschedule_error - sync task returns error
> >
> >>>There are 6 "time_t" arguments be_ptask_create.
> >>>perriod and first_delay are tested.
> >>>enabled_delay,
> >
> >enabled_delay is tested as well.
> >
> >
> > random_offset, timeout, max_backoff are tested just with zero.
> >>>It would be good to add tests at least for timeout, max_backoff.
> >>>The most lines without code coverage are due to this two variables.
> >
> >Digging now into the implementation of the backoff feature, I believe it is
> >wrong. It seems to do what it is supposed to, however it overrides even
> >first_delay and enabled delay. We should fix that... or was it an intended
> >behaviour?
> >
> >>BTW: gcc 4.9 produces a warning in unit test.
> >>
> >>   CC       src/providers/ad_gpo_tests-dp_ptask.o
> >>src/tests/cmocka/test_be_ptask.c: In function ‘test_be_ptask_reschedule_error’:
> >>src/tests/cmocka/test_be_ptask.c:558:53: error: ‘now’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >>      assert_true(now + period <= ptask->next_execution);
> >>                                                      ^
> >>cc1: all warnings being treated as errors
> >>Makefile:16655: recipe for target 'src/tests/cmocka/test_be_ptask-test_be_ptask.o' failed
> >>
> >
> >Thanks, fixed.
> >
> >>LS
> >
> 
> >From 150e5559226aa48cf1376b1b0769c7bd092e4ef1 Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >Date: Mon, 10 Nov 2014 13:30:28 +0100
> >Subject: [PATCH 1/5] be_ptask: create a private header file
> >
> >This is done so we gain access to the be_ptask structure in unit tests.
> >---
> ACK
> 
> >From e1ca3edce0cca74097c3a0514a00d686fd936a90 Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >Date: Mon, 10 Nov 2014 13:59:27 +0100
> >Subject: [PATCH 2/5] be_ptask: handle OFFLINE_DISABLE mode before task
> > execution
> >
> >---
> ACK
> 
> >From eb3a9e90972915ce4683ed2628c6a315fbc017f0 Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >Date: Mon, 10 Nov 2014 14:26:32 +0100
> >Subject: [PATCH 3/5] be_ptask: add next_execution time to struct be_ptask
> >
> >For debugging and testing purposes.
> >---
> ACK
> 
> >From 9bd5d69ab90f66731a467eea4d4917922a28746e Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >Date: Thu, 20 Nov 2014 16:53:10 +0100
> >Subject: [PATCH 4/5] be_ptask: do not store sync ctx to _task
> >
> >The _task is an output variable of type struct be_ptask * which
> >is filled by be_ptask_create(). However, we tried to set sync ctx there
> >as a result of copy and paste error.
> >---
> ACK
> 
> >From db2a48e3855bae423e9cbf4a9e7736425a1a9569 Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >Date: Fri, 7 Nov 2014 14:47:12 +0100
> >Subject: [PATCH 5/5] tests: be_ptask
> >
> >Resolves:
> >https://fedorahosted.org/sssd/ticket/1939
> >---
> Tests passed on all distros
> http://sssd-ci.duckdns.org/logs/job/4/18/summary.html
> 
> The failed test on RHEL7 is unrelated to your patches.
> 
> ACK
> 
> LS

* master:
    5900a5d9b51779a890d0a606f7c7e1dec0d5e32c
    087da85079ec1ac51b3b97cafca19c5d243769fb
    da747252563b94069ccc7fa6882bc608bb84053d
    fa70db6004a099afb7cb55031cd7bacb9e78202e
    aff8b0e3b41644c70704b78e15501779d52b6ff4



More information about the sssd-devel mailing list