Hi!
This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests.
Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb.
I saw the failures only on Fedora 20 CI machine.
See the attached patch.
Michal
On 08/19/2015 08:26 PM, Michal Židek wrote:
Hi!
This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests.
Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb.
I saw the failures only on Fedora 20 CI machine.
See the attached patch.
Michal
Hi, I just run your patch on my F22 VM and I see some trouble here... see attachment. Petr PS: I used clean GIT and your patch, nothing else. I know that this problem is another then you solved. But it is still issue.
On 08/20/2015 01:50 PM, Petr Cech wrote:
On 08/19/2015 08:26 PM, Michal Židek wrote:
Hi!
This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests.
Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb.
I saw the failures only on Fedora 20 CI machine.
See the attached patch.
Michal
Hi, I just run your patch on my F22 VM and I see some trouble here... see attachment. Petr PS: I used clean GIT and your patch, nothing else. I know that this problem is another then you solved. But it is still issue.
I just saw 2 more fails in the CI because of the short cache timeout. The problem you see, as you said as well, is a different one and I agree it should be solved as well but so far we were able to reproduce it on your computer only and I did not see fails in the CI because of that problem. I would suggest pushing this patch (if you ACK it that is) to fix CI and look at the problem you found later.
Michal
On 08/21/2015 01:33 PM, Michal Židek wrote:
On 08/20/2015 01:50 PM, Petr Cech wrote:
On 08/19/2015 08:26 PM, Michal Židek wrote:
Hi!
This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests.
Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb.
I saw the failures only on Fedora 20 CI machine.
See the attached patch.
Michal
Hi, I just run your patch on my F22 VM and I see some trouble here... see attachment. Petr PS: I used clean GIT and your patch, nothing else. I know that this problem is another then you solved. But it is still issue.
I just saw 2 more fails in the CI because of the short cache timeout. The problem you see, as you said as well, is a different one and I agree it should be solved as well but so far we were able to reproduce it on your computer only and I did not see fails in the CI because of that problem. I would suggest pushing this patch (if you ACK it that is) to fix CI and look at the problem you found later.
Michal
OK, I agree.
There is new ticket about the mentioned bug: https://fedorahosted.org/sssd/ticket/2768
And there are CI tests: http://sssd-ci.duckdns.org/logs/job/23/57/summary.html (The failing is not connected to this patch.)
ACK
Petr
On (19/08/15 20:26), Michal Židek wrote:
Hi!
This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests.
Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb.
I saw the failures only on Fedora 20 CI machine.
See the attached patch.
Michal
From 35ee376cc0e9674b5fa9ef1c1c3cc3e67152560e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 19 Aug 2015 20:10:28 +0200 Subject: [PATCH] TESTS: ldap_id_cleanup timeouts
The one second timeout interval was sometimes too short when the tests where running under Valgrind in the CI and the entries expired too soon.
src/tests/cmocka/test_ldap_id_cleanup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/tests/cmocka/test_ldap_id_cleanup.c b/src/tests/cmocka/test_ldap_id_cleanup.c index 941427e..bdc2568 100644 --- a/src/tests/cmocka/test_ldap_id_cleanup.c +++ b/src/tests/cmocka/test_ldap_id_cleanup.c @@ -186,23 +186,26 @@ static void test_id_cleanup_exp_group(void **state) const char *empty_special_grp = "empty_gr*o/u\p(2016)"; const char *empty_grp = "empty_grp"; const char *grp = "grp";
- /* This timeout can be bigger because we will call invalidate_group
* to expire entries without waiting. */- uint64_t cache_timeout = 30;
It's better to define such variable with const modifier. So it cannot be misused for other purposes. (ideally with uppercase name of variable)
LS
On 08/27/2015 01:01 PM, Lukas Slebodnik wrote:
On (19/08/15 20:26), Michal Židek wrote:
Hi!
This is another patch to avoid failing tests in the CI (make-check-valgrind). This time the ldap_id_cleanup tests.
Looks like the one second cache timeout was too short and the tests sometimes failed because they expected the entries to be still valid for a short while after they were added to sysdb.
I saw the failures only on Fedora 20 CI machine.
See the attached patch.
Michal
From 35ee376cc0e9674b5fa9ef1c1c3cc3e67152560e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 19 Aug 2015 20:10:28 +0200 Subject: [PATCH] TESTS: ldap_id_cleanup timeouts
The one second timeout interval was sometimes too short when the tests where running under Valgrind in the CI and the entries expired too soon.
src/tests/cmocka/test_ldap_id_cleanup.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/tests/cmocka/test_ldap_id_cleanup.c b/src/tests/cmocka/test_ldap_id_cleanup.c index 941427e..bdc2568 100644 --- a/src/tests/cmocka/test_ldap_id_cleanup.c +++ b/src/tests/cmocka/test_ldap_id_cleanup.c @@ -186,23 +186,26 @@ static void test_id_cleanup_exp_group(void **state) const char *empty_special_grp = "empty_gr*o/u\p(2016)"; const char *empty_grp = "empty_grp"; const char *grp = "grp";
- /* This timeout can be bigger because we will call invalidate_group
* to expire entries without waiting. */- uint64_t cache_timeout = 30;
It's better to define such variable with const modifier. So it cannot be misused for other purposes. (ideally with uppercase name of variable)
LS
I agree. Attached is the same patch that was already ACKed, just with the changed name and added const.
Michal
On Fri, Aug 28, 2015 at 03:19:42PM +0200, Petr Cech wrote:
On 08/27/2015 05:49 PM, Michal Židek wrote:
I agree. Attached is the same patch that was already ACKed, just with the changed name and added const.
Michal
Yes, it is the same, with const.
ACK again.
Petr
* master: f02b62138466c876f6e8d6382769105f2e920d96
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Michal Židek -
Petr Cech