[SSSD] [PATCH] tests: be_ptask

Lukas Slebodnik lslebodn at redhat.com
Thu Nov 20 17:04:46 UTC 2014


On (20/11/14 17:41), Pavel Březina 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?
>
Michal,
that's question for you because you implemented
features random_offset, max_backoff.

LS

BTW:
I can review patches next week due to higher priority task.



More information about the sssd-devel mailing list