10:15 a.m.
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
10:45 a.m.
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
11:21 a.m.
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-in...
Nick
4:27 a.m.
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_...
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(a)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(a)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):
user1_case1 = 'User1@LDAP'
user1_case2 = 'uSer1@LDAP'
user1_case_last = 'usEr1@LDAP'
@@ -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):
user1_case1 = 'usEr1@LDAP'
user1_case2 = 'User1@LDAP'
user1_case_last = 'uSer1@LDAP'
@@ -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):
user1_case1 = 'uSer1@LDAP'
user1_case2 = 'usEr1@LDAP'
user1_case_last = 'User1@LDAP'
@@ -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(a)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(a)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:
- break
- time.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)
- 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:
+ break
+ time.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(a)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 = rfc2307bis
+ ldap_group_object_class = groupOfNames
+ """)
+ else:
+ schema_conf = unindent("""\
+ ldap_schema = rfc2307
+ """)
+ return unindent("""\
+ [sssd]
+ debug_level = 0xffff
+ domains = LDAP
+ services = nss, pam
+
+ [nss]
+ debug_level = 0xffff
+ memcache_timeout = 0
+
+ [pam]
+ debug_level = 0xffff
+
+ [domain/LDAP]
+ ldap_auth_disable_tls_never_use_in_production = true
+ debug_level = 0xffff
+ enumerate = {enum}
+ {schema_conf}
+ id_provider = ldap
+ auth_provider = ldap
+ ldap_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 = 0xffff
- domains = LDAP
- services = nss, pam
-
- [nss]
- debug_level = 0xffff
- memcache_timeout = 0
-
- [pam]
- debug_level = 0xffff
-
- [domain/LDAP]
- ldap_auth_disable_tls_never_use_in_production = true
- debug_level = 0xffff
- enumerate = true
- ldap_schema = rfc2307
- id_provider = ldap
- auth_provider = ldap
- sudo_provider = ldap
- ldap_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 = 2
- domains = LDAP
- services = nss, pam
-
- [nss]
-
- [pam]
-
- [domain/LDAP]
- ldap_auth_disable_tls_never_use_in_production = true
- ldap_schema = rfc2307
- id_provider = ldap
- auth_provider = ldap
- ldap_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 = 0xffff
- domains = LDAP
- services = nss, pam
-
- [nss]
- debug_level = 0xffff
- memcache_timeout = 0
-
- [pam]
- debug_level = 0xffff
-
- [domain/LDAP]
- ldap_auth_disable_tls_never_use_in_production = true
- debug_level = 0xffff
- enumerate = true
- ldap_schema = rfc2307bis
- ldap_group_object_class = groupOfNames
- id_provider = ldap
- auth_provider = ldap
- sudo_provider = ldap
- ldap_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 = 2
- domains = LDAP
- services = nss
-
- [nss]
- memcache_timeout = 0
-
- [domain/LDAP]
- ldap_auth_disable_tls_never_use_in_production = true
- debug_level = 0xffff
- ldap_schema = rfc2307bis
- ldap_group_object_class = groupOfNames
- id_provider = ldap
- auth_provider = ldap
- sudo_provider = ldap
- ldap_uri = {ldap_conn.ds_inst.ldap_url}
- ldap_search_base = {ldap_conn.ds_inst.base_dn}
-
- entry_cache_user_timeout = 1
- entry_cache_group_timeout = 5000
- ldap_purge_cache_timeout = 3
-
- """).format(**locals())
+ conf = \
+ format_basic_conf(ldap_conn, True, False) + \
+ unindent("""
+ [domain/LDAP]
+ entry_cache_user_timeout = 1
+ entry_cache_group_timeout = 5000
+ ldap_purge_cache_timeout = 3
+ """).format(**locals())
create_conf_fixture(request, conf)
create_sssd_fixture(request)
return None
--
2.5.1
From e1feef5e13125301e0281de102f56b2301c5cc89 Mon Sep 17 00:00:00
2001
From: Nikolai Kondrashov <Nikolai.Kondrashov(a)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 for
- uid in member_uids
- ])
- )
- if len(member_gids) > 0:
- attr_list.append(
- ('member', [
- "cn=" + gid + ",ou=Groups," + base_dn for
- gid 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))
return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
--
2.5.1
From 72222b8fbc565de66874cfaa62ea0213ebf0f5a9 Mon Sep 17 00:00:00
2001
From: Nikolai Kondrashov <Nikolai.Kondrashov(a)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 = 4
+ enum_cache_timeout = 4
+ entry_negative_timeout = 4
+
+ [domain/LDAP]
+ ldap_enumeration_refresh_timeout = 4
+ ldap_purge_cache_timeout = 1
+ entry_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")))
+
+
+(a)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)
+
+
+(a)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)
+
+
+(a)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
+
+
+(a)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()))
+
+
+(a)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)
+
+
+(a)pytest.fixture
+def void_conf(request):
+ create_conf_cleanup(request)
+
+
+(a)pytest.fixture
+def void_sssd(request):
+ create_sssd_cleanup(request)
+
+
+(a)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_users
+ filter_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_groups
+ filter_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()
+
+
+(a)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_groups
+ filter_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()
+
+
+(a)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")
+ )
+ )
+
+
+(a)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")
+ )
+ )
+
+
+(a)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")
+ )
+ )
+
+
+(a)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/fallback
+ allowed_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="")
+ )
+ )
+
+
+(a)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/default
+ allowed_shells = /bin/default, /bin/not_installed
+ shell_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")
+ )
+ )
+
+
+(a)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/default
+ vetoed_shells = /bin/vetoed
+ shell_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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
2:51 a.m.
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_...
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(a)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
9:09 a.m.
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
8:02 a.m.
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_...
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
8:13 a.m.
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_...
>
>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) ?
8:56 a.m.
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
12:51 p.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
2:32 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
3:33 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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.
3:39 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
8:29 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
8:48 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
8:50 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 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.
+1
LS
8:50 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 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.
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
8:55 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 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.
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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
6:39 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
8:31 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
7:04 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 more
tests
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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
9:07 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 more
> tests
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
6:54 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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 = 4
+ entry_negative_timeout = 4
I would set negative cache timeout to zero as well,
see comments below.
+
+ [domain/LDAP]
+ ldap_enumeration_refresh_timeout = 4
+ ldap_purge_cache_timeout = 1
+ entry_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")))
+
+
+(a)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)
+
+
+(a)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)
+
+
+(a)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
+
+
+(a)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()))
> +
> +
> +(a)pytest.fixture
> +def void_conf(request):
> + create_conf_cleanup(request)
> +
> +
> +(a)pytest.fixture
> +def void_sssd(request):
> + create_sssd_cleanup(request)
> +
> +
> +(a)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_users
> + filter_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_users
+ else 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_groups
+ filter_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()
+
+
+(a)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_groups
+ filter_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()
+
+
+(a)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")
+ )
+ )
+
+
+(a)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")
+ )
+ )
+
+
+(a)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")
+ )
+ )
+
+
+(a)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/fallback
+ allowed_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="")
+ )
+ )
+
+
+(a)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/default
+ allowed_shells = /bin/default, /bin/not_installed
+ shell_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")
+ )
+ )
+
+
+(a)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/default
+ vetoed_shells = /bin/vetoed
+ shell_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 Kondrashov<Nikolai.Kondrashov(a)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 = 0xffff
> + domains = LDAP
> + services = nss, pam
> +
> + [nss]
> + memcache_timeout = 0
> +
> + [pam]
> +
> + [domain/LDAP]
> + ldap_auth_disable_tls_never_use_in_production = true
> + debug_level = 0xffff
> + ldap_schema = rfc2307
> + enumerate = true
> + id_provider = ldap
> + auth_provider = ldap
> + ldap_uri = {ldap_conn.ds_inst.ldap_url}
> + ldap_search_base = {ldap_conn.ds_inst.base_dn}
> +
> + ldap_enumeration_refresh_timeout = 4
> + ldap_purge_cache_timeout = 1
> + entry_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 = LDAP
> + services = nss
> +
> + [nss]
> + debug_level = 0xffff
> +
> + [domain/LDAP]
> + ldap_auth_disable_tls_never_use_in_production = true
> + ldap_schema = rfc2307
> + id_provider = ldap
> + auth_provider = ldap
> + sudo_provider = ldap
> + ldap_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:
> + break
> + time.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)
> +
> +
> +(a)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
10:10 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 = 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.
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 = 4
> + entry_negative_timeout = 4
I 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
9:52 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 = 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.
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 = 4
>> + entry_negative_timeout = 4
>
> I 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.
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
11:30 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 = 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.
>
> 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 = 4
>>> + entry_negative_timeout = 4
>>
>> I 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.
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
4:10 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
8:25 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 = 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.
>>
>> 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 = 4
>>>> + entry_negative_timeout = 4
>>>
>>> I 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.
>
> 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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
9:01 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
8:27 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
4:49 p.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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_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
2:19 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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
11:03 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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(a)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
11:05 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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(a)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
6:43 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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(a)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..
6:50 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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(a)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
3:53 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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(a)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
7:53 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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(a)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
9:31 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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(a)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
3:22 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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(a)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..
4:44 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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(a)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
5:34 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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(a)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
11:43 a.m.
New subject: [PATCH v2] intg: Add more LDAP tests
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 Kondrashov<Nikolai.Kondrashov(a)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(a)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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
3002
days inactive
3124
days old
sssd-devel@lists.fedorahosted.org
43 comments
6 participants
participants (6)
-
Dan Lavu -
Jakub Hrozek -
Lukas Slebodnik -
Michal Židek -
Nikolai Kondrashov -
Pavel Reichl