Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.
These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Nick
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
- create_conf_fixture(request, conf)
- create_conf_fixture(request, format_basic_conf(ldap_conn, False, True))
Actually, I wanted to do one more thing with this and forgot: name the "bis" and "enum" parameters explicitly on each invocation to make it clear what's happening. E.g.:
create_conf_fixture(request, format_basic_conf(ldap_conn, bis=False, enum=True))
Anyone in favor of/against this?
Nick
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
NOTE: These still break test_memory_cache.py as seen in the attached log file.
Here's a fresher log, with the correct line numbers:
http://sssd-ci.duckdns.org/logs/job/28/19/rhel7/ci-build-debug/ci-make-intgc...
Nick
On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
(Not a full review, just adding my ideas and impressions)
I read these patches when I tried to add tests for the failing POSIX check. That didn't work out, but at least I have a better idea about the integration tests now :-)
Most importantly, the current tests are largely using enumeration. That not wrong and we want this codepath to be tested, but it's not the default of SSSD, so we want the non-enumeration also.
I also wonder if instead of bool-like parameter that says if we're using rfc2307 or rfc2307bis we should have a more generic 'schema' parameter.
Both are in some way here: https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=intg_tes...
Most of the (not well formatted) patch is the schema change, you can also see ldap_schema=ad being added. The test_broken_posix_in_ad() function also shows a test without enumeration -- I think we want tests for adding/removing a user/group/membership with rfc2307(bis) schema along these lines as well.
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Nick
From d09a103901a6f86873f9510152c600a062e255ed Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 20:00:14 +0300 Subject: [PATCH 1/7] intg: Get base DN from LDAP connection object
Don't use the global LDAP_BASE_DN in integration tests and fixtures, but instead take it from the LDAP connection object (ldap_conn) passed to them explicitly. This makes the tests and fixtures a bit more modular.
src/tests/intg/ldap_test.py | 8 ++++---- src/tests/intg/test_memory_cache.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index ea114dc..0287a28 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -105,7 +105,7 @@ def create_sssd_fixture(request):
@pytest.fixture def sanity_rfc2307(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) ent_list.add_user("user3", 1003, 2003)
@@ -150,7 +150,7 @@ def sanity_rfc2307(request, ldap_conn):
@pytest.fixture def simple_rfc2307(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user('usr\\001', 181818, 181818) ent_list.add_group("group1", 181818)
@@ -181,7 +181,7 @@ def simple_rfc2307(request, ldap_conn):
@pytest.fixture def sanity_rfc2307_bis(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) ent_list.add_user("user3", 1003, 2003)
@@ -309,7 +309,7 @@ def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis):
@pytest.fixture def refresh_after_cleanup_task(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001)
ent_list.add_group_bis("group1", 2001, ["user1"])
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py index 1f4ccb0..76d85fd 100644 --- a/src/tests/intg/test_memory_cache.py +++ b/src/tests/intg/test_memory_cache.py @@ -109,7 +109,7 @@ def create_sssd_fixture(request):
def load_data_to_ldap(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) ent_list.add_user("user3", 1003, 2003)
-- 2.5.1
From 8eb90565998f43fdc6b9ea3564527620c862f7fb Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 20:13:04 +0300 Subject: [PATCH 2/7] intg: Remove _rfc2307 from function names
Remove "_rfc2307" from integration test function names for brevity.
src/tests/intg/ldap_test.py | 12 +++---- src/tests/intg/test_memory_cache.py | 70 ++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 0287a28..e359ab4 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -104,7 +104,7 @@ def create_sssd_fixture(request):
@pytest.fixture -def sanity_rfc2307(request, ldap_conn): +def sanity(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) @@ -149,7 +149,7 @@ def sanity_rfc2307(request, ldap_conn):
@pytest.fixture -def simple_rfc2307(request, ldap_conn): +def simple(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user('usr\\001', 181818, 181818) ent_list.add_group("group1", 181818) @@ -180,7 +180,7 @@ def simple_rfc2307(request, ldap_conn):
@pytest.fixture -def sanity_rfc2307_bis(request, ldap_conn): +def sanity_bis(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) @@ -238,14 +238,14 @@ def sanity_rfc2307_bis(request, ldap_conn): return None
-def test_regression_ticket2163(ldap_conn, simple_rfc2307): +def test_regression_ticket2163(ldap_conn, simple): ent.assert_passwd_by_name( 'usr\001', dict(name='usr\001', passwd='*', uid=181818, gid=181818, gecos='181818', shell='/bin/bash'))
-def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): +def test_sanity(ldap_conn, sanity): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'), dict(name='user2', passwd='*', uid=1002, gid=2002, gecos='1002', dir='/home/user2', shell='/bin/bash'), @@ -272,7 +272,7 @@ def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): grp.getgrgid(1)
-def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): +def test_sanity_bis(ldap_conn, sanity_bis): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'), dict(name='user2', passwd='*', uid=1002, gid=2002, gecos='1002', dir='/home/user2', shell='/bin/bash'), diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py index 76d85fd..1a98a53 100644 --- a/src/tests/intg/test_memory_cache.py +++ b/src/tests/intg/test_memory_cache.py @@ -131,7 +131,7 @@ def load_data_to_ldap(request, ldap_conn):
@pytest.fixture -def sanity_rfc2307(request, ldap_conn): +def sanity(request, ldap_conn): load_data_to_ldap(request, ldap_conn)
conf = unindent("""\@@ -156,7 +156,7 @@ def sanity_rfc2307(request, ldap_conn):
@pytest.fixture -def fqname_rfc2307(request, ldap_conn): +def fqname(request, ldap_conn): load_data_to_ldap(request, ldap_conn)
conf = unindent("""\@@ -182,7 +182,7 @@ def fqname_rfc2307(request, ldap_conn):
@pytest.fixture -def fqname_case_insensitive_rfc2307(request, ldap_conn): +def fqname_case_insensitive(request, ldap_conn): load_data_to_ldap(request, ldap_conn)
conf = unindent("""\@@ -208,7 +208,7 @@ def fqname_case_insensitive_rfc2307(request, ldap_conn): return None
-def test_getpwnam(ldap_conn, sanity_rfc2307): +def test_getpwnam(ldap_conn, sanity): ent.assert_passwd_by_name( 'user1', dict(name='user1', passwd='*', uid=1001, gid=2001, @@ -291,13 +291,13 @@ def test_getpwnam(ldap_conn, sanity_rfc2307): gecos='1023', shell='/bin/bash'))
-def test_getpwnam_with_mc(ldap_conn, sanity_rfc2307):
- test_getpwnam(ldap_conn, sanity_rfc2307)
+def test_getpwnam_with_mc(ldap_conn, sanity):
- test_getpwnam(ldap_conn, sanity) stop_sssd()
- test_getpwnam(ldap_conn, sanity_rfc2307)
- test_getpwnam(ldap_conn, sanity)
-def test_getgrnam_simple(ldap_conn, sanity_rfc2307): +def test_getgrnam_simple(ldap_conn, sanity): ent.assert_group_by_name("group1", dict(name="group1", gid=2001)) ent.assert_group_by_gid(2001, dict(name="group1", gid=2001))
@@ -317,13 +317,13 @@ def test_getgrnam_simple(ldap_conn, sanity_rfc2307): ent.assert_group_by_gid(2020, dict(name="group2x", gid=2020))
-def test_getgrnam_simple_with_mc(ldap_conn, sanity_rfc2307):
- test_getgrnam_simple(ldap_conn, sanity_rfc2307)
+def test_getgrnam_simple_with_mc(ldap_conn, sanity):
- test_getgrnam_simple(ldap_conn, sanity) stop_sssd()
- test_getgrnam_simple(ldap_conn, sanity_rfc2307)
- test_getgrnam_simple(ldap_conn, sanity)
-def test_getgrnam_membership(ldap_conn, sanity_rfc2307): +def test_getgrnam_membership(ldap_conn, sanity): ent.assert_group_by_name( "group1", dict(mem=ent.contains_only("user1", "user11", "user21"))) @@ -367,10 +367,10 @@ def test_getgrnam_membership(ldap_conn, sanity_rfc2307): dict(mem=ent.contains_only("user21", "user22", "user23")))
-def test_getgrnam_membership_with_mc(ldap_conn, sanity_rfc2307):
- test_getgrnam_membership(ldap_conn, sanity_rfc2307)
+def test_getgrnam_membership_with_mc(ldap_conn, sanity):
- test_getgrnam_membership(ldap_conn, sanity) stop_sssd()
- test_getgrnam_membership(ldap_conn, sanity_rfc2307)
- test_getgrnam_membership(ldap_conn, sanity)
def assert_user_gids_equal(user, expected_gids): @@ -385,7 +385,7 @@ def assert_user_gids_equal(user, expected_gids): )
-def test_initgroups(ldap_conn, sanity_rfc2307): +def test_initgroups(ldap_conn, sanity): assert_user_gids_equal('user1', [2000, 2001]) assert_user_gids_equal('user2', [2000, 2002]) assert_user_gids_equal('user3', [2000, 2003]) @@ -399,13 +399,13 @@ def test_initgroups(ldap_conn, sanity_rfc2307): assert_user_gids_equal('user23', [2020, 2003])
-def test_initgroups_with_mc(ldap_conn, sanity_rfc2307):
- test_initgroups(ldap_conn, sanity_rfc2307)
+def test_initgroups_with_mc(ldap_conn, sanity):
- test_initgroups(ldap_conn, sanity) stop_sssd()
- test_initgroups(ldap_conn, sanity_rfc2307)
- test_initgroups(ldap_conn, sanity)
-def test_initgroups_fqname_with_mc(ldap_conn, fqname_rfc2307): +def test_initgroups_fqname_with_mc(ldap_conn, fqname): assert_user_gids_equal('user1@LDAP', [2000, 2001]) stop_sssd() assert_user_gids_equal('user1@LDAP', [2000, 2001]) @@ -447,7 +447,7 @@ def assert_stored_last_initgroups(user1_case1, user1_case2, user1_case_last,
def test_initgroups_case_insensitive_with_mc1(ldap_conn,
fqname_case_insensitive_rfc2307):
fqname_case_insensitive):@@ -459,7 +459,7 @@ def test_initgroups_case_insensitive_with_mc1(ldap_conn,
def test_initgroups_case_insensitive_with_mc2(ldap_conn,
fqname_case_insensitive_rfc2307):
fqname_case_insensitive):@@ -471,7 +471,7 @@ def test_initgroups_case_insensitive_with_mc2(ldap_conn,
def test_initgroups_case_insensitive_with_mc3(ldap_conn,
fqname_case_insensitive_rfc2307):
fqname_case_insensitive):@@ -510,7 +510,7 @@ def run_simple_test_with_initgroups(): assert_initgroups_equal("user1", 2001, [2000, 2001])
-def test_invalidation_of_gids_after_initgroups(ldap_conn, sanity_rfc2307): +def test_invalidation_of_gids_after_initgroups(ldap_conn, sanity):
# the sssd cache was empty and not all user's group were # resolved with getgr{nm,gid}. Therefore there is a change in@@ -549,7 +549,7 @@ def test_invalidation_of_gids_after_initgroups(ldap_conn, sanity_rfc2307): grp.getgrgid(gid)
-def test_initgroups_without_change_in_membership(ldap_conn, sanity_rfc2307): +def test_initgroups_without_change_in_membership(ldap_conn, sanity):
# the sssd cache was empty and not all user's group were # resolved with getgr{nm,gid}. Therefore there is a change in@@ -615,7 +615,7 @@ def assert_missing_mc_records_for_user1(): "User user1, errno:%d" % err
-def test_invalidate_user_before_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_user_before_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -628,7 +628,7 @@ def test_invalidate_user_before_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_user_after_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_user_after_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -641,7 +641,7 @@ def test_invalidate_user_after_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_users_before_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_users_before_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -654,7 +654,7 @@ def test_invalidate_users_before_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_users_after_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_users_after_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -667,7 +667,7 @@ def test_invalidate_users_after_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_group_before_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_group_before_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -680,7 +680,7 @@ def test_invalidate_group_before_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_group_after_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_group_after_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -693,7 +693,7 @@ def test_invalidate_group_after_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_groups_before_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_groups_before_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -706,7 +706,7 @@ def test_invalidate_groups_before_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_groups_after_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_groups_after_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -719,7 +719,7 @@ def test_invalidate_groups_after_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_everything_before_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_everything_before_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ @@ -732,7 +732,7 @@ def test_invalidate_everything_before_stop(ldap_conn, sanity_rfc2307): assert_missing_mc_records_for_user1()
-def test_invalidate_everything_after_stop(ldap_conn, sanity_rfc2307): +def test_invalidate_everything_after_stop(ldap_conn, sanity): # initialize cache with full ID (res, errno, _) = sssd_id.get_user_groups("user1") assert res == sssd_id.NssReturnCode.SUCCESS, \ -- 2.5.1
From 866570e06f449981891cdc39d5c67cb677654d8e Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Mon, 28 Sep 2015 16:12:48 +0300 Subject: [PATCH 3/7] intg: Add support for specifying all user attrs
Support passing all user attributes to ldap_ent.py's user-creation functions, in integration tests.
src/tests/intg/ldap_ent.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index 30eed9d..0bac514 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -18,7 +18,7 @@ #
-def user(base_dn, uid, uidNumber, gidNumber): +def user(base_dn, uid, uidNumber, gidNumber, **kwargs): """ Generate an RFC2307(bis) user add-modlist for passing to ldap.add* """ @@ -28,13 +28,23 @@ def user(base_dn, uid, uidNumber, gidNumber): "uid=" + uid + ",ou=Users," + base_dn, [ ('objectClass', ['top', 'inetOrgPerson', 'posixAccount']),
('cn', [uidNumber]),('sn', ['User']),
('cn', [kwargs['cn'] \if 'cn' in kwargs else \uidNumber]),('sn', [kwargs['sn'] \if 'sn' in kwargs else \'User']), ('uidNumber', [uidNumber]), ('gidNumber', [gidNumber]),
('userPassword', ['Password' + uidNumber]),('homeDirectory', ['/home/' + uid]),('loginShell', ['/bin/bash']),
('userPassword', [kwargs['userPassword'] \if 'userPassword' in kwargs else \'Password' + uidNumber]),('homeDirectory', [kwargs['homeDirectory'] \if 'homeDirectory' in kwargs else \'/home/' + uid]),('loginShell', [kwargs['loginShell'] \if 'loginShell' in kwargs else \'/bin/bash']), ]@@ -86,10 +96,10 @@ class List(list): self.base_dn = base_dn
def add_user(self, uid, uidNumber, gidNumber,
base_dn=None):
base_dn=None, **kwargs): """Add an RFC2307(bis) user add-modlist.""" self.append(user(base_dn or self.base_dn,
uid, uidNumber, gidNumber))
uid, uidNumber, gidNumber, **kwargs))def add_group(self, cn, gidNumber, member_uids=[], base_dn=None):
-- 2.5.1
From 496f11e16d2949f085f5cc1cf59bfa4fd80090bc Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 20:28:58 +0300 Subject: [PATCH 4/7] intg: Split LDAP test fixtures for flexibility
Split ldap_test.py fixtures into several functions to allow for partial fixtures and direct use within tests.
src/tests/intg/ldap_test.py | 113 ++++++++++++++++++++++++++++++++------------ 1 file changed, 83 insertions(+), 30 deletions(-)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index e359ab4..d68a890 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -59,48 +59,101 @@ def ldap_conn(request, ds_inst): return ldap_conn
-def create_ldap_fixture(request, ldap_conn, ent_list):
- """Add LDAP entries and add teardown for removing them"""
- for entry in ent_list:
ldap_conn.add_s(entry[0], entry[1])- def teardown():
+def create_ldap_entries(ldap_conn, ent_list = None):
- """Add LDAP entries from ent_list"""
- if ent_list != None:
for entry in ent_list:ldap_conn.add_s(entry[0], entry[1])+def cleanup_ldap_entries(ldap_conn, ent_list = None):
- """Remove LDAP entries added by create_ldap_entries"""
- if ent_list == None:
for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):for entry in ldap_conn.search_s("ou=" + ou + "," +ldap_conn.ds_inst.base_dn,ldap.SCOPE_ONELEVEL,attrlist=[]):ldap_conn.delete_s(entry[0])- else: for entry in ent_list: ldap_conn.delete_s(entry[0])
- request.addfinalizer(teardown)
-def create_conf_fixture(request, contents):
- """Generate sssd.conf and add teardown for removing it"""
+def create_ldap_cleanup(request, ldap_conn, ent_list = None):
- """Add teardown for removing all user/group LDAP entries"""
- request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+def create_ldap_fixture(request, ldap_conn, ent_list = None):
- """Add LDAP entries and add teardown for removing them"""
- create_ldap_entries(ldap_conn, ent_list)
- create_ldap_cleanup(request, ldap_conn, ent_list)
+def create_conf_file(contents):
- """Create sssd.conf with specified contents""" conf = open(config.CONF_PATH, "w") conf.write(contents) conf.close() os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
- request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
-def create_sssd_fixture(request):
- """Start sssd and add teardown for stopping it and removing state"""
+def cleanup_conf_file():
- """Remove sssd.conf, if it exists"""
- if os.path.lexists(config.CONF_PATH):
os.unlink(config.CONF_PATH)+def create_conf_cleanup(request):
- """Add teardown for removing sssd.conf"""
- request.addfinalizer(cleanup_conf_file)
+def create_conf_fixture(request, contents):
- """
- Create sssd.conf with specified contents and add teardown for removing it
- """
- create_conf_file(contents)
- create_conf_cleanup(request)
+def create_sssd_process():
- """Start the SSSD process""" if subprocess.call(["sssd", "-D", "-f"]) != 0: raise Exception("sssd start failed")
- def teardown():
try:pid_file = open(config.PIDFILE_PATH, "r")pid = int(pid_file.read())os.kill(pid, signal.SIGTERM)while True:try:os.kill(pid, signal.SIGCONT)except:breaktime.sleep(1)except:passsubprocess.call(["sss_cache", "-E"])for path in os.listdir(config.DB_PATH):os.unlink(config.DB_PATH + "/" + path)for path in os.listdir(config.MCACHE_PATH):os.unlink(config.MCACHE_PATH + "/" + path)- request.addfinalizer(teardown)
+def cleanup_sssd_process():
- """Stop the SSSD process and remove its state"""
- try:
pid_file = open(config.PIDFILE_PATH, "r")pid = int(pid_file.read())os.kill(pid, signal.SIGTERM)while True:try:os.kill(pid, signal.SIGCONT)except:breaktime.sleep(1)- except:
pass- subprocess.call(["sss_cache", "-E"])
- for path in os.listdir(config.DB_PATH):
os.unlink(config.DB_PATH + "/" + path)- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)+def create_sssd_cleanup(request):
- """Add teardown for stopping SSSD and removing its state"""
- request.addfinalizer(cleanup_sssd_process)
+def create_sssd_fixture(request):
- """Start SSSD and add teardown for stopping it and removing its state"""
- create_sssd_process()
- create_sssd_cleanup(request)
@pytest.fixture
2.5.1
From f7b700da9e12b877cdb0d2eb24a3c0978c730ddb Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 20:46:06 +0300 Subject: [PATCH 5/7] intg: Reduce sssd.conf duplication in test_ldap.py
Use a function to generate basic sssd.conf in test_ldap.py to reduce code duplication.
src/tests/intg/ldap_test.py | 145 ++++++++++++++------------------------------ 1 file changed, 47 insertions(+), 98 deletions(-)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index d68a890..a29f701 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -91,6 +91,42 @@ def create_ldap_fixture(request, ldap_conn, ent_list = None): create_ldap_cleanup(request, ldap_conn, ent_list)
+def format_basic_conf(ldap_conn, bis, enum):
- """Format a basic SSSD configuration"""
- if bis:
schema_conf = unindent("""\ldap_schema = rfc2307bisldap_group_object_class = groupOfNames""")- else:
schema_conf = unindent("""\ldap_schema = rfc2307""")- return unindent("""\
[sssd]debug_level = 0xffffdomains = LDAPservices = nss, pam[nss]debug_level = 0xffffmemcache_timeout = 0[pam]debug_level = 0xffff[domain/LDAP]ldap_auth_disable_tls_never_use_in_production = truedebug_level = 0xffffenumerate = {enum}{schema_conf}id_provider = ldapauth_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}- """).format(**locals())
def create_conf_file(contents): """Create sssd.conf with specified contents""" conf = open(config.CONF_PATH, "w") @@ -172,31 +208,7 @@ def sanity(request, ldap_conn): ent_list.add_group("two_user_group", 2012, ["user1", "user2"]) create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]debug_level = 0xffffdomains = LDAPservices = nss, pam[nss]debug_level = 0xffffmemcache_timeout = 0[pam]debug_level = 0xffff[domain/LDAP]ldap_auth_disable_tls_never_use_in_production = truedebug_level = 0xffffenumerate = trueldap_schema = rfc2307id_provider = ldapauth_provider = ldapsudo_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}- """).format(**locals())
- create_conf_fixture(request, conf)
- create_conf_fixture(request, format_basic_conf(ldap_conn, False, True)) create_sssd_fixture(request) return None
@@ -206,28 +218,8 @@ def simple(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user('usr\\001', 181818, 181818) ent_list.add_group("group1", 181818)
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]config_file_version = 2domains = LDAPservices = nss, pam[nss][pam][domain/LDAP]ldap_auth_disable_tls_never_use_in_production = trueldap_schema = rfc2307id_provider = ldapauth_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}- """).format(**locals())
- create_conf_fixture(request, conf)
- create_conf_fixture(request, format_basic_conf(ldap_conn, False, False)) create_sssd_fixture(request) return None
@@ -260,33 +252,7 @@ def sanity_bis(request, ldap_conn): [], ["one_user_group1", "one_user_group2"])
create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]debug_level = 0xffffdomains = LDAPservices = nss, pam[nss]debug_level = 0xffffmemcache_timeout = 0[pam]debug_level = 0xffff[domain/LDAP]ldap_auth_disable_tls_never_use_in_production = truedebug_level = 0xffffenumerate = trueldap_schema = rfc2307bisldap_group_object_class = groupOfNamesid_provider = ldapauth_provider = ldapsudo_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}- """).format(**locals())
- create_conf_fixture(request, conf)
- create_conf_fixture(request, format_basic_conf(ldap_conn, True, True)) create_sssd_fixture(request) return None
@@ -370,31 +336,14 @@ def refresh_after_cleanup_task(request, ldap_conn):
create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]config_file_version = 2domains = LDAPservices = nss[nss]memcache_timeout = 0[domain/LDAP]ldap_auth_disable_tls_never_use_in_production = truedebug_level = 0xffffldap_schema = rfc2307bisldap_group_object_class = groupOfNamesid_provider = ldapauth_provider = ldapsudo_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}entry_cache_user_timeout = 1entry_cache_group_timeout = 5000ldap_purge_cache_timeout = 3- """).format(**locals())
- conf = \
format_basic_conf(ldap_conn, True, False) + \unindent("""[domain/LDAP]entry_cache_user_timeout = 1entry_cache_group_timeout = 5000ldap_purge_cache_timeout = 3""").format(**locals())-- 2.5.1
From e1feef5e13125301e0281de102f56b2301c5cc89 Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Wed, 30 Sep 2015 14:13:22 +0300 Subject: [PATCH 6/7] intg: Fix RFC2307bis group member creation
Fix creation of mixed user/group "member" attribute for RFC2307bis group entries in ldap_ent.py.
src/tests/intg/ldap_ent.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index 0bac514..1407349 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -72,20 +72,13 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): ('objectClass', ['top', 'extensibleObject', 'groupOfNames']), ('gidNumber', [gidNumber]) ]
- if len(member_uids) > 0:
attr_list.append(('member', ["uid=" + uid + ",ou=Users," + base_dn foruid in member_uids]))- if len(member_gids) > 0:
attr_list.append(('member', ["cn=" + gid + ",ou=Groups," + base_dn forgid in member_gids]))
- member_list = []
- for uid in member_uids:
member_list.append("uid=" + uid + ",ou=Users," + base_dn)- for gid in member_gids:
member_list.append("cn=" + gid + ",ou=Groups," + base_dn)- if len(member_list) > 0:
attr_list.append(('member', member_list))-- 2.5.1
From 72222b8fbc565de66874cfaa62ea0213ebf0f5a9 Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 7/7] intg: Add more LDAP tests
Add a bunch of LDAP tests.
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.
src/tests/intg/ldap_test.py | 536 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 536 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index a29f701..b20d648 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -127,6 +127,23 @@ def format_basic_conf(ldap_conn, bis, enum): """).format(**locals())
+def format_interactive_conf(ldap_conn, bis):
- """Format an SSSD configuration with all caches refreshing in 4 seconds"""
- return \
format_basic_conf(ldap_conn, bis, True) + \unindent("""[nss]memcache_timeout = 4enum_cache_timeout = 4entry_negative_timeout = 4[domain/LDAP]ldap_enumeration_refresh_timeout = 4ldap_purge_cache_timeout = 1entry_cache_timeout = 4""")def create_conf_file(contents): """Create sssd.conf with specified contents""" conf = open(config.CONF_PATH, "w") @@ -368,3 +385,522 @@ def test_refresh_after_cleanup_task(ldap_conn, refresh_after_cleanup_task): ent.assert_group_by_name( "group2", dict(mem=ent.contains_only("user1")))
+@pytest.fixture +def blank(request, ldap_conn):
- """Create blank RFC2307 directory fixture with interactive SSSD conf"""
- create_ldap_cleanup(request, ldap_conn)
- create_conf_fixture(request, format_interactive_conf(ldap_conn, False))
- create_sssd_fixture(request)
+@pytest.fixture +def blank_bis(request, ldap_conn):
- """Create blank RFC2307bis directory fixture with interactive SSSD conf"""
- create_ldap_cleanup(request, ldap_conn)
- create_conf_fixture(request, format_interactive_conf(ldap_conn, True))
- create_sssd_fixture(request)
+@pytest.fixture +def user_and_group(request, ldap_conn):
- """
- Create an RFC2307 directory fixture with interactive SSSD conf,
- one user and one group
- """
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user", 1001, 2000)
- ent_list.add_group("group", 2001)
- create_ldap_fixture(request, ldap_conn, ent_list)
- create_conf_fixture(request, format_interactive_conf(ldap_conn, False))
- create_sssd_fixture(request)
- return None
+@pytest.fixture +def user_and_groups_bis(request, ldap_conn):
- """
- Create an RFC2307bis directory fixture with interactive SSSD conf,
- one user and two groups
- """
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user", 1001, 2000)
- ent_list.add_group_bis("group1", 2001)
- ent_list.add_group_bis("group2", 2002)
- create_ldap_fixture(request, ldap_conn, ent_list)
- create_conf_fixture(request, format_interactive_conf(ldap_conn, True))
- create_sssd_fixture(request)
- return None
+def test_add_remove_user(ldap_conn, blank):
- """Test user addition and removal are reflected by SSSD"""
- e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 1001, 2000)
- time.sleep(2)
- # Add the user
- ent.assert_passwd(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
- time.sleep(4)
- ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001)))
- # Remove the user
- ldap_conn.delete_s(e[0])
- ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001)))
- time.sleep(4)
- ent.assert_passwd(ent.contains_only())
+def test_add_remove_group(ldap_conn, blank):
- """Test RFC2307 group addition and removal are reflected by SSSD"""
- e = ldap_ent.group(ldap_conn.ds_inst.base_dn, "group", 2001)
- time.sleep(2)
- # Add the group
- ent.assert_group(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_group(ent.contains_only())
- time.sleep(4)
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- # Remove the group
- ldap_conn.delete_s(e[0])
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- time.sleep(4)
- ent.assert_group(ent.contains_only())
+def test_add_remove_group_bis(ldap_conn, blank_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(2)
- # Add the group
- ent.assert_group(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_group(ent.contains_only())
- time.sleep(4)
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- # Remove the group
- ldap_conn.delete_s(e[0])
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- time.sleep(4)
- ent.assert_group(ent.contains_only())
+def test_add_remove_membership(ldap_conn, user_and_group):
- """Test user membership addition and removal are reflected by SSSD"""
- time.sleep(2)
- # 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,
[(ldap.MOD_REPLACE, "memberUid", "user")])- ent.assert_group_by_name("group", dict(mem = ent.contains_only()))
- time.sleep(4)
- ent.assert_group_by_name("group", dict(mem = ent.contains_only("user")))
- # Remove user from group
- ldap_conn.modify_s("cn=group,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_DELETE, "memberUid", None)])- ent.assert_group_by_name("group", dict(mem = ent.contains_only("user")))
- time.sleep(4)
- ent.assert_group_by_name("group", dict(mem = ent.contains_only()))
+def test_add_remove_membership_bis(ldap_conn, user_and_groups_bis):
- """
- Test user and group membership addition and removal are reflected by SSSD,
- with RFC2307bis schema
- """
- time.sleep(2)
- # 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,
[(ldap.MOD_REPLACE, "member","uid=user,ou=Users," + ldap_conn.ds_inst.base_dn)])- ent.assert_group_by_name("group1", dict(mem = ent.contains_only()))
- time.sleep(4)
- ent.assert_group_by_name("group1", dict(mem = ent.contains_only("user")))
- # Add group1 to group2
- ldap_conn.modify_s("cn=group2,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_REPLACE, "member","cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn)])- ent.assert_group_by_name("group2", dict(mem = ent.contains_only()))
- time.sleep(4)
- ent.assert_group_by_name("group2", dict(mem = ent.contains_only("user")))
- # Remove group1 from group2
- ldap_conn.modify_s("cn=group2,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_DELETE, "member", None)])- ent.assert_group_by_name("group2", dict(mem = ent.contains_only("user")))
- time.sleep(4)
- ent.assert_group_by_name("group2", dict(mem = ent.contains_only()))
- # Remove user from group1
- ldap_conn.modify_s("cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_DELETE, "member", None)])- ent.assert_group_by_name("group1", dict(mem = ent.contains_only("user")))
- time.sleep(4)
- ent.assert_group_by_name("group1", dict(mem = ent.contains_only()))
+@pytest.fixture +def blank(request, ldap_conn):
- """Create blank RFC2307 directory fixture with interactive SSSD conf"""
- create_ldap_cleanup(request, ldap_conn)
- create_conf_fixture(request, format_interactive_conf(ldap_conn, False))
- create_sssd_fixture(request)
+@pytest.fixture +def void_conf(request):
- create_conf_cleanup(request)
+@pytest.fixture +def void_sssd(request):
- create_sssd_cleanup(request)
+@pytest.fixture +def three_users_three_groups(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_user("user2", 1002, 2002)
- ent_list.add_user("user3", 1003, 2003)
- ent_list.add_group("group1", 2001, ["user1"])
- ent_list.add_group("group2", 2002, ["user2"])
- ent_list.add_group("group3", 2003, ["user3"])
- create_ldap_fixture(request, ldap_conn, ent_list)
+def test_filter_users(request, ldap_conn, three_users_three_groups,
void_conf, void_sssd):- """Test the effect of the "filter_users" option"""
- all_users = frozenset([1, 2, 3])
- for filter_users_in_groups in [False, True]:
for filter_users in [frozenset([]),frozenset([1]),frozenset([1, 2]),frozenset([1, 2, 3])]:unfiltered_users = all_users - filter_usersfilter_users_str = ",".join(map(lambda i: "user" + str(i),filter_users))conf = \format_basic_conf(ldap_conn, False, True) + \unindent("""[nss]filter_users = {filter_users_str}filter_users_in_groups = {filter_users_in_groups}""").format(**locals())create_conf_file(conf)create_sssd_process()ent.assert_passwd(ent.contains_only(*map(lambda i: \dict(name = "user" + str(i), uid = 1000 + i),unfiltered_users)))ent.assert_group(ent.contains_only(*map(lambda i: \dict(name = "group" + str(i),gid = 2000 + i,mem = ent.contains_only() \if filter_users_in_groups and \i in filter_users else \ent.contains_only("user" + str(i))),all_users)))cleanup_sssd_process()cleanup_conf_file()+def test_filter_groups(request, ldap_conn, three_users_three_groups,
void_conf, void_sssd):- """Test the effect of the "filter_groups" option with RFC2307 groups"""
- all_groups = frozenset([1, 2, 3])
- for filter_groups in [frozenset([]),
frozenset([1]),frozenset([1, 2]),frozenset([1, 2, 3])]:unfiltered_groups = all_groups - filter_groupsfilter_groups_str = ",".join(map(lambda i: "group" + str(i),filter_groups))conf = \format_basic_conf(ldap_conn, False, True) + \unindent("""[nss]filter_groups = {filter_groups_str}""").format(**locals())create_conf_file(conf)create_sssd_process()ent.assert_group(ent.contains_only(*map(lambda i: \dict(name = "group" + str(i),gid = 2000 + i,mem = ent.contains_only("user" + str(i))),unfiltered_groups)))cleanup_sssd_process()cleanup_conf_file()+@pytest.fixture +def three_users_three_groups_bis(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_user("user2", 1002, 2002)
- ent_list.add_user("user3", 1003, 2003)
- ent_list.add_group_bis("group1", 2001, ["user1"])
- ent_list.add_group_bis("group2", 2002, ["user2"], ["group1"])
- ent_list.add_group_bis("group3", 2003, ["user3"], ["group2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
+def test_filter_groups_bis(request, ldap_conn, three_users_three_groups_bis,
void_conf, void_sssd):- """Test the effect of the "filter_groups" option with RFC2307bis groups"""
- all_groups = frozenset([1, 2, 3])
- for filter_groups in [frozenset([]),
frozenset([1]),frozenset([1, 2]),frozenset([1, 2, 3])]:unfiltered_groups = all_groups - filter_groupsfilter_groups_str = ",".join(map(lambda i: "group" + str(i),filter_groups))conf = \format_basic_conf(ldap_conn, True, True) + \unindent("""[nss]filter_groups = {filter_groups_str}""").format(**locals())create_conf_file(conf)create_sssd_process()ent.assert_group(ent.contains_only(*map(lambda i: \dict(name = "group" + str(i),gid = 2000 + i,mem = ent.contains_only(*map(lambda j: "user" + str(j),range(1, i + 1)))),unfiltered_groups)))cleanup_sssd_process()cleanup_conf_file()+@pytest.fixture +def override_homedir(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_homedir_A", 1001, 2001,
homeDirectory = "/home/A")- ent_list.add_user("user_with_homedir_B", 1002, 2002,
homeDirectory = "/home/B")- ent_list.add_user("user_with_empty_homedir", 1003, 2003,
homeDirectory = "")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, False, True) + \unindent("""\[nss]override_homedir = /home/B""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_override_homedir(override_homedir):
- """Test the effect of the "override_homedir" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_homedir_A", uid=1001, dir="/home/B"),dict(name="user_with_homedir_B", uid=1002, dir="/home/B"),dict(name="user_with_empty_homedir", uid=1003, dir="/home/B"))- )
+@pytest.fixture +def fallback_homedir(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_homedir_A", 1001, 2001,
homeDirectory = "/home/A")- ent_list.add_user("user_with_homedir_B", 1002, 2002,
homeDirectory = "/home/B")- ent_list.add_user("user_with_empty_homedir", 1003, 2003,
homeDirectory = "")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, False, True) + \unindent("""\[nss]fallback_homedir = /home/B""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_fallback_homedir(fallback_homedir):
- """Test the effect of the "fallback_homedir" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_homedir_A", uid=1001, dir="/home/A"),dict(name="user_with_homedir_B", uid=1002, dir="/home/B"),dict(name="user_with_empty_homedir", uid=1003, dir="/home/B"))- )
+@pytest.fixture +def override_shell(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_shell_A", 1001, 2001,
loginShell = "/bin/A")- ent_list.add_user("user_with_shell_B", 1002, 2002,
loginShell = "/bin/B")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell = "")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, False, True) + \unindent("""\[nss]override_shell = /bin/B""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_override_shell(override_shell):
- """Test the effect of the "override_shell" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_shell_A", uid=1001, shell="/bin/B"),dict(name="user_with_shell_B", uid=1002, shell="/bin/B"),dict(name="user_with_empty_shell", uid=1003, shell="/bin/B"))- )
+@pytest.fixture +def shell_fallback(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_sh_shell", 1001, 2001,
loginShell = "/bin/sh")- ent_list.add_user("user_with_not_installed_shell", 1002, 2002,
loginShell = "/bin/not_installed")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell = "")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, False, True) + \unindent("""\[nss]shell_fallback = /bin/fallbackallowed_shells = /bin/not_installed""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_shell_fallback(shell_fallback):
- """Test the effect of the "shell_fallback" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_sh_shell", uid=1001, shell="/bin/sh"),dict(name="user_with_not_installed_shell", uid=1002,shell="/bin/fallback"),dict(name="user_with_empty_shell", uid=1003, shell=""))- )
+@pytest.fixture +def default_shell(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_sh_shell", 1001, 2001,
loginShell = "/bin/sh")- ent_list.add_user("user_with_not_installed_shell", 1002, 2002,
loginShell = "/bin/not_installed")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell = "")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, False, True) + \unindent("""\[nss]default_shell = /bin/defaultallowed_shells = /bin/default, /bin/not_installedshell_fallback = /bin/fallback""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_default_shell(default_shell):
- """Test the effect of the "default_shell" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_sh_shell", uid=1001, shell="/bin/sh"),dict(name="user_with_not_installed_shell", uid=1002,shell="/bin/fallback"),dict(name="user_with_empty_shell", uid=1003,shell="/bin/default"))- )
+@pytest.fixture +def vetoed_shells(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_sh_shell", 1001, 2001,
loginShell = "/bin/sh")- ent_list.add_user("user_with_vetoed_shell", 1002, 2002,
loginShell = "/bin/vetoed")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell = "")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, False, True) + \unindent("""\[nss]default_shell = /bin/defaultvetoed_shells = /bin/vetoedshell_fallback = /bin/fallback""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_vetoed_shells(vetoed_shells):
- """Test the effect of the "vetoed_shells" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_sh_shell", uid=1001, shell="/bin/sh"),dict(name="user_with_vetoed_shell", uid=1002,shell="/bin/fallback"),dict(name="user_with_empty_shell", uid=1003,shell="/bin/default"))- )
-- 2.5.1
____________________________________________________________________________________________________________________________________________________ test_getpwnam_with_mc ____________________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 297, in test_getpwnam_with_mc test_getpwnam(ldap_conn, sanity_rfc2307) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 215, in test_getpwnam gecos='1001', shell='/bin/bash')) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/ent.py", line 218, in assert_passwd_by_name assert False, err AssertionError: 'getpwnam(): name not found: user1' ________________________________________________________________________________________________________________________________________________ test_getgrnam_simple_with_mc _________________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 323, in test_getgrnam_simple_with_mc test_getgrnam_simple(ldap_conn, sanity_rfc2307) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 301, in test_getgrnam_simple ent.assert_group_by_name("group1", dict(name="group1", gid=2001)) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/ent.py", line 378, in assert_group_by_name assert False, err AssertionError: 'getgrnam(): name not found: group1' ______________________________________________________________________________________________________________________________________________ test_getgrnam_membership_with_mc _______________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 373, in test_getgrnam_membership_with_mc test_getgrnam_membership(ldap_conn, sanity_rfc2307) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 329, in test_getgrnam_membership dict(mem=ent.contains_only("user1", "user11", "user21"))) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/ent.py", line 378, in assert_group_by_name assert False, err AssertionError: 'getgrnam(): name not found: group1' ___________________________________________________________________________________________________________________________________________________ test_initgroups_with_mc ___________________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 405, in test_initgroups_with_mc test_initgroups(ldap_conn, sanity_rfc2307) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 389, in test_initgroups assert_user_gids_equal('user1', [2000, 2001]) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 377, in assert_user_gids_equal (res, errno, gids) = sssd_id.get_user_gids(user) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/sssd_id.py", line 91, in get_user_gids pwd_user = pwd.getpwnam(user) KeyError: 'getpwnam(): name not found: user1' _______________________________________________________________________________________________________________________________________________ test_initgroups_fqname_with_mc ________________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 411, in test_initgroups_fqname_with_mc assert_user_gids_equal('user1@LDAP', [2000, 2001]) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 377, in assert_user_gids_equal (res, errno, gids) = sssd_id.get_user_gids(user) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/sssd_id.py", line 91, in get_user_gids pwd_user = pwd.getpwnam(user) KeyError: 'getpwnam(): name not found: user1@LDAP' _________________________________________________________________________________________________________________________________________ test_invalidation_of_gids_after_initgroups __________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 526, in test_invalidation_of_gids_after_initgroups gecos='1001', shell='/bin/bash')) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/ent.py", line 218, in assert_passwd_by_name assert False, err AssertionError: 'getpwnam(): name not found: user1' ________________________________________________________________________________________________________________________________________ test_initgroups_without_change_in_membership _________________________________________________________________________________________________________________________________________ Traceback (most recent call last): File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 570, in test_initgroups_without_change_in_membership run_simple_test_with_initgroups() File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/test_memory_cache.py", line 489, in run_simple_test_with_initgroups gecos='1001', shell='/bin/bash')) File "/home/nkondras/projects/fedorahosted.org/sssd/src/tests/intg/ent.py", line 218, in assert_passwd_by_name assert False, err AssertionError: 'getpwnam(): name not found: user1'
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (06/10/15 11:27), Jakub Hrozek wrote:
On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
(Not a full review, just adding my ideas and impressions)
I read these patches when I tried to add tests for the failing POSIX check. That didn't work out, but at least I have a better idea about the integration tests now :-)
Most importantly, the current tests are largely using enumeration. That not wrong and we want this codepath to be tested, but it's not the default of SSSD, so we want the non-enumeration also.
I also wonder if instead of bool-like parameter that says if we're using rfc2307 or rfc2307bis we should have a more generic 'schema' parameter.
Both are in some way here: https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=intg_tes...
Most of the (not well formatted) patch is the schema change, you can also see ldap_schema=ad being added. The test_broken_posix_in_ad() function also shows a test without enumeration -- I think we want tests for adding/removing a user/group/membership with rfc2307(bis) schema along these lines as well.
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Nick
From 8eb90565998f43fdc6b9ea3564527620c862f7fb Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 20:13:04 +0300 Subject: [PATCH 2/7] intg: Remove _rfc2307 from function names
Remove "_rfc2307" from integration test function names for brevity.
src/tests/intg/ldap_test.py | 12 +++---- src/tests/intg/test_memory_cache.py | 70 ++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 0287a28..e359ab4 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -104,7 +104,7 @@ def create_sssd_fixture(request):
@pytest.fixture -def sanity_rfc2307(request, ldap_conn): +def sanity(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) @@ -149,7 +149,7 @@ def sanity_rfc2307(request, ldap_conn):
@pytest.fixture -def simple_rfc2307(request, ldap_conn): +def simple(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user('usr\\001', 181818, 181818) ent_list.add_group("group1", 181818) @@ -180,7 +180,7 @@ def simple_rfc2307(request, ldap_conn):
@pytest.fixture -def sanity_rfc2307_bis(request, ldap_conn): +def sanity_bis(request, ldap_conn): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) @@ -238,14 +238,14 @@ def sanity_rfc2307_bis(request, ldap_conn): return None
-def test_regression_ticket2163(ldap_conn, simple_rfc2307): +def test_regression_ticket2163(ldap_conn, simple): ent.assert_passwd_by_name( 'usr\001', dict(name='usr\001', passwd='*', uid=181818, gid=181818, gecos='181818', shell='/bin/bash'))
-def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): +def test_sanity(ldap_conn, sanity): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'), dict(name='user2', passwd='*', uid=1002, gid=2002, gecos='1002', dir='/home/user2', shell='/bin/bash'), @@ -272,7 +272,7 @@ def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): grp.getgrgid(1)
-def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): +def test_sanity_bis(ldap_conn, sanity_bis): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'), dict(name='user2', passwd='*', uid=1002, gid=2002, gecos='1002', dir='/home/user2', shell='/bin/bash'), diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py index 76d85fd..1a98a53 100644 --- a/src/tests/intg/test_memory_cache.py +++ b/src/tests/intg/test_memory_cache.py @@ -131,7 +131,7 @@ def load_data_to_ldap(request, ldap_conn):
Please do not remove sanity_rfc2307 from this test. It was added intentionaly. So it's clear from the name of fixture which ldap schema is used.
If you want to remove it from ldap_test.py feel free to do it but please do not chage it in test_memory_cache.py
BTW: there are introduced new pep8 warning by your patches: 0003-intg-Add-support-for-specifying-all-user-attrs.patch sh$ git diff -U0 HEAD~..HEAD | pep8 --diff ./src/tests/intg/ldap_ent.py:31:47: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:32:57: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:34:47: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:35:57: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:39:57: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:40:67: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:42:58: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:43:68: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:45:55: E502 the backslash is redundant between brackets ./src/tests/intg/ldap_ent.py:46:65: E502 the backslash is redundant between brackets
0004-intg-Split-LDAP-test-fixtures-for-flexibility.patch ./src/tests/intg/ldap_test.py:62:44: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:62:46: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:64:17: E711 comparison to None should be 'if cond is not None:' ./src/tests/intg/ldap_test.py:69:45: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:69:47: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:71:17: E711 comparison to None should be 'if cond is None:' ./src/tests/intg/ldap_test.py:83:53: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:83:55: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:88:53: E251 unexpected spaces around keyword / parameter equals ./src/tests/intg/ldap_test.py:88:55: E251 unexpected spaces around keyword / parameter equals
0007-intg-Add-more-LDAP-tests.patch In attachement :-)
Some of warnings neen't be caught by pep8 --diff (blank lines after function ..
And now back to failing test memory cache. It works for me if I change the order of tests (mv ldap_test_.py test_xldap.py) So your test had to introduce an issue in last patch. It also constantly fails for me; so it can be related. ================================================ FAILURES ================================================ __________________________________________ test_add_remove_user __________________________________________ Traceback (most recent call last): File "/dev/shm/sssd/src/tests/intg/test_xldap.py", line 451, in test_add_remove_user ent.assert_passwd(ent.contains_only()) File "/dev/shm/sssd/src/tests/intg/ent.py", line 348, in assert_passwd assert not d, d AssertionError: list mismatch: unexpected users found: [{'dir': '/home/user', 'gecos': '1001', 'gid': 2000, 'name': 'user', 'passwd': '*', 'shell': '/bin/bash', 'uid': 1001}] ----------------------------------------- Captured stderr setup ------------------------------------------ ================================= 1 failed, 58 passed in 161.58 seconds ================================== Makefile:697: recipe for target 'intgcheck-installed' failed make: *** [intgcheck-installed] Error 1 make: Leaving directory '/dev/shm/gcc/intg/bld/src/tests/intg
LS
On 10/07/2015 10:51 AM, Lukas Slebodnik wrote:
On (06/10/15 11:27), Jakub Hrozek wrote:
On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
Remove "_rfc2307" from integration test function names for brevity.
Please do not remove sanity_rfc2307 from this test. It was added intentionaly. So it's clear from the name of fixture which ldap schema is used.
If you want to remove it from ldap_test.py feel free to do it but please do not chage it in test_memory_cache.py
Sure, no problem. Moreover, based on conversation with Jakub, I think I'll need to keep them everywhere, one way or another.
BTW: there are introduced new pep8 warning by your patches:
0007-intg-Add-more-LDAP-tests.patch In attachement :-)
Some of warnings neen't be caught by pep8 --diff (blank lines after function ..
Right, I forgot to run it again. Thank you for checking.
And now back to failing test memory cache. It works for me if I change the order of tests (mv ldap_test_.py test_xldap.py) So your test had to introduce an issue in last patch.
Yeah, my tests are obviously interfering with your tests. I hoped you might suggest the reason, since you know them and SSSD better, but I can dig it myself.
It also constantly fails for me; so it can be related. ================================================ FAILURES ================================================ __________________________________________ test_add_remove_user __________________________________________
Now, this is interesting. Is this the only test that fails? If not, which tests fail? Is the failure pattern constant? Which environment were you running them in? Was it my unchanged patch set, or was that after you switched the order?
Thank you.
Nick
On 10/06/2015 12:27 PM, Jakub Hrozek wrote:
On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
(Not a full review, just adding my ideas and impressions)
I read these patches when I tried to add tests for the failing POSIX check. That didn't work out, but at least I have a better idea about the integration tests now :-)
Was the problem with the integration test framework or with something else? If it's the former, can I offer my help?
Most importantly, the current tests are largely using enumeration. That not wrong and we want this codepath to be tested, but it's not the default of SSSD, so we want the non-enumeration also.
Alright, I'll try to use the py.test fixture parameters for this. It's a cumbersome facility, but perhaps we can use it. If that fails, I'll try something else.
I worry a bit about the doubled test runtime, though. Can we have some tests running only with either disabled or enabled enumeration?
I also wonder if instead of bool-like parameter that says if we're using rfc2307 or rfc2307bis we should have a more generic 'schema' parameter.
Both are in some way here: https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=intg_tes...
Most of the (not well formatted) patch is the schema change, you can also see ldap_schema=ad being added. The test_broken_posix_in_ad() function also shows a test without enumeration -- I think we want tests for adding/removing a user/group/membership with rfc2307(bis) schema along these lines as well.
Riiight, I completely forgot about the other schemas. Sure, we'll need something more than a boolean. However, how about we define a couple constant string variables and supply them instead of literal strings to reduce the likelihood of typos?
Nick
On Wed, Oct 07, 2015 at 04:02:32PM +0300, Nikolai Kondrashov wrote:
On 10/06/2015 12:27 PM, Jakub Hrozek wrote:
On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
(Not a full review, just adding my ideas and impressions)
I read these patches when I tried to add tests for the failing POSIX check. That didn't work out, but at least I have a better idea about the integration tests now :-)
Was the problem with the integration test framework or with something else? If it's the former, can I offer my help?
No, it was that while the code that triggered the error was in the generic LDAP provider, the bug itself manifested only in AD provider..so we'd need to wrap Samba to really test the bug.
Most importantly, the current tests are largely using enumeration. That not wrong and we want this codepath to be tested, but it's not the default of SSSD, so we want the non-enumeration also.
Alright, I'll try to use the py.test fixture parameters for this. It's a cumbersome facility, but perhaps we can use it. If that fails, I'll try something else.
I worry a bit about the doubled test runtime, though. Can we have some tests running only with either disabled or enabled enumeration?
Maybe..in general I think non-enumeration are more important so if the rigorous/essential parameters also apply to which integration tests we run, then I would prefer non-enumeration tests.
I also wonder if instead of bool-like parameter that says if we're using rfc2307 or rfc2307bis we should have a more generic 'schema' parameter.
Both are in some way here: https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=intg_tes...
Most of the (not well formatted) patch is the schema change, you can also see ldap_schema=ad being added. The test_broken_posix_in_ad() function also shows a test without enumeration -- I think we want tests for adding/removing a user/group/membership with rfc2307(bis) schema along these lines as well.
Riiight, I completely forgot about the other schemas. Sure, we'll need something more than a boolean. However, how about we define a couple constant string variables and supply them instead of literal strings to reduce the likelihood of typos?
Yes, that's even better. Would you prefer if I sent such patch (atop your patches) ?
On 10/07/2015 04:13 PM, Jakub Hrozek wrote:
On Wed, Oct 07, 2015 at 04:02:32PM +0300, Nikolai Kondrashov wrote:
On 10/06/2015 12:27 PM, Jakub Hrozek wrote:
On Wed, Sep 30, 2015 at 06:15:52PM +0300, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
(Not a full review, just adding my ideas and impressions)
I read these patches when I tried to add tests for the failing POSIX check. That didn't work out, but at least I have a better idea about the integration tests now :-)
Was the problem with the integration test framework or with something else? If it's the former, can I offer my help?
No, it was that while the code that triggered the error was in the generic LDAP provider, the bug itself manifested only in AD provider..so we'd need to wrap Samba to really test the bug.
Well, I can help with *that* :)
Most importantly, the current tests are largely using enumeration. That not wrong and we want this codepath to be tested, but it's not the default of SSSD, so we want the non-enumeration also.
Alright, I'll try to use the py.test fixture parameters for this. It's a cumbersome facility, but perhaps we can use it. If that fails, I'll try something else.
I worry a bit about the doubled test runtime, though. Can we have some tests running only with either disabled or enabled enumeration?
Maybe..in general I think non-enumeration are more important so if the rigorous/essential parameters also apply to which integration tests we run, then I would prefer non-enumeration tests.
Alright. I think we can arrange limiting the number of tests for the essential run, somehow, if need be.
Riiight, I completely forgot about the other schemas. Sure, we'll need something more than a boolean. However, how about we define a couple constant string variables and supply them instead of literal strings to reduce the likelihood of typos?
Yes, that's even better. Would you prefer if I sent such patch (atop your patches) ?
Err, no, I think I'd better do it myself, thank you :) I'll be changing these parts anyway.
Nick
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Here's another version of the patch set. It's not complete, but takes some comments into account. Namely:
* Explicitly name the new arguments for ldap_ent.user and ldap_ent.List.add_user, instead of using "kwargs". Add "gecos" too. This makes the function more suitable for Pavel Reichl's needs. * Don't remove "_rfc2307" from function names anywhere. * Use a string "schema" argument with configuration formatting functions instead of boolean "bis" argument to support other schemas. Use constants to specify the values in invocations. * Explicitly specify "enum" argument name when invoking configuration formatting functions. * Remove duplicate "blank" fixture function.
I'll continue working on the patch set. Namely adding tests without enumeration, looking for and fixing the memory cache test failures induced by the new tests, trying to move commonly used fixtures and other functions to a module so we don't copy them.
Nick
On (07/10/15 20:51), Nikolai Kondrashov wrote:
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Here's another version of the patch set. It's not complete, but takes some comments into account. Namely:
- Explicitly name the new arguments for ldap_ent.user and ldap_ent.List.add_user, instead of using "kwargs". Add "gecos" too. This makes the function more suitable for Pavel Reichl's needs.
- Don't remove "_rfc2307" from function names anywhere.
- Use a string "schema" argument with configuration formatting functions instead of boolean "bis" argument to support other schemas. Use constants to specify the values in invocations.
- Explicitly specify "enum" argument name when invoking configuration formatting functions.
- Remove duplicate "blank" fixture function.
I'll continue working on the patch set. Namely adding tests without enumeration, looking for and fixing the memory cache test failures induced by the new tests, trying to move commonly used fixtures and other functions to a module so we don't copy them.
The problem with failing test_add_remove_user after changing order with test_memorycache is fixed with this patch set. It would good to push first 5 patches so we can avoid conflicts with other patches. Then we can focus on inter test failures.
First 5 patches looks good to me. But there are still new pep8 warnings
git diff HEAD~6..HEAD~ | pep8 --diff | wc -l
Jakub, does it look good to you?
Pavel R., could these patches used with your tests for local override?
LS
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
On (08/10/15 10:33), Pavel Reichl wrote:
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
Nikolai,
please fix pep8 warnings in first 5 patches so we can push them and unblock other patches.
It might be good to send it in separate thread because it might take some time to address issues with odering of tests.
LS
On 10/08/2015 11:39 AM, Lukas Slebodnik wrote:
On (08/10/15 10:33), Pavel Reichl wrote:
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
Nikolai,
please fix pep8 warnings in first 5 patches so we can push them and unblock other patches.
I'm doing that right now. However, should we really comply with the following warnings?
E126 continuation line over-indented for hanging indent E131 continuation line unaligned for hanging indent E221 multiple spaces before operator E241 multiple spaces after ',' E251 unexpected spaces around keyword / parameter equals
These complain about whitespace which is used to try to format the code better for humans and is subjective. Do we agree with them? Or can we keep our formatting?
Nick
On (08/10/15 16:29), Nikolai Kondrashov wrote:
On 10/08/2015 11:39 AM, Lukas Slebodnik wrote:
On (08/10/15 10:33), Pavel Reichl wrote:
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
Nikolai,
please fix pep8 warnings in first 5 patches so we can push them and unblock other patches.
I'm doing that right now. However, should we really comply with the following warnings?
E126 continuation line over-indented for hanging indent E131 continuation line unaligned for hanging indent E221 multiple spaces before operator E241 multiple spaces after ',' E251 unexpected spaces around keyword / parameter equals
These complain about whitespace which is used to try to format the code better for humans and is subjective. Do we agree with them? Or can we keep our formatting?
I ask our collegues from FreeIPA team and they follow these pep8 recommendations. Most of us are not python experts so it would be good to the follow the same quidelines as they.
They do not strict require 80 characters per line.
LS
On 10/08/2015 03:48 PM, Lukas Slebodnik wrote:
On (08/10/15 16:29), Nikolai Kondrashov wrote:
On 10/08/2015 11:39 AM, Lukas Slebodnik wrote:
On (08/10/15 10:33), Pavel Reichl wrote:
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
Nikolai,
please fix pep8 warnings in first 5 patches so we can push them and unblock other patches.
I'm doing that right now. However, should we really comply with the following warnings?
E126 continuation line over-indented for hanging indent E131 continuation line unaligned for hanging indent E221 multiple spaces before operator E241 multiple spaces after ',' E251 unexpected spaces around keyword / parameter equalsThese complain about whitespace which is used to try to format the code better for humans and is subjective. Do we agree with them? Or can we keep our formatting?
I ask our collegues from FreeIPA team and they follow these pep8 recommendations. Most of us are not python experts so it would be good to the follow the same quidelines as they.
They do not strict require 80 characters per line.
+1
LS
On 10/08/2015 04:48 PM, Lukas Slebodnik wrote:
On (08/10/15 16:29), Nikolai Kondrashov wrote:
On 10/08/2015 11:39 AM, Lukas Slebodnik wrote:
On (08/10/15 10:33), Pavel Reichl wrote:
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
Nikolai,
please fix pep8 warnings in first 5 patches so we can push them and unblock other patches.
I'm doing that right now. However, should we really comply with the following warnings?
E126 continuation line over-indented for hanging indent E131 continuation line unaligned for hanging indent E221 multiple spaces before operator E241 multiple spaces after ',' E251 unexpected spaces around keyword / parameter equalsThese complain about whitespace which is used to try to format the code better for humans and is subjective. Do we agree with them? Or can we keep our formatting?
I ask our collegues from FreeIPA team and they follow these pep8 recommendations. Most of us are not python experts so it would be good to the follow the same quidelines as they.
Argh. OK.
They do not strict require 80 characters per line.
I'd like to keep that requirement.
I'll try to follow with a patch fixing the rest of the issues in the code (not just introduced with these patches).
Nick
On 10/08/2015 03:50 PM, Nikolai Kondrashov wrote:
On 10/08/2015 04:48 PM, Lukas Slebodnik wrote:
On (08/10/15 16:29), Nikolai Kondrashov wrote:
On 10/08/2015 11:39 AM, Lukas Slebodnik wrote:
On (08/10/15 10:33), Pavel Reichl wrote:
On 10/08/2015 09:32 AM, Lukas Slebodnik wrote:
Pavel R., could these patches used with your tests for local override?
LS
I'll have to amend the patches, but that was agreed to. Seems that functionality I need is there so fine by me.
Nikolai,
please fix pep8 warnings in first 5 patches so we can push them and unblock other patches.
I'm doing that right now. However, should we really comply with the following warnings?
E126 continuation line over-indented for hanging indent E131 continuation line unaligned for hanging indent E221 multiple spaces before operator E241 multiple spaces after ',' E251 unexpected spaces around keyword / parameter equalsThese complain about whitespace which is used to try to format the code better for humans and is subjective. Do we agree with them? Or can we keep our formatting?
I ask our collegues from FreeIPA team and they follow these pep8 recommendations. Most of us are not python experts so it would be good to the follow the same quidelines as they.
Argh. OK.
They do not strict require 80 characters per line.
I'd like to keep that requirement.
I'll try to follow with a patch fixing the rest of the issues in the code (not just introduced with these patches).
We have a ticket for this. It is owned by Pavel Picka but I suppose he didn't had any time to work on this. https://fedorahosted.org/sssd/ticket/2774
Nick _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 10/08/2015 10:32 AM, Lukas Slebodnik wrote:
On (07/10/15 20:51), Nikolai Kondrashov wrote:
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Here's another version of the patch set. It's not complete, but takes some comments into account. Namely:
* Explicitly name the new arguments for ldap_ent.user and ldap_ent.List.add_user, instead of using "kwargs". Add "gecos" too. This makes the function more suitable for Pavel Reichl's needs. * Don't remove "_rfc2307" from function names anywhere. * Use a string "schema" argument with configuration formatting functions instead of boolean "bis" argument to support other schemas. Use constants to specify the values in invocations. * Explicitly specify "enum" argument name when invoking configuration formatting functions. * Remove duplicate "blank" fixture function.I'll continue working on the patch set. Namely adding tests without enumeration, looking for and fixing the memory cache test failures induced by the new tests, trying to move commonly used fixtures and other functions to a module so we don't copy them.
The problem with failing test_add_remove_user after changing order with test_memorycache is fixed with this patch set.
Hmm, curioser and curioser. I have no idea what that was.
It would good to push first 5 patches so we can avoid conflicts with other patches. Then we can focus on inter test failures.
Sure, good idea.
First 5 patches looks good to me. But there are still new pep8 warnings
git diff HEAD~6..HEAD~ | pep8 --diff | wc -l
Yes, I was leaving that for today, forgot to mention above.
Thank you.
Nick
On 10/08/2015 02:39 PM, Nikolai Kondrashov wrote:
On 10/08/2015 10:32 AM, Lukas Slebodnik wrote:
On (07/10/15 20:51), Nikolai Kondrashov wrote:
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Here's another version of the patch set. It's not complete, but takes some comments into account. Namely:
* Explicitly name the new arguments for ldap_ent.user and ldap_ent.List.add_user, instead of using "kwargs". Add "gecos" too. This makes the function more suitable for Pavel Reichl's needs. * Don't remove "_rfc2307" from function names anywhere. * Use a string "schema" argument with configuration formatting functions instead of boolean "bis" argument to support other schemas. Use constants to specify the values in invocations. * Explicitly specify "enum" argument name when invoking configuration formatting functions. * Remove duplicate "blank" fixture function.I'll continue working on the patch set. Namely adding tests without enumeration, looking for and fixing the memory cache test failures induced by the new tests, trying to move commonly used fixtures and other functions to a module so we don't copy them.
The problem with failing test_add_remove_user after changing order with test_memorycache is fixed with this patch set.
Hmm, curioser and curioser. I have no idea what that was.
Here's another version of the patch, which is still interfering with the memory cache tests. However, I did some digging around and added a test in a separate patch which makes it easier to reproduce the issue. In this test it seems that the cache file is not invalidated with "sss_cache -E", if there was a LDAP enumeration refresh before that.
Other changes are:
* Add full PEP8 cleanup for integration tests prior to adding more tests (can submit as a separate patch if necessary, otherwise feel free to merge) * Cleaned up the PEP8 errors in the new tests.
I'll not be able to work on this anymore this week, but will resume next week. Perhaps someone will be interested to look at that new test and figure out the failure reason in the meantime.
Nick
On 10/12/2015 03:31 PM, Nikolai Kondrashov wrote:
On 10/08/2015 02:39 PM, Nikolai Kondrashov wrote:
On 10/08/2015 10:32 AM, Lukas Slebodnik wrote:
On (07/10/15 20:51), Nikolai Kondrashov wrote:
On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:
Hi everyone,
Here is a patch set fixing some things in integration tests and adding more LDAP tests:
* Adding/removing a user/group/membership with rfc2307(bis)schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.
These are pretty basic, but I think they're good for the start. Suggestions for more tests are welcome :)
NOTE: These still break test_memory_cache.py as seen in the attached log file. We need to figure out why and do something with it. Otherwise, the tests work fine.
Here's another version of the patch set. It's not complete, but takes some comments into account. Namely:
* Explicitly name the new arguments for ldap_ent.user and ldap_ent.List.add_user, instead of using "kwargs". Add "gecos"too. This makes the function more suitable for Pavel Reichl's needs. * Don't remove "_rfc2307" from function names anywhere. * Use a string "schema" argument with configuration formatting functions instead of boolean "bis" argument to support other schemas. Use constants to specify the values in invocations. * Explicitly specify "enum" argument name when invoking configuration formatting functions. * Remove duplicate "blank" fixture function.
I'll continue working on the patch set. Namely adding tests without enumeration, looking for and fixing the memory cache test failures induced by the new tests, trying to move commonly used fixtures and other functions to a module so we don't copy them.
The problem with failing test_add_remove_user after changing order with test_memorycache is fixed with this patch set.
Hmm, curioser and curioser. I have no idea what that was.
Here's another version of the patch, which is still interfering with the memory cache tests. However, I did some digging around and added a test in a separate patch which makes it easier to reproduce the issue. In this test it seems that the cache file is not invalidated with "sss_cache -E", if there was a LDAP enumeration refresh before that.
Other changes are:
* Add full PEP8 cleanup for integration tests prior to adding moretests
Could you please send the PEP8 changes in a separate thread? They LGTM, but I would like to ACK them in a separate thread so that they are not blocked by review of the other 2 patches (btw. I will look at those as well soon).
(can submit as a separate patch if necessary, otherwise feel free to merge) * Cleaned up the PEP8 errors in the new tests.I'll not be able to work on this anymore this week, but will resume next week. Perhaps someone will be interested to look at that new test and figure out the failure reason in the meantime.
Nick
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 10/22/2015 03:04 PM, Michal Židek wrote:
On 10/12/2015 03:31 PM, Nikolai Kondrashov wrote:
Here's another version of the patch, which is still interfering with the memory cache tests. However, I did some digging around and added a test in a separate patch which makes it easier to reproduce the issue. In this test it seems that the cache file is not invalidated with "sss_cache -E", if there was a LDAP enumeration refresh before that.
Other changes are:
* Add full PEP8 cleanup for integration tests prior to adding moretests
Could you please send the PEP8 changes in a separate thread? They LGTM, but I would like to ACK them in a separate thread so that they are not blocked by review of the other 2 patches (btw. I will look at those as well soon).
Done. Thanks!
Nick
Hi!
There is one new pep8 error in the code: ../src/tests/intg/ldap_test.py:819:37: E126 continuation line over-indented for hanging indent
For the memcache workaroud please do this change in the code: diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 8263d88..eb466ab 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -194,8 +194,10 @@ def cleanup_sssd_process(): subprocess.call(["sss_cache", "-E"]) for path in os.listdir(config.DB_PATH): os.unlink(config.DB_PATH + "/" + path) - for path in os.listdir(config.MCACHE_PATH): - os.unlink(config.MCACHE_PATH + "/" + path) + # FIXME: Uncomment this when ticket #2726 is solved + # https://fedorahosted.org/sssd/ticket/2726 + # for path in os.listdir(config.MCACHE_PATH): + # os.unlink(config.MCACHE_PATH + "/" + path)
See some comments bellow...
On 10/12/2015 03:31 PM, Nikolai Kondrashov wrote:
0002-intg-Add-more-LDAP-tests.patch
From a2c75e045f8f341402a5c771489b462f18e27a39 Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 2/3] intg: Add more LDAP tests
Add a bunch of LDAP tests.
* Adding/removing a user/group/membership with rfc2307(bis) schema. * Filtering users/groups with rfc2307(bis) schema. * The effect of override_homedir option. * The effect of fallback_homedir option. * The effect of override_shell option. * The effect of shell_fallback option. * The effect of default_shell option. * The effect of vetoed_shells option.
src/tests/intg/ldap_test.py | 534 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 534 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 6a09b37..cca9431 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -125,6 +125,23 @@ def format_basic_conf(ldap_conn, schema, enum): """).format(**locals())
+def format_interactive_conf(ldap_conn, schema):
- """Format an SSSD configuration with all caches refreshing in 4 seconds"""
- return \
format_basic_conf(ldap_conn, schema, enum=True) + \unindent("""[nss]memcache_timeout = 4
It is better to set memcache timeout to zero outside tests that are not dedicated to memcache. This will also probably solve the membership tests failure that you saw when using the workaround for the memcache tests failure.
enum_cache_timeout = 4entry_negative_timeout = 4
I would set negative cache timeout to zero as well, see comments below.
[domain/LDAP]ldap_enumeration_refresh_timeout = 4ldap_purge_cache_timeout = 1entry_cache_timeout = 4""")- def create_conf_file(contents): """Create sssd.conf with specified contents""" conf = open(config.CONF_PATH, "w")
@@ -387,3 +404,520 @@ def test_refresh_after_cleanup_task(ldap_conn, refresh_after_cleanup_task): ent.assert_group_by_name( "group2", dict(mem=ent.contains_only("user1")))
+@pytest.fixture +def blank_rfc2307(request, ldap_conn):
- """Create blank RFC2307 directory fixture with interactive SSSD conf"""
- create_ldap_cleanup(request, ldap_conn)
- create_conf_fixture(request,
format_interactive_conf(ldap_conn, SCHEMA_RFC2307))- create_sssd_fixture(request)
+@pytest.fixture +def blank_rfc2307_bis(request, ldap_conn):
- """Create blank RFC2307bis directory fixture with interactive SSSD conf"""
- create_ldap_cleanup(request, ldap_conn)
- create_conf_fixture(request,
format_interactive_conf(ldap_conn, SCHEMA_RFC2307_BIS))- create_sssd_fixture(request)
+@pytest.fixture +def user_and_group_rfc2307(request, ldap_conn):
- """
- Create an RFC2307 directory fixture with interactive SSSD conf,
- one user and one group
- """
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user", 1001, 2000)
- ent_list.add_group("group", 2001)
- create_ldap_fixture(request, ldap_conn, ent_list)
- create_conf_fixture(request,
format_interactive_conf(ldap_conn, SCHEMA_RFC2307))- create_sssd_fixture(request)
- return None
+@pytest.fixture +def user_and_groups_rfc2307_bis(request, ldap_conn):
- """
- Create an RFC2307bis directory fixture with interactive SSSD conf,
- one user and two groups
- """
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user", 1001, 2000)
- ent_list.add_group_bis("group1", 2001)
- ent_list.add_group_bis("group2", 2002)
- create_ldap_fixture(request, ldap_conn, ent_list)
- create_conf_fixture(request,
format_interactive_conf(ldap_conn, SCHEMA_RFC2307_BIS))- create_sssd_fixture(request)
- return None
+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", 1001, 2000)
- time.sleep(2)
What is the purpose of this timeout? It does not seem to be necessary. You use it in all the other tests as well, so I guess it had some meaning (but I tried to remove them and tests passed for me without problems).
- # Add the user
- ent.assert_passwd(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
I would avoid testing the negative cache outside tests that are dedicated to negative cache. We had this same pattern in our C code tests and it timed out when CI was under heavy load. We can add dedicated tests for negative cache in CI later with big enough timeout to pass even under heavy load CI.
So please, remove the negative cache testing from everywhere in these tests.
- time.sleep(4)
Instead of the number 4, could you use some global constant. You could use the constant to generate the "interactive" config file as well. If we later see some timeout issues related to this, we can change it on one place. Also I think that this particular timeout will not be necessary if we stop testing the negative cache, which will speed up the tests.
- ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001)))
- # Remove the user
- ldap_conn.delete_s(e[0])
- ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001)))
- time.sleep(4)
- ent.assert_passwd(ent.contains_only())
+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(2)
- # Add the group
- ent.assert_group(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_group(ent.contains_only())
- time.sleep(4)
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- # Remove the group
- ldap_conn.delete_s(e[0])
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- time.sleep(4)
- ent.assert_group(ent.contains_only())
+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(2)
- # Add the group
- ent.assert_group(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_group(ent.contains_only())
- time.sleep(4)
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- # Remove the group
- ldap_conn.delete_s(e[0])
- ent.assert_group(ent.contains_only(dict(name="group", gid=2001)))
- time.sleep(4)
- ent.assert_group(ent.contains_only())
+def test_add_remove_membership_rfc2307(ldap_conn, user_and_group_rfc2307):
- """Test user membership addition and removal are reflected by SSSD"""
- time.sleep(2)
- # 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,
[(ldap.MOD_REPLACE, "memberUid", "user")])- ent.assert_group_by_name("group", dict(mem=ent.contains_only()))
- time.sleep(4)
- ent.assert_group_by_name("group", dict(mem=ent.contains_only("user")))
- # Remove user from group
- ldap_conn.modify_s("cn=group,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_DELETE, "memberUid", None)])- ent.assert_group_by_name("group", dict(mem=ent.contains_only("user")))
- time.sleep(4)
- ent.assert_group_by_name("group", dict(mem=ent.contains_only()))
+def test_add_remove_membership_rfc2307_bis(ldap_conn,
user_and_groups_rfc2307_bis):- """
- Test user and group membership addition and removal are reflected by SSSD,
- with RFC2307bis schema
- """
- time.sleep(2)
- # 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,
[(ldap.MOD_REPLACE, "member","uid=user,ou=Users," + ldap_conn.ds_inst.base_dn)])- ent.assert_group_by_name("group1", dict(mem=ent.contains_only()))
- time.sleep(4)
- ent.assert_group_by_name("group1", dict(mem=ent.contains_only("user")))
- # Add group1 to group2
- ldap_conn.modify_s("cn=group2,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_REPLACE, "member","cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn)])- ent.assert_group_by_name("group2", dict(mem=ent.contains_only()))
- time.sleep(4)
- ent.assert_group_by_name("group2", dict(mem=ent.contains_only("user")))
- # Remove group1 from group2
- ldap_conn.modify_s("cn=group2,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_DELETE, "member", None)])- ent.assert_group_by_name("group2", dict(mem=ent.contains_only("user")))
- time.sleep(4)
- ent.assert_group_by_name("group2", dict(mem=ent.contains_only()))
- # Remove user from group1
- ldap_conn.modify_s("cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn,
[(ldap.MOD_DELETE, "member", None)])- ent.assert_group_by_name("group1", dict(mem=ent.contains_only("user")))
- time.sleep(4)
- ent.assert_group_by_name("group1", dict(mem=ent.contains_only()))
+@pytest.fixture +def void_conf(request):
- create_conf_cleanup(request)
+@pytest.fixture +def void_sssd(request):
- create_sssd_cleanup(request)
+@pytest.fixture +def three_users_three_groups_rfc2307(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_user("user2", 1002, 2002)
- ent_list.add_user("user3", 1003, 2003)
- ent_list.add_group("group1", 2001, ["user1"])
- ent_list.add_group("group2", 2002, ["user2"])
- ent_list.add_group("group3", 2003, ["user3"])
- create_ldap_fixture(request, ldap_conn, ent_list)
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307,
void_conf, void_sssd):- """Test the effect of the "filter_users" option"""
- all_users = frozenset([1, 2, 3])
- for filter_users_in_groups in [False, True]:
for filter_users in [frozenset([]),frozenset([1]),frozenset([1, 2]),frozenset([1, 2, 3])]:unfiltered_users = all_users - filter_usersfilter_users_str = ",".join(map(lambda i: "user" + str(i),filter_users))conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""[nss]filter_users = {filter_users_str}filter_users_in_groups = {filter_users_in_groups}
Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.
""").format(**locals())create_conf_file(conf)create_sssd_process()ent.assert_passwd(ent.contains_only(*map(lambda i:dict(name="user" + str(i), uid=1000 + i),unfiltered_users)))ent.assert_group(ent.contains_only(*map(lambda i:dict(name="group" + str(i),gid=2000 + i,mem=ent.contains_only()if filter_users_in_groups and i in filter_userselse ent.contains_only("user" + str(i))),all_users)))cleanup_sssd_process()cleanup_conf_file()+def test_filter_groups_rfc2307(request, ldap_conn,
three_users_three_groups_rfc2307,void_conf, void_sssd):- """Test the effect of the "filter_groups" option with RFC2307 groups"""
- all_groups = frozenset([1, 2, 3])
- for filter_groups in [frozenset([]),
frozenset([1]),frozenset([1, 2]),frozenset([1, 2, 3])]:unfiltered_groups = all_groups - filter_groupsfilter_groups_str = ",".join(map(lambda i: "group" + str(i),filter_groups))conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""[nss]filter_groups = {filter_groups_str}
Same as above, please do not generate the conf file inside the test, but split it to multiple tests.
""").format(**locals())create_conf_file(conf)create_sssd_process()ent.assert_group(ent.contains_only(*map(lambda i:dict(name="group" + str(i),gid=2000 + i,mem=ent.contains_only("user" + str(i))),unfiltered_groups)))cleanup_sssd_process()cleanup_conf_file()+@pytest.fixture +def three_users_three_groups_rfc2307_bis(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_user("user2", 1002, 2002)
- ent_list.add_user("user3", 1003, 2003)
- ent_list.add_group_bis("group1", 2001, ["user1"])
- ent_list.add_group_bis("group2", 2002, ["user2"], ["group1"])
- ent_list.add_group_bis("group3", 2003, ["user3"], ["group2"])
- create_ldap_fixture(request, ldap_conn, ent_list)
+def test_filter_groups_rfc2307_bis(request, ldap_conn,
three_users_three_groups_rfc2307_bis,void_conf, void_sssd):- """Test the effect of the "filter_groups" option with RFC2307bis groups"""
- all_groups = frozenset([1, 2, 3])
- for filter_groups in [frozenset([]),
frozenset([1]),frozenset([1, 2]),frozenset([1, 2, 3])]:unfiltered_groups = all_groups - filter_groupsfilter_groups_str = ",".join(map(lambda i: "group" + str(i),filter_groups))conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True) + \unindent("""[nss]filter_groups = {filter_groups_str}
The same thing as above.
""").format(**locals())create_conf_file(conf)create_sssd_process()ent.assert_group(ent.contains_only(*map(lambda i:dict(name="group" + str(i),gid=2000 + i,mem=ent.contains_only(*map(lambda j: "user" + str(j),range(1, i + 1)))),unfiltered_groups)))cleanup_sssd_process()cleanup_conf_file()+@pytest.fixture +def override_homedir(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_homedir_A", 1001, 2001,
homeDirectory="/home/A")- ent_list.add_user("user_with_homedir_B", 1002, 2002,
homeDirectory="/home/B")- ent_list.add_user("user_with_empty_homedir", 1003, 2003,
homeDirectory="")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""\[nss]override_homedir = /home/B""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_override_homedir(override_homedir):
- """Test the effect of the "override_homedir" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_homedir_A", uid=1001, dir="/home/B"),dict(name="user_with_homedir_B", uid=1002, dir="/home/B"),dict(name="user_with_empty_homedir", uid=1003, dir="/home/B"))- )
+@pytest.fixture +def fallback_homedir(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_homedir_A", 1001, 2001,
homeDirectory="/home/A")- ent_list.add_user("user_with_homedir_B", 1002, 2002,
homeDirectory="/home/B")- ent_list.add_user("user_with_empty_homedir", 1003, 2003,
homeDirectory="")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""\[nss]fallback_homedir = /home/B""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_fallback_homedir(fallback_homedir):
- """Test the effect of the "fallback_homedir" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_homedir_A", uid=1001, dir="/home/A"),dict(name="user_with_homedir_B", uid=1002, dir="/home/B"),dict(name="user_with_empty_homedir", uid=1003, dir="/home/B"))- )
+@pytest.fixture +def override_shell(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_shell_A", 1001, 2001,
loginShell="/bin/A")- ent_list.add_user("user_with_shell_B", 1002, 2002,
loginShell="/bin/B")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell="")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""\[nss]override_shell = /bin/B""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_override_shell(override_shell):
- """Test the effect of the "override_shell" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_shell_A", uid=1001, shell="/bin/B"),dict(name="user_with_shell_B", uid=1002, shell="/bin/B"),dict(name="user_with_empty_shell", uid=1003, shell="/bin/B"))- )
+@pytest.fixture +def shell_fallback(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_sh_shell", 1001, 2001,
loginShell="/bin/sh")- ent_list.add_user("user_with_not_installed_shell", 1002, 2002,
loginShell="/bin/not_installed")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell="")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""\[nss]shell_fallback = /bin/fallbackallowed_shells = /bin/not_installed""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_shell_fallback(shell_fallback):
- """Test the effect of the "shell_fallback" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_sh_shell", uid=1001, shell="/bin/sh"),dict(name="user_with_not_installed_shell", uid=1002,shell="/bin/fallback"),dict(name="user_with_empty_shell", uid=1003, shell=""))- )
+@pytest.fixture +def default_shell(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_sh_shell", 1001, 2001,
loginShell="/bin/sh")- ent_list.add_user("user_with_not_installed_shell", 1002, 2002,
loginShell="/bin/not_installed")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell="")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""\[nss]default_shell = /bin/defaultallowed_shells = /bin/default, /bin/not_installedshell_fallback = /bin/fallback""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_default_shell(default_shell):
- """Test the effect of the "default_shell" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_sh_shell", uid=1001, shell="/bin/sh"),dict(name="user_with_not_installed_shell", uid=1002,shell="/bin/fallback"),dict(name="user_with_empty_shell", uid=1003,shell="/bin/default"))- )
+@pytest.fixture +def vetoed_shells(request, ldap_conn):
- ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
- ent_list.add_user("user_with_sh_shell", 1001, 2001,
loginShell="/bin/sh")- ent_list.add_user("user_with_vetoed_shell", 1002, 2002,
loginShell="/bin/vetoed")- ent_list.add_user("user_with_empty_shell", 1003, 2003,
loginShell="")- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = \
format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""\[nss]default_shell = /bin/defaultvetoed_shells = /bin/vetoedshell_fallback = /bin/fallback""").format(**locals())- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
+def test_vetoed_shells(vetoed_shells):
- """Test the effect of the "vetoed_shells" option"""
- ent.assert_passwd(
ent.contains_only(dict(name="user_with_sh_shell", uid=1001, shell="/bin/sh"),dict(name="user_with_vetoed_shell", uid=1002,shell="/bin/fallback"),dict(name="user_with_empty_shell", uid=1003,shell="/bin/default"))- )
-- 2.6.1
0003-intg-Add-a-memcache-invalidation-failure-test.patch
From 2c13856c9f650503b8eeb2d4446469548728804b Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Mon, 12 Oct 2015 16:17:46 +0300 Subject: [PATCH 3/3] intg: Add a memcache invalidation failure test
Add a memcache invalidation failure integration test. It fails as is, and passes if ldap_enumeration_refresh_timeout in the first half is set to e.g. 30. The sss_nss_check_header function was observed to always return 0 for the second half of the test, when it was failing.
This test passes for me with the memcache workaround. I guess we do not need it then. The reproducer for the failure can be achieved by removing the workaround (which we will do once we fix the root cause).
src/tests/intg/ldap_test.py | 98 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index cca9431..cdd8c8d 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -474,6 +474,99 @@ def test_add_remove_user(ldap_conn, blank_rfc2307): ent.assert_passwd(ent.contains_only())
+def test_add_remove_user_memcache_mix(ldap_conn,
void_conf,void_sssd,void_ldap):- # Setup
- conf = unindent("""\
[sssd]debug_level = 0xffffdomains = LDAPservices = nss, pam[nss]memcache_timeout = 0[pam][domain/LDAP]ldap_auth_disable_tls_never_use_in_production = truedebug_level = 0xffffldap_schema = rfc2307enumerate = trueid_provider = ldapauth_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}ldap_enumeration_refresh_timeout = 4ldap_purge_cache_timeout = 1entry_cache_timeout = 4- """).format(**locals())
- create_conf_file(conf)
- create_sssd_process()
- e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 1001, 2001)
- time.sleep(2)
- # Add the user
- ldap_conn.add_s(*e)
- time.sleep(4)
- pwd.getpwnam("user")
- # Remove the user
- ldap_conn.delete_s(e[0])
- time.sleep(4)
- # Teardown
- cleanup_sssd_process()
- cleanup_conf_file()
- cleanup_ldap_entries(ldap_conn)
- # Setup
- e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 1001, 2001)
- ldap_conn.add_s(*e)
- conf = unindent("""\
[sssd]domains = LDAPservices = nss[nss]debug_level = 0xffff[domain/LDAP]ldap_auth_disable_tls_never_use_in_production = trueldap_schema = rfc2307id_provider = ldapauth_provider = ldapsudo_provider = ldapldap_uri = {ldap_conn.ds_inst.ldap_url}ldap_search_base = {ldap_conn.ds_inst.base_dn}- """).format(**locals())
- create_conf_file(conf)
- create_sssd_process()
- # Check user presence
- ent.assert_passwd_by_name(
'user',dict(name='user', passwd='*', uid=1001, gid=2001,gecos='1001', shell='/bin/bash'))- # Stop sssd
- pid_file = open(config.PIDFILE_PATH, "r")
- pid = int(pid_file.read())
- os.kill(pid, signal.SIGTERM)
- while True:
try:os.kill(pid, signal.SIGCONT)except:breaktime.sleep(1)- # Check user presence
- ent.assert_passwd_by_name(
'user',dict(name='user', passwd='*', uid=1001, gid=2001,gecos='1001', shell='/bin/bash'))- # Teardown
- cleanup_conf_file()
- cleanup_ldap_entries(ldap_conn)
- 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)
@@ -566,6 +659,11 @@ def test_add_remove_membership_rfc2307_bis(ldap_conn,
@pytest.fixture +def void_ldap(request, ldap_conn):
- create_ldap_cleanup(request, ldap_conn)
+@pytest.fixture def void_conf(request): create_conf_cleanup(request)
-- 2.6.1
Just for info, this is git diff with changes I made when testing your patches:
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 8263d88..fc21275 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -133,9 +133,9 @@ def format_interactive_conf(ldap_conn, schema): format_basic_conf(ldap_conn, schema, enum=True) + \ unindent(""" [nss] - memcache_timeout = 4 + memcache_timeout = 0 enum_cache_timeout = 4 - entry_negative_timeout = 4 + entry_negative_timeout = 0
[domain/LDAP] ldap_enumeration_refresh_timeout = 4 @@ -194,8 +194,10 @@ def cleanup_sssd_process(): subprocess.call(["sss_cache", "-E"]) for path in os.listdir(config.DB_PATH): os.unlink(config.DB_PATH + "/" + path) - for path in os.listdir(config.MCACHE_PATH): - os.unlink(config.MCACHE_PATH + "/" + path) + # FIXME: Uncomment this when ticket #2726 is solved + # https://fedorahosted.org/sssd/ticket/2726 + # for path in os.listdir(config.MCACHE_PATH): + # os.unlink(config.MCACHE_PATH + "/" + path)
def create_sssd_cleanup(request): @@ -462,12 +464,9 @@ 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", 1001, 2000) - time.sleep(2) # Add the user ent.assert_passwd(ent.contains_only()) ldap_conn.add_s(*e) - ent.assert_passwd(ent.contains_only()) - time.sleep(4) ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001))) # Remove the user ldap_conn.delete_s(e[0]) @@ -509,10 +508,8 @@ def test_add_remove_user_memcache_mix(ldap_conn, create_conf_file(conf) create_sssd_process() e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 1001, 2001) - time.sleep(2) # Add the user ldap_conn.add_s(*e) - time.sleep(4) pwd.getpwnam("user") # Remove the user ldap_conn.delete_s(e[0]) @@ -572,12 +569,9 @@ def test_add_remove_user_memcache_mix(ldap_conn, 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(2) # Add the group ent.assert_group(ent.contains_only()) ldap_conn.add_s(*e) - ent.assert_group(ent.contains_only()) - time.sleep(4) ent.assert_group(ent.contains_only(dict(name="group", gid=2001))) # Remove the group ldap_conn.delete_s(e[0]) @@ -589,12 +583,9 @@ 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(2) # Add the group ent.assert_group(ent.contains_only()) ldap_conn.add_s(*e) - ent.assert_group(ent.contains_only()) - time.sleep(4) ent.assert_group(ent.contains_only(dict(name="group", gid=2001))) # Remove the group ldap_conn.delete_s(e[0]) @@ -605,13 +596,10 @@ 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(2) # 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, [(ldap.MOD_REPLACE, "memberUid", "user")]) - ent.assert_group_by_name("group", dict(mem=ent.contains_only())) - time.sleep(4) ent.assert_group_by_name("group", dict(mem=ent.contains_only("user"))) # Remove user from group ldap_conn.modify_s("cn=group,ou=Groups," + ldap_conn.ds_inst.base_dn, @@ -627,22 +615,17 @@ def test_add_remove_membership_rfc2307_bis(ldap_conn, Test user and group membership addition and removal are reflected by SSSD, with RFC2307bis schema """ - time.sleep(2) # 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, [(ldap.MOD_REPLACE, "member", "uid=user,ou=Users," + ldap_conn.ds_inst.base_dn)]) - ent.assert_group_by_name("group1", dict(mem=ent.contains_only())) - time.sleep(4) ent.assert_group_by_name("group1", dict(mem=ent.contains_only("user")))
# Add group1 to group2 ldap_conn.modify_s("cn=group2,ou=Groups," + ldap_conn.ds_inst.base_dn, [(ldap.MOD_REPLACE, "member", "cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn)]) - ent.assert_group_by_name("group2", dict(mem=ent.contains_only())) - time.sleep(4) ent.assert_group_by_name("group2", dict(mem=ent.contains_only("user")))
# Remove group1 from group2
Hi Michal,
Thanks a lot for the detailed review and testing! Please see my comments below.
On 10/23/2015 02:54 PM, Michal Židek wrote:
Hi!
There is one new pep8 error in the code: ../src/tests/intg/ldap_test.py:819:37: E126 continuation line over-indented for hanging indent
Sure, will fix it.
For the memcache workaroud please do this change in the code: diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 8263d88..eb466ab 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -194,8 +194,10 @@ def cleanup_sssd_process(): subprocess.call(["sss_cache", "-E"]) for path in os.listdir(config.DB_PATH): os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
- # FIXME: Uncomment this when ticket #2726 is solved
- # https://fedorahosted.org/sssd/ticket/2726
- # for path in os.listdir(config.MCACHE_PATH):
- # os.unlink(config.MCACHE_PATH + "/" + path)
+def format_interactive_conf(ldap_conn, schema):
- """Format an SSSD configuration with all caches refreshing in 4 seconds"""
- return \
format_basic_conf(ldap_conn, schema, enum=True) + \unindent("""[nss]memcache_timeout = 4It is better to set memcache timeout to zero outside tests that are not dedicated to memcache. This will also probably solve the membership tests failure that you saw when using the workaround for the memcache tests failure.
Hmm, perhaps. However, could you please explain why the membership tests fail?
I noticed that they also fail if I simply don't run the group addition/removal tests before them, in addition to enabling the workaround. Also, they work if I put "run_shell" before each "assert" and immediately exit the spawned shells even without doing anything in them.
enum_cache_timeout = 4entry_negative_timeout = 4I would set negative cache timeout to zero as well, see comments below.
Replying below.
+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", 1001, 2000)
- time.sleep(2)
What is the purpose of this timeout? It does not seem to be necessary. You use it in all the other tests as well, so I guess it had some meaning (but I tried to remove them and tests passed for me without problems).
This puts test actions and tests in the middle of the 4 second cache timeouts, so they are more reliable and time drift doesn't affect them that much.
E.g.:
0 - sssd start 1 - 2 - add user, check it's not yet present 3 - 4 - cache expiry/purging 5 - 6 - check user added 7 - 8 - cache expiry/purging
etc.
IIRC, I got 400+ perfect runs before the first failure occurred with this - better than other schemes.
- # Add the user
- ent.assert_passwd(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
I would avoid testing the negative cache outside tests that are dedicated to negative cache. We had this same pattern in our C code tests and it timed out when CI was under heavy load. We can add dedicated tests for negative cache in CI later with big enough timeout to pass even under heavy load CI.
So please, remove the negative cache testing from everywhere in these tests.
I'm OK with minimizing the tests and not exercising some caching mechanisms. I understand it will make tests more reliable.
However, my intention was to test the full end-user functionality, what actually matters to users. Users don't really care about caches, they just want everything work fast and reliably. They just want their LDAP changes propagated. For that matter I only touch cache timeouts to make tests run in reasonable time. I don't disable them, because users will probably have them enabled as well.
We can test all the cache mechanisms separately, but we will still have to test them working together. Can we do that? Can we have these particular tests do that? Or is it too hard/impossible?
If we can do that, how would you like to see them?
If not, I'll just disable memory cache and remove negative cache testing as you request.
Or do I misunderstand the idea behind this?
- time.sleep(4)
Instead of the number 4, could you use some global constant. You could use the constant to generate the "interactive" config file as well. If we later see some timeout issues related to this, we can change it on one place.
Yes, good idea, thank you.
Also I think that this particular timeout will not be necessary if we stop testing the negative cache, which will speed up the tests.
OK, if we decide to disable negative tests, then I'll see if I can speed it up.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""[nss]filter_users = {filter_users_str}filter_users_in_groups = {filter_users_in_groups}Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.
I would really like to do that, but the problem is we will either need to copy-paste a ton of tests and fixtures, or use py.test's convoluted means of sharing parameters between fixtures and tests which will produce much more complicated code that this. At least this is my impression after researching that.
Do you have any particular design for this in mind?
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""[nss]filter_groups = {filter_groups_str}Same as above, please do not generate the conf file inside the test, but split it to multiple tests.
Please see my comment right above.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS, enum=True) + \unindent("""[nss]filter_groups = {filter_groups_str}The same thing as above.
Please see my comment above.
Add a memcache invalidation failure integration test. It fails as is, and passes if ldap_enumeration_refresh_timeout in the first half is set to e.g. 30. The sss_nss_check_header function was observed to always return 0 for the second half of the test, when it was failing.
This test passes for me with the memcache workaround. I guess we do not need it then. The reproducer for the failure can be achieved by removing the workaround (which we will do once we fix the root cause).
Do you mean we should not merge it, remove it from the patchset? Or add it along with a workaround, so after we fix the failure and remove the workaround it stays on the lookout?
Just for info, this is git diff with changes I made when testing your patches:
I see, thank you!
Sincerely, Nick
On 10/27/2015 04:10 PM, Nikolai Kondrashov wrote:
Hi Michal,
Thanks a lot for the detailed review and testing! Please see my comments below.
On 10/23/2015 02:54 PM, Michal Židek wrote:
Hi!
There is one new pep8 error in the code: ../src/tests/intg/ldap_test.py:819:37: E126 continuation line over-indented for hanging indent
Sure, will fix it.
For the memcache workaroud please do this change in the code: diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index 8263d88..eb466ab 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -194,8 +194,10 @@ def cleanup_sssd_process(): subprocess.call(["sss_cache", "-E"]) for path in os.listdir(config.DB_PATH): os.unlink(config.DB_PATH + "/" + path)
- for path in os.listdir(config.MCACHE_PATH):
os.unlink(config.MCACHE_PATH + "/" + path)
- # FIXME: Uncomment this when ticket #2726 is solved
- # https://fedorahosted.org/sssd/ticket/2726
- # for path in os.listdir(config.MCACHE_PATH):
- # os.unlink(config.MCACHE_PATH + "/" + path)
+def format_interactive_conf(ldap_conn, schema):
- """Format an SSSD configuration with all caches refreshing in 4
seconds"""
- return \
format_basic_conf(ldap_conn, schema, enum=True) + \unindent("""[nss]memcache_timeout = 4It is better to set memcache timeout to zero outside tests that are not dedicated to memcache. This will also probably solve the membership tests failure that you saw when using the workaround for the memcache tests failure.
Hmm, perhaps. However, could you please explain why the membership tests fail?
I noticed that they also fail if I simply don't run the group addition/removal tests before them, in addition to enabling the workaround. Also, they work if I put "run_shell" before each "assert" and immediately exit the spawned shells even without doing anything in them.
I really do not know why the run_shell helped. Maybe it forced pytest to initialize new client memory cache context. I tried to do some tests with run_shell, but they did not work for me.
Anyway, the failures were connected to memory cache and memory cache currently is not reliable in these test, so I did not investigated further, why they failed.
enum_cache_timeout = 4entry_negative_timeout = 4I would set negative cache timeout to zero as well, see comments below.
Replying below.
+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", 1001, 2000)
- time.sleep(2)
What is the purpose of this timeout? It does not seem to be necessary. You use it in all the other tests as well, so I guess it had some meaning (but I tried to remove them and tests passed for me without problems).
This puts test actions and tests in the middle of the 4 second cache timeouts, so they are more reliable and time drift doesn't affect them that much.
E.g.:
0 - sssd start 1 - 2 - add user, check it's not yet present 3 - 4 - cache expiry/purging 5 - 6 - check user added 7 - 8 - cache expiry/purgingetc.
IIRC, I got 400+ perfect runs before the first failure occurred with this - better than other schemes.
Hmmm...maybe this was because SSSD was not initialized (fully started) when the test was run. In which case it is problem of the fixture that starts SSSD and should be solved inside the fixture (the sleep() should be added there).
- # Add the user
- ent.assert_passwd(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
I would avoid testing the negative cache outside tests that are dedicated to negative cache. We had this same pattern in our C code tests and it timed out when CI was under heavy load. We can add dedicated tests for negative cache in CI later with big enough timeout to pass even under heavy load CI.
So please, remove the negative cache testing from everywhere in these tests.
I'm OK with minimizing the tests and not exercising some caching mechanisms. I understand it will make tests more reliable.
However, my intention was to test the full end-user functionality, what actually matters to users. Users don't really care about caches, they just want everything work fast and reliably. They just want their LDAP changes propagated. For that matter I only touch cache timeouts to make tests run in reasonable time. I don't disable them, because users will probably have them enabled as well.
We can test all the cache mechanisms separately, but we will still have to test them working together. Can we do that? Can we have these particular tests do that? Or is it too hard/impossible?
If we can do that, how would you like to see them?
If not, I'll just disable memory cache and remove negative cache testing as you request.
Or do I misunderstand the idea behind this?
I understand your point, but negative cache IMO just slows down the tests. If we disable it, we still actually use it under the hood, it just timeouts immediately (so we trigger the same codepath, that most users will trigger in their environment). Users will usually not add and delete users in such short time, so using negative cache does not get us closer to real cases and it really is just test scenario.
Main reason why I would like to have separate tests for negative cache (which I do not want you to add now) is that in order to test it in CI, we need higher timeout value for pattern: test_if_user_exists (does not) add_user test_if_user_exists (does not - negative cache hit)
This is the pattern that you use and we had problems with this pattern in our C unit test (which we solved by increasing the timeout). In python we may need even higher values than in C.
- time.sleep(4)
Instead of the number 4, could you use some global constant. You could use the constant to generate the "interactive" config file as well. If we later see some timeout issues related to this, we can change it on one place.
Yes, good idea, thank you.
Also I think that this particular timeout will not be necessary if we stop testing the negative cache, which will speed up the tests.
OK, if we decide to disable negative tests, then I'll see if I can speed it up.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307,enum=True) + \
unindent("""[nss]filter_users = {filter_users_str}filter_users_in_groups = {filter_users_in_groups}Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.
I would really like to do that, but the problem is we will either need to copy-paste a ton of tests and fixtures, or use py.test's convoluted means of sharing parameters between fixtures and tests which will produce much more complicated code that this. At least this is my impression after researching that.
Do you have any particular design for this in mind?
No, I dont. I think copy-pasting is Ok in this case. The copy-pasted functions will not be big and all on one place.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307, enum=True) + \unindent("""[nss]filter_groups = {filter_groups_str}Same as above, please do not generate the conf file inside the test, but split it to multiple tests.
Please see my comment right above.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS,enum=True) + \
unindent("""[nss]filter_groups = {filter_groups_str}The same thing as above.
Please see my comment above.
Add a memcache invalidation failure integration test. It fails as is, and passes if ldap_enumeration_refresh_timeout in the first half is set to e.g. 30. The sss_nss_check_header function was observed to always return 0 for the second half of the test, when it was failing.
This test passes for me with the memcache workaround. I guess we do not need it then. The reproducer for the failure can be achieved by removing the workaround (which we will do once we fix the root cause).
Do you mean we should not merge it, remove it from the patchset? Or add it along with a workaround, so after we fix the failure and remove the workaround it stays on the lookout?
I wanted to remove the test completely, but maybe it can serve as regression test. Can you add the patch as attachment to the ticket #2726? We can decide later if we want to use it or not, still I would like to push it out of this thread.
Just for info, this is git diff with changes I made when testing your patches:
I see, thank you!
Sincerely, Nick
On 10/29/2015 04:52 PM, Michal Židek wrote:
On 10/27/2015 04:10 PM, Nikolai Kondrashov wrote:
On 10/23/2015 02:54 PM, Michal Židek wrote:
+def format_interactive_conf(ldap_conn, schema):
- """Format an SSSD configuration with all caches refreshing in 4
seconds"""
- return \
format_basic_conf(ldap_conn, schema, enum=True) + \unindent("""[nss]memcache_timeout = 4It is better to set memcache timeout to zero outside tests that are not dedicated to memcache. This will also probably solve the membership tests failure that you saw when using the workaround for the memcache tests failure.
Hmm, perhaps. However, could you please explain why the membership tests fail?
I noticed that they also fail if I simply don't run the group addition/removal tests before them, in addition to enabling the workaround. Also, they work if I put "run_shell" before each "assert" and immediately exit the spawned shells even without doing anything in them.
I really do not know why the run_shell helped. Maybe it forced pytest to initialize new client memory cache context. I tried to do some tests with run_shell, but they did not work for me.
Anyway, the failures were connected to memory cache and memory cache currently is not reliable in these test, so I did not investigated further, why they failed.
Alright.
enum_cache_timeout = 4entry_negative_timeout = 4I would set negative cache timeout to zero as well, see comments below.
Replying below.
+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", 1001, 2000)
- time.sleep(2)
What is the purpose of this timeout? It does not seem to be necessary. You use it in all the other tests as well, so I guess it had some meaning (but I tried to remove them and tests passed for me without problems).
This puts test actions and tests in the middle of the 4 second cache timeouts, so they are more reliable and time drift doesn't affect them that much.
E.g.:
0 - sssd start 1 - 2 - add user, check it's not yet present 3 - 4 - cache expiry/purging 5 - 6 - check user added 7 - 8 - cache expiry/purgingetc.
IIRC, I got 400+ perfect runs before the first failure occurred with this - better than other schemes.
Hmmm...maybe this was because SSSD was not initialized (fully started) when the test was run. In which case it is problem of the fixture that starts SSSD and should be solved inside the fixture (the sleep() should be added there).
IIRC it wasn't always failing on the first test. That rules out the initialization issue. Besides, IIRC, about two years ago we worked on ensuring SSSD is fully initialized and ready to serve request when service start completes. Red Hat's downstream tests rely on that.
However, the initial delay might be better moved to the fixture. Will try to do that and see how it fits.
- # Add the user
- ent.assert_passwd(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
I would avoid testing the negative cache outside tests that are dedicated to negative cache. We had this same pattern in our C code tests and it timed out when CI was under heavy load. We can add dedicated tests for negative cache in CI later with big enough timeout to pass even under heavy load CI.
So please, remove the negative cache testing from everywhere in these tests.
I'm OK with minimizing the tests and not exercising some caching mechanisms. I understand it will make tests more reliable.
However, my intention was to test the full end-user functionality, what actually matters to users. Users don't really care about caches, they just want everything work fast and reliably. They just want their LDAP changes propagated. For that matter I only touch cache timeouts to make tests run in reasonable time. I don't disable them, because users will probably have them enabled as well.
We can test all the cache mechanisms separately, but we will still have to test them working together. Can we do that? Can we have these particular tests do that? Or is it too hard/impossible?
If we can do that, how would you like to see them?
If not, I'll just disable memory cache and remove negative cache testing as you request.
Or do I misunderstand the idea behind this?
I understand your point, but negative cache IMO just slows down the tests. If we disable it, we still actually use it under the hood, it just timeouts immediately (so we trigger the same codepath, that most users will trigger in their environment). Users will usually not add and delete users in such short time, so using negative cache does not get us closer to real cases and it really is just test scenario.
Main reason why I would like to have separate tests for negative cache (which I do not want you to add now) is that in order to test it in CI, we need higher timeout value for pattern: test_if_user_exists (does not) add_user test_if_user_exists (does not - negative cache hit)
This is the pattern that you use and we had problems with this pattern in our C unit test (which we solved by increasing the timeout). In python we may need even higher values than in C.
Now, I think I got confused here. I'm not actually testing negative cache, but only the fact that the changes to LDAP don't appear before the caches expire. Since the negative cache timeout equals the timeout of positive caches, there are really no negative cache-specific tests.
That said, I can still remove the tests you mention in your patch, if you think we should remove them. However, just for the record, I didn't see any issues with them.
Another thing is, I couldn't make the tests work with those sleep(4)'s removed as you did in your patch up-thread. In fact all *add_remove* tests failed except the add_remove_user_memcache_mix (delay results are not checked there).
The problem being, the assertions following these delays check enumeration and enumeration timeout is still 4 seconds, so we need to wait for it. For that matter removing tests you mention doesn't have an impact on execution time.
I'm not quite sure how you made it work. Was some other change omitted from your patch accidentally? I'm talking about changes like this:
def create_sssd_cleanup(request): @@ -462,12 +464,9 @@ 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", 1001, 2000) - time.sleep(2) # Add the user ent.assert_passwd(ent.contains_only()) ldap_conn.add_s(*e) - ent.assert_passwd(ent.contains_only()) - time.sleep(4) ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001))) # Remove the user ldap_conn.delete_s(e[0])
All in all, setting memcache_timeout to zero and commenting-out memcache file removal was enough to make *all* tests work fine.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307,enum=True) + \
unindent("""[nss]filter_users = {filter_users_str}filter_users_in_groups = {filter_users_in_groups}Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.
I would really like to do that, but the problem is we will either need to copy-paste a ton of tests and fixtures, or use py.test's convoluted means of sharing parameters between fixtures and tests which will produce much more complicated code that this. At least this is my impression after researching that.
Do you have any particular design for this in mind?
No, I dont. I think copy-pasting is Ok in this case. The copy-pasted functions will not be big and all on one place.
Alright, I'll try to do that and we'll see how it goes.
Add a memcache invalidation failure integration test. It fails as is, and passes if ldap_enumeration_refresh_timeout in the first half is set to e.g. 30. The sss_nss_check_header function was observed to always return 0 for the second half of the test, when it was failing.
This test passes for me with the memcache workaround. I guess we do not need it then. The reproducer for the failure can be achieved by removing the workaround (which we will do once we fix the root cause).
Do you mean we should not merge it, remove it from the patchset? Or add it along with a workaround, so after we fix the failure and remove the workaround it stays on the lookout?
I wanted to remove the test completely, but maybe it can serve as regression test. Can you add the patch as attachment to the ticket #2726? We can decide later if we want to use it or not, still I would like to push it out of this thread.
Alright, will try to remember to do that when we merge the other tests and I could rebase it.
Nick
On 11/03/2015 07:30 PM, Nikolai Kondrashov wrote:
All in all, setting memcache_timeout to zero and commenting-out memcache file removal was enough to make *all* tests work fine.
FWIW, I left it running repeatedly overnight and it got 250 successful runs and is still going.
Nick
On 11/03/2015 06:30 PM, Nikolai Kondrashov wrote:
On 10/29/2015 04:52 PM, Michal Židek wrote:
On 10/27/2015 04:10 PM, Nikolai Kondrashov wrote:
On 10/23/2015 02:54 PM, Michal Židek wrote:
+def format_interactive_conf(ldap_conn, schema):
- """Format an SSSD configuration with all caches refreshing in 4
seconds"""
- return \
format_basic_conf(ldap_conn, schema, enum=True) + \unindent("""[nss]memcache_timeout = 4It is better to set memcache timeout to zero outside tests that are not dedicated to memcache. This will also probably solve the membership tests failure that you saw when using the workaround for the memcache tests failure.
Hmm, perhaps. However, could you please explain why the membership tests fail?
I noticed that they also fail if I simply don't run the group addition/removal tests before them, in addition to enabling the workaround. Also, they work if I put "run_shell" before each "assert" and immediately exit the spawned shells even without doing anything in them.
I really do not know why the run_shell helped. Maybe it forced pytest to initialize new client memory cache context. I tried to do some tests with run_shell, but they did not work for me.
Anyway, the failures were connected to memory cache and memory cache currently is not reliable in these test, so I did not investigated further, why they failed.
Alright.
enum_cache_timeout = 4entry_negative_timeout = 4I would set negative cache timeout to zero as well, see comments below.
Replying below.
+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", 1001, 2000)
- time.sleep(2)
What is the purpose of this timeout? It does not seem to be necessary. You use it in all the other tests as well, so I guess it had some meaning (but I tried to remove them and tests passed for me without problems).
This puts test actions and tests in the middle of the 4 second cache timeouts, so they are more reliable and time drift doesn't affect them that much.
E.g.:
0 - sssd start 1 - 2 - add user, check it's not yet present 3 - 4 - cache expiry/purging 5 - 6 - check user added 7 - 8 - cache expiry/purgingetc.
IIRC, I got 400+ perfect runs before the first failure occurred with this - better than other schemes.
Hmmm...maybe this was because SSSD was not initialized (fully started) when the test was run. In which case it is problem of the fixture that starts SSSD and should be solved inside the fixture (the sleep() should be added there).
IIRC it wasn't always failing on the first test. That rules out the initialization issue. Besides, IIRC, about two years ago we worked on ensuring SSSD is fully initialized and ready to serve request when service start completes. Red Hat's downstream tests rely on that.
However, the initial delay might be better moved to the fixture. Will try to do that and see how it fits.
- # Add the user
- ent.assert_passwd(ent.contains_only())
- ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
I would avoid testing the negative cache outside tests that are dedicated to negative cache. We had this same pattern in our C code tests and it timed out when CI was under heavy load. We can add dedicated tests for negative cache in CI later with big enough timeout to pass even under heavy load CI.
So please, remove the negative cache testing from everywhere in these tests.
I'm OK with minimizing the tests and not exercising some caching mechanisms. I understand it will make tests more reliable.
However, my intention was to test the full end-user functionality, what actually matters to users. Users don't really care about caches, they just want everything work fast and reliably. They just want their LDAP changes propagated. For that matter I only touch cache timeouts to make tests run in reasonable time. I don't disable them, because users will probably have them enabled as well.
We can test all the cache mechanisms separately, but we will still have to test them working together. Can we do that? Can we have these particular tests do that? Or is it too hard/impossible?
If we can do that, how would you like to see them?
If not, I'll just disable memory cache and remove negative cache testing as you request.
Or do I misunderstand the idea behind this?
I understand your point, but negative cache IMO just slows down the tests. If we disable it, we still actually use it under the hood, it just timeouts immediately (so we trigger the same codepath, that most users will trigger in their environment). Users will usually not add and delete users in such short time, so using negative cache does not get us closer to real cases and it really is just test scenario.
Main reason why I would like to have separate tests for negative cache (which I do not want you to add now) is that in order to test it in CI, we need higher timeout value for pattern: test_if_user_exists (does not) add_user test_if_user_exists (does not - negative cache hit)
This is the pattern that you use and we had problems with this pattern in our C unit test (which we solved by increasing the timeout). In python we may need even higher values than in C.
Now, I think I got confused here. I'm not actually testing negative cache, but only the fact that the changes to LDAP don't appear before the caches expire. Since the negative cache timeout equals the timeout of positive caches, there are really no negative cache-specific tests.
That said, I can still remove the tests you mention in your patch, if you think we should remove them. However, just for the record, I didn't see any issues with them.
Another thing is, I couldn't make the tests work with those sleep(4)'s removed as you did in your patch up-thread. In fact all *add_remove* tests failed except the add_remove_user_memcache_mix (delay results are not checked there).
The problem being, the assertions following these delays check enumeration and enumeration timeout is still 4 seconds, so we need to wait for it. For that matter removing tests you mention doesn't have an impact on execution time.
I see where the confusion comes from now. But the enumeration should not play a role here. You can think about enumeration as forced sysdb fill operation, but it is not necessary in order to grab user from LDAP. If the user is not found in the sysdb (and also not found in the negcache) SSSD grabs the user from LDAP even before the next enumeration.
I'm not quite sure how you made it work. Was some other change omitted from your patch accidentally? I'm talking about changes like this:
I do not think I omitted something. Did you really set the negative_cache to 0 in sssd.conf?
def create_sssd_cleanup(request): @@ -462,12 +464,9 @@ 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", 1001, 2000)
- time.sleep(2) # Add the user ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Here you fill the negative cache.
ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Here you are testing the negative cache. If there was no negcache hit, SSSD would grab the user from LDAP.
- time.sleep(4)
^^^^^^^^^^^^^ So this sleep should not be necessary if the negcache is disabled.
ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001))) # Remove the user ldap_conn.delete_s(e[0])All in all, setting memcache_timeout to zero and commenting-out memcache file removal was enough to make *all* tests work fine.
That is good! If the patches do not work for you without the sleep()s you can send them with the sleep()s for now and I will take a look.
conf = \format_basic_conf(ldap_conn, SCHEMA_RFC2307,enum=True) + \
unindent("""[nss]filter_users = {filter_users_str}filter_users_in_groups ={filter_users_in_groups}
Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.
I would really like to do that, but the problem is we will either need to copy-paste a ton of tests and fixtures, or use py.test's convoluted means of sharing parameters between fixtures and tests which will produce much more complicated code that this. At least this is my impression after researching that.
Do you have any particular design for this in mind?
No, I dont. I think copy-pasting is Ok in this case. The copy-pasted functions will not be big and all on one place.
Alright, I'll try to do that and we'll see how it goes.
Thanks.
Add a memcache invalidation failure integration test. It fails as is, and passes if ldap_enumeration_refresh_timeout in the first half is set to e.g. 30. The sss_nss_check_header function was observed to always return 0 for the second half of the test, when it was failing.
This test passes for me with the memcache workaround. I guess we do not need it then. The reproducer for the failure can be achieved by removing the workaround (which we will do once we fix the root cause).
Do you mean we should not merge it, remove it from the patchset? Or add it along with a workaround, so after we fix the failure and remove the workaround it stays on the lookout?
I wanted to remove the test completely, but maybe it can serve as regression test. Can you add the patch as attachment to the ticket #2726? We can decide later if we want to use it or not, still I would like to push it out of this thread.
Alright, will try to remember to do that when we merge the other tests and I could rebase it.
Nick _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 11/04/2015 04:25 PM, Michal Židek wrote:
On 11/03/2015 06:30 PM, Nikolai Kondrashov wrote:
Now, I think I got confused here. I'm not actually testing negative cache, but only the fact that the changes to LDAP don't appear before the caches expire. Since the negative cache timeout equals the timeout of positive caches, there are really no negative cache-specific tests.
That said, I can still remove the tests you mention in your patch, if you think we should remove them. However, just for the record, I didn't see any issues with them.
Another thing is, I couldn't make the tests work with those sleep(4)'s removed as you did in your patch up-thread. In fact all *add_remove* tests failed except the add_remove_user_memcache_mix (delay results are not checked there).
The problem being, the assertions following these delays check enumeration and enumeration timeout is still 4 seconds, so we need to wait for it. For that matter removing tests you mention doesn't have an impact on execution time.
I see where the confusion comes from now. But the enumeration should not play a role here. You can think about enumeration as forced sysdb fill operation, but it is not necessary in order to grab user from LDAP. If the user is not found in the sysdb (and also not found in the negcache) SSSD grabs the user from LDAP even before the next enumeration.
Sure. However, ent.assert_passwd does pwd.getpwall, not only pwd.getpwnam and pwd.getpwuid. Pwd.getpwall result check is the one that fails. Ent.assert_group behaves similarly.
I think I remember Jakub asking to have this test without enumeration as well, and I can do that, but I think enumeration test is also valuable.
I'm not quite sure how you made it work. Was some other change omitted from your patch accidentally? I'm talking about changes like this:
I do not think I omitted something. Did you really set the negative_cache to 0 in sssd.conf?
Yes. I tried the exact patch you attached.
def create_sssd_cleanup(request): @@ -462,12 +464,9 @@ 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", 1001, 2000)
- time.sleep(2) # Add the user ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Here you fill the negative cache.
ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Here you are testing the negative cache. If there was no negcache hit, SSSD would grab the user from LDAP.
Ah, yes, it seems this should fetch from the negative cache.
- time.sleep(4)
^^^^^^^^^^^^^So this sleep should not be necessary if the negcache is disabled.
ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001))) # Remove the user ldap_conn.delete_s(e[0])All in all, setting memcache_timeout to zero and commenting-out memcache file removal was enough to make *all* tests work fine.
That is good! If the patches do not work for you without the sleep()s you can send them with the sleep()s for now and I will take a look.
Alright.
Nick
On 11/04/2015 05:01 PM, Nikolai Kondrashov wrote:
On 11/04/2015 04:25 PM, Michal Židek wrote:
On 11/03/2015 06:30 PM, Nikolai Kondrashov wrote:
Now, I think I got confused here. I'm not actually testing negative cache, but only the fact that the changes to LDAP don't appear before the caches expire. Since the negative cache timeout equals the timeout of positive caches, there are really no negative cache-specific tests.
That said, I can still remove the tests you mention in your patch, if you think we should remove them. However, just for the record, I didn't see any issues with them.
Another thing is, I couldn't make the tests work with those sleep(4)'s removed as you did in your patch up-thread. In fact all *add_remove* tests failed except the add_remove_user_memcache_mix (delay results are not checked there).
The problem being, the assertions following these delays check enumeration and enumeration timeout is still 4 seconds, so we need to wait for it. For that matter removing tests you mention doesn't have an impact on execution time.
I see where the confusion comes from now. But the enumeration should not play a role here. You can think about enumeration as forced sysdb fill operation, but it is not necessary in order to grab user from LDAP. If the user is not found in the sysdb (and also not found in the negcache) SSSD grabs the user from LDAP even before the next enumeration.
Sure. However, ent.assert_passwd does pwd.getpwall, not only pwd.getpwnam and pwd.getpwuid. Pwd.getpwall result check is the one that fails. Ent.assert_group behaves similarly.
I think I remember Jakub asking to have this test without enumeration as well, and I can do that, but I think enumeration test is also valuable.
I'm not quite sure how you made it work. Was some other change omitted from your patch accidentally? I'm talking about changes like this:
I do not think I omitted something. Did you really set the negative_cache to 0 in sssd.conf?
Yes. I tried the exact patch you attached.
def create_sssd_cleanup(request): @@ -462,12 +464,9 @@ 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", 1001, 2000)
- time.sleep(2) # Add the user ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Here you fill the negative cache.
ldap_conn.add_s(*e)
- ent.assert_passwd(ent.contains_only())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Here you are testing the negative cache. If there was no negcache hit, SSSD would grab the user from LDAP.
Ah, yes, it seems this should fetch from the negative cache.
- time.sleep(4)
^^^^^^^^^^^^^So this sleep should not be necessary if the negcache is disabled.
ent.assert_passwd(ent.contains_only(dict(name="user", uid=1001))) # Remove the user ldap_conn.delete_s(e[0])All in all, setting memcache_timeout to zero and commenting-out memcache file removal was enough to make *all* tests work fine.
That is good! If the patches do not work for you without the sleep()s you can send them with the sleep()s for now and I will take a look.
Please find attached patches with what I have working right now. The remaining work includes:
* Put the 4s timeout into a global variable and use it instead of all the literal timeouts throughout the code. Instead of 2s timeouts use that variable divided by 2 - that's its meaning.
* Attempt refactoring configuration generation inside loops within tests into separate tests with separate fixtures. That concerns the following functions:
test_filter_users test_filter_groups_rfc2307 test_filter_groups_rfc2307_bis
Michal's comment was:
Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.
My response was:
I would really like to do that, but the problem is we will either need to copy-paste a ton of tests and fixtures, or use py.test's convoluted means of sharing parameters between fixtures and tests which will produce much more complicated code that this. At least this is my impression after researching that.
We can try doing that and seeing how it works.
* Consider avoiding any assertions hitting negative cache, and disabling negative cache to possibly increase reliability - Michal's suggestion. Michal, please correct me if I cite it wrong or misunderstood it.
* Check the patch with pep8 and resolve any found issues.
* After the tests are merged, rebase the "intg: Add a memcache invalidation failure test" patch on top of it and attach it to ticket #2726 as the possible regression tracker.
Nick
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
It is an ACK from me.
CI: http://sssd-ci.duckdns.org/logs/job/32/19/summary.html
I removed some tests from Nick's patch that need more refactoring. I will send them as separate patch.
On 11/05/2015 03:27 PM, Nikolai Kondrashov wrote:
On 11/04/2015 05:01 PM, Nikolai Kondrashov wrote:
On 11/04/2015 04:25 PM, Michal Židek wrote:
That is good! If the patches do not work for you without the sleep()s you can send them with the sleep()s for now and I will take a look.
Just for the record, it did not work for me without the sleeps either. It probably worked before, because I had some unsaved changes in code. So Nick was right and the sleeps are necessary because of the enum_cache_timeout.
Please find attached patches with what I have working right now. The remaining work includes:
Put the 4s timeout into a global variable and use it instead of all the literal timeouts throughout the code. Instead of 2s timeouts use that variable divided by 2 - that's its meaning.
Attempt refactoring configuration generation inside loops within tests
into separate tests with separate fixtures. That concerns the following functions:
test_filter_users test_filter_groups_rfc2307 test_filter_groups_rfc2307_bisMichal's comment was:
Instead of generating the conf files in nested for loops, it would be better to split the test to multiple smaller tests where each has different config file fixture (instad of void_conf and void_sssd). This may seem like code duplication, but I believe it will result in better readable code.My response was:
I would really like to do that, but the problem is we will eitherneed to copy-paste a ton of tests and fixtures, or use py.test's convoluted means of sharing parameters between fixtures and tests which will produce much more complicated code that this. At least this is my impression after researching that.
We can try doing that and seeing how it works.
Consider avoiding any assertions hitting negative cache, and disabling negative cache to possibly increase reliability - Michal's suggestion. Michal, please correct me if I cite it wrong or misunderstood it.
Check the patch with pep8 and resolve any found issues.
After the tests are merged, rebase the "intg: Add a memcache invalidation failure test" patch on top of it and attach it to ticket #2726 as the possible regression tracker.
Nick
On (08/11/15 23:49), Michal Židek wrote:
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
Patches for bug with memory cache are already on the list. So it would be good to remove workarounds. Could you prepare new patches on top of mc patches?
LS
On 11/09/2015 09:19 AM, Lukas Slebodnik wrote:
On (08/11/15 23:49), Michal Židek wrote:
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
Patches for bug with memory cache are already on the list. So it would be good to remove workarounds. Could you prepare new patches on top of mc patches?
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
Michal
On 11/11/2015 06:03 PM, Michal Židek wrote:
On 11/09/2015 09:19 AM, Lukas Slebodnik wrote:
On (08/11/15 23:49), Michal Židek wrote:
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
Patches for bug with memory cache are already on the list. So it would be good to remove workarounds. Could you prepare new patches on top of mc patches?
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
Michal
I forgot to say that the memcache fix must be pushed before this.
Michal
On Wed, Nov 11, 2015 at 06:05:02PM +0100, Michal Židek wrote:
On 11/11/2015 06:03 PM, Michal Židek wrote:
On 11/09/2015 09:19 AM, Lukas Slebodnik wrote:
On (08/11/15 23:49), Michal Židek wrote:
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
Patches for bug with memory cache are already on the list. So it would be good to remove workarounds. Could you prepare new patches on top of mc patches?
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
Michal
I forgot to say that the memcache fix must be pushed before this.
Michal
CI passed some time ago: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/3304/
and we agreed off-list that the fixup is a good thing to do.
I'm going to push the patches..
On Sat, Nov 14, 2015 at 01:43:45PM +0100, Jakub Hrozek wrote:
On Wed, Nov 11, 2015 at 06:05:02PM +0100, Michal Židek wrote:
On 11/11/2015 06:03 PM, Michal Židek wrote:
On 11/09/2015 09:19 AM, Lukas Slebodnik wrote:
On (08/11/15 23:49), Michal Židek wrote:
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
Patches for bug with memory cache are already on the list. So it would be good to remove workarounds. Could you prepare new patches on top of mc patches?
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
Michal
I forgot to say that the memcache fix must be pushed before this.
Michal
CI passed some time ago: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/3304/
and we agreed off-list that the fixup is a good thing to do.
I'm going to push the patches..
Squashed into one and pushed to master with Michal's RB: c20811708e584b49ef12ffe1950d71356604bd3b
On (14/11/15 13:50), Jakub Hrozek wrote:
On Sat, Nov 14, 2015 at 01:43:45PM +0100, Jakub Hrozek wrote:
On Wed, Nov 11, 2015 at 06:05:02PM +0100, Michal Židek wrote:
On 11/11/2015 06:03 PM, Michal Židek wrote:
On 11/09/2015 09:19 AM, Lukas Slebodnik wrote:
On (08/11/15 23:49), Michal Židek wrote:
I am attaching patch that can be applied and squashed into Nick's patch. I am sending both patches to see what changes have been made, *please squash them before pushing*.
This patch just removes the negchace (together with the asserts that are testing it) and memcache, and adds global constant to avoid using magic values.
Patches for bug with memory cache are already on the list. So it would be good to remove workarounds. Could you prepare new patches on top of mc patches?
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
Michal
I forgot to say that the memcache fix must be pushed before this.
Michal
CI passed some time ago: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/3304/
and we agreed off-list that the fixup is a good thing to do.
I'm going to push the patches..
Squashed into one and pushed to master with Michal's RB: c20811708e584b49ef12ffe1950d71356604bd3b
and also pushed to sssd-1-13: * 3ab0400da8d6f140a2f3c99c13ea35ed75861823
LS
Hi Jakub, Michal, everyone,
On 11/11/2015 07:03 PM, Michal Židek wrote:
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn,
From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch
-def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn,
These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet.
What shall we do with them?
Nick
On 01/21/2016 02:53 PM, Nikolai Kondrashov wrote:
Hi Jakub, Michal, everyone,
On 11/11/2015 07:03 PM, Michal Židek wrote:
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn,
From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch
-def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn,
These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet.
What shall we do with them?
Nick
Hi Nick,
I have this in my todo list, but I did not started with it yet. I will probably not get to it anytime soon.
Feel free to create a ticket and assign it to me. We can triage it next week.
Michal
On Thu, Jan 21, 2016 at 03:53:19PM +0200, Nikolai Kondrashov wrote:
Hi Jakub, Michal, everyone,
On 11/11/2015 07:03 PM, Michal Židek wrote:
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn,
From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch
-def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn,
These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet.
What shall we do with them?
I looked at the integration tests today when I tried to write regression test for a bug I found and I realized the current tests use enumeration.
It's very useful to test that codepath also, but non-enumeration is even more useful. Perhaps we'll find some time next week to hack on the tests so we can add the same ones, just with non-enum..
On (29/01/16 10:22), Jakub Hrozek wrote:
On Thu, Jan 21, 2016 at 03:53:19PM +0200, Nikolai Kondrashov wrote:
Hi Jakub, Michal, everyone,
On 11/11/2015 07:03 PM, Michal Židek wrote:
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn,
From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch
-def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn,
These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet.
What shall we do with them?
I looked at the integration tests today when I tried to write regression test for a bug I found and I realized the current tests use enumeration.
It's very useful to test that codepath also, but non-enumeration is even more useful. Perhaps we'll find some time next week to hack on the tests so we can add the same ones, just with non-enum..
I think you ment ldap_tests. Test for memory cache does not use enumeration :-)
LS
On 01/29/2016 11:22 AM, Jakub Hrozek wrote:
On Thu, Jan 21, 2016 at 03:53:19PM +0200, Nikolai Kondrashov wrote:
Hi Jakub, Michal, everyone,
On 11/11/2015 07:03 PM, Michal Židek wrote:
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn,
From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch
-def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn,
These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet.
What shall we do with them?
I looked at the integration tests today when I tried to write regression test for a bug I found and I realized the current tests use enumeration.
It's very useful to test that codepath also, but non-enumeration is even more useful. Perhaps we'll find some time next week to hack on the tests so we can add the same ones, just with non-enum..
Yeah, that would be good, I can do join this if we have time.
Nick
If you guys happen to chat about upstream tests at all and have enough time to let me know ahead of time, I'd love to dial in and participate.
Dan
On 1/29/16 6:34 AM, Nikolai Kondrashov wrote:
On 01/29/2016 11:22 AM, Jakub Hrozek wrote:
On Thu, Jan 21, 2016 at 03:53:19PM +0200, Nikolai Kondrashov wrote:
Hi Jakub, Michal, everyone,
On 11/11/2015 07:03 PM, Michal Židek wrote:
I modified the "squashing" patch to remove the workaround for failing memcache.
SQUASH THESE 2 ATTACHED PATCHES BEFORE PUSHING and use the commit message from Nick's (first) patch (I altered the message a little so that the removed test are not mentioned).
CI link passed: http://sssd-ci.duckdns.org/logs/job/32/83/summary.html
From 904a94d406eaaa2a0f8389951b007552bfc04e1d Mon Sep 17 00:00:00 2001 From: Nikolai KondrashovNikolai.Kondrashov@redhat.com Date: Tue, 29 Sep 2015 21:18:18 +0300 Subject: [PATCH 5/6] intg: Add more LDAP tests
+def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, +def test_filter_groups_rfc2307(request, ldap_conn, +def test_filter_groups_rfc2307_bis(request, ldap_conn,
From da10890813f315826e60088fb938e581df504656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Sun, 8 Nov 2015 22:17:44 +0100 Subject: [PATCH 6/6] Fixup this to Nick's patch
-def test_filter_users(request, ldap_conn, three_users_three_groups_rfc2307, -def test_filter_groups_rfc2307(request, ldap_conn, -def test_filter_groups_rfc2307_bis(request, ldap_conn,
These three tests didn't make it into our integration tests. IIRC, Michal wanted to refactor them, but I assume he had no time yet.
What shall we do with them?
I looked at the integration tests today when I tried to write regression test for a bug I found and I realized the current tests use enumeration.
It's very useful to test that codepath also, but non-enumeration is even more useful. Perhaps we'll find some time next week to hack on the tests so we can add the same ones, just with non-enum..
Yeah, that would be good, I can do join this if we have time.
Nick _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
sssd-devel@lists.fedorahosted.org
-
Dan Lavu -
Jakub Hrozek -
Lukas Slebodnik -
Michal Židek -
Nikolai Kondrashov -
Pavel Reichl