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