From 51047ea6694f71fb1b9d2d42435c0f9684c1ea91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Tue, 26 Nov 2019 12:26:32 +0100 Subject: [PATCH 1/2] tests: fix race confition in files provider tests Lets give files provider more time to load changes from files to avoid race condition failures. The files provider tests works like this: 1. File is changed 2. inotify callback is triggered in SSSD 3. Cache is updated 4. Assertions are done This sleep is supposed to eliminated race condition between step 1 and 2 so tests do not continue before inotify callback had a chance to kick in. One second was not enough in some slow virtual environments where the test non-deterministicly failed (SSSD is starting, doing some initial tasks, so it is possible that inotify callback is delayed a little bit). Three seconds is a hopefully safe random value. --- src/tests/intg/test_files_provider.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index 9f3aad9949..795adf5551 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -451,7 +451,11 @@ def user_generator(seqnum): shell='/bin/bash') -def check_user(exp_user, delay=1.0): +def check_user(exp_user, delay=3.0): + # We need to delay the flow a little bit to ensure that inotify callback + # in SSSD had a chance to fire. One second was sometimes not enough + # on some slow virtual environments. Three seconds is large hopefully + # safe random value as there is nothing else we can do here. if delay > 0: time.sleep(delay) @@ -466,7 +470,11 @@ def group_generator(seqnum): mem=[]) -def check_group(exp_group, delay=1.0): +def check_group(exp_group, delay=3.0): + # We need to delay the flow a little bit to ensure that inotify callback + # in SSSD had a chance to fire. One second was sometimes not enough + # on some slow virtual environments. Three seconds is large hopefully + # safe random value as there is nothing else we can do here. if delay > 0: time.sleep(delay) @@ -475,7 +483,7 @@ def check_group(exp_group, delay=1.0): assert found_group == exp_group -def check_group_by_gid(exp_group, delay=1.0): +def check_group_by_gid(exp_group, delay=3.0): if delay > 0: time.sleep(delay) @@ -779,7 +787,7 @@ def test_add_remove_add_file_group(setup_gr_with_canary, files_domain_only): check_group(GROUP1) setup_gr_with_canary.groupdel(GROUP1["name"]) - time.sleep(1) + time.sleep(3) res, group = call_sssd_getgrnam(GROUP1["name"]) assert res == NssReturnCode.NOTFOUND From 04aa621eb68bdf304195da59ac253a8a5c00c967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Wed, 9 Aug 2017 07:59:41 +0200 Subject: [PATCH 2/2] INTG: Increase the sleep() time so the changes are reflected on SSSD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Those tests have been failing a lot recently and it does happen becase the time to reflect the changes on SSSD is not enough for the machine where the tests are running. There's no reasonable explanation in the code why 4 seconds is used as INTERACTIVE_TIMEOUT, neither a reasonable explanation why 2 seconds is used as the time waited in order to have those changes reflected on SSSD (neither in the code nor in the commit messages). This patch uses the most simple empiric way to determine a better value for this timeout, which was "run the tests a considerable amount of time and check that there were no failures". So, in order to avoid failures and our tests giving us more reliable information, let's give more time so the changes are reflected on SSSD. Resolves: https://pagure.io/SSSD/sssd/issue/3463 Signed-off-by: Fabiano Fidêncio --- src/tests/intg/test_enumeration.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/tests/intg/test_enumeration.py b/src/tests/intg/test_enumeration.py index c105c6df02..c25588a566 100644 --- a/src/tests/intg/test_enumeration.py +++ b/src/tests/intg/test_enumeration.py @@ -33,6 +33,15 @@ from util import * LDAP_BASE_DN = "dc=example,dc=com" + +# There is no explation neither in the code nor in the commit message that +# introduced this timeout why 4 was chosen as value. The very same happens +# with respect to why the time we should wait in order to have the changes +# reflected on SSSD is INTERACTIVE_TIMEOUT/2. +# Having INTERACTIVE_TIMEOUT/2 has been causing a lot of failures in some of +# our CI tests, so it's been changed to INTERACTIVE_TIMEOUT and the way it's +# been tested was just empirically by running or CI several times and checking +# whether a failure happened or not. INTERACTIVE_TIMEOUT = 4 @@ -412,7 +421,7 @@ def user_and_groups_rfc2307_bis(request, ldap_conn): def test_add_remove_user(ldap_conn, blank_rfc2307): """Test user addition and removal are reflected by SSSD""" e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 2001, 2000) - time.sleep(INTERACTIVE_TIMEOUT/2) + time.sleep(INTERACTIVE_TIMEOUT) # Add the user ent.assert_passwd(ent.contains_only()) ldap_conn.add_s(*e) @@ -427,7 +436,7 @@ def test_add_remove_user(ldap_conn, blank_rfc2307): def test_add_remove_group_rfc2307(ldap_conn, blank_rfc2307): """Test RFC2307 group addition and removal are reflected by SSSD""" e = ldap_ent.group(ldap_conn.ds_inst.base_dn, "group", 2001) - time.sleep(INTERACTIVE_TIMEOUT/2) + time.sleep(INTERACTIVE_TIMEOUT) # Add the group ent.assert_group(ent.contains_only()) ldap_conn.add_s(*e) @@ -442,7 +451,7 @@ def test_add_remove_group_rfc2307(ldap_conn, blank_rfc2307): def test_add_remove_group_rfc2307_bis(ldap_conn, blank_rfc2307_bis): """Test RFC2307bis group addition and removal are reflected by SSSD""" e = ldap_ent.group_bis(ldap_conn.ds_inst.base_dn, "group", 2001) - time.sleep(INTERACTIVE_TIMEOUT/2) + time.sleep(INTERACTIVE_TIMEOUT) # Add the group ent.assert_group(ent.contains_only()) ldap_conn.add_s(*e) @@ -456,7 +465,7 @@ def test_add_remove_group_rfc2307_bis(ldap_conn, blank_rfc2307_bis): def test_add_remove_membership_rfc2307(ldap_conn, user_and_group_rfc2307): """Test user membership addition and removal are reflected by SSSD""" - time.sleep(INTERACTIVE_TIMEOUT/2) + time.sleep(INTERACTIVE_TIMEOUT) # Add user to group ent.assert_group_by_name("group", dict(mem=ent.contains_only())) ldap_conn.modify_s("cn=group,ou=Groups," + ldap_conn.ds_inst.base_dn, @@ -478,7 +487,7 @@ def test_add_remove_membership_rfc2307_bis(ldap_conn, """ base_dn_bytes = ldap_conn.ds_inst.base_dn.encode('utf-8') - time.sleep(INTERACTIVE_TIMEOUT/2) + time.sleep(INTERACTIVE_TIMEOUT) # Add user to group1 ent.assert_group_by_name("group1", dict(mem=ent.contains_only())) ldap_conn.modify_s("cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn,