On 10/26/2015 06:05 PM, Martin Basti wrote:


On 26.10.2015 09:01, Lenka Doudova wrote:


On 10/21/2015 06:53 AM, Lenka Doudova wrote:


On 10/20/2015 06:21 PM, Martin Basti wrote:


On 20.10.2015 15:53, Martin Basti wrote:


On 19.10.2015 14:16, Martin Basti wrote:


On 19.10.2015 12:30, Martin Basti wrote:
Attribute nsaccountlock has not been processed correctly

Patch attached.



Self-NACK, more fixes required



Updated patch attached, but it still needs to improve because tests in my patch 331 are still failing.


Eternal self-NACK for this patch

I'm not able to fix UserTracker, I need help from somebody with higher view of how this tracker is supposed to work.
Follow my patch 0331

Hi, I'll take a look at it today.
Lenka


Hi,

I fixed the trackers and tests, rebased patch attached.
Lenka



Thank you,

1)
************* Module ipatests.test_xmlrpc.test_stageuser_plugin
ipatests/test_xmlrpc/test_stageuser_plugin.py:938: [E0102(function-redefined), TestMultipleManagers] class already defined line 913)

2)
Because the patch contains tests too, I suggest to rename patch to Multiple manager per user tests.
Also you should change commiter of patch to you.

Martin^2
Fixed patch attached.
Lenka
From aca3dfb072c8bb86efd7fe247157d630a332e691 Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Tue, 3 Nov 2015 10:03:15 +0100
Subject: [PATCH] Multiple manager per user tests

Multiple managers per user tests
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py |  52 ++++++++++++-
 ipatests/test_xmlrpc/test_user_plugin.py      | 103 +++++++++++++++++++++-----
 2 files changed, 134 insertions(+), 21 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index b09ef6e84cd95a32061b07d833c5a39f1750f80b..bd3e790fb4c318f449a9e36763245f2ea7f39924 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -100,10 +100,10 @@ class StageUserTracker(Tracker):
         u'usercertificate', u'dn', u'has_keytab', u'has_password',
         u'street', u'postalcode', u'facsimiletelephonenumber',
         u'carlicense', u'ipasshpubkey', u'sshpubkeyfp', u'l',
-        u'st', u'mobile', u'pager', }
+        u'st', u'mobile', u'pager', u'manager'}
     retrieve_all_keys = retrieve_keys | {
         u'cn', u'ipauniqueid', u'objectclass', u'description',
-        u'displayname', u'gecos', u'initials', u'krbprincipalname', u'manager'}
+        u'displayname', u'gecos', u'initials', u'krbprincipalname'}
 
     create_keys = retrieve_all_keys | {
         u'objectclass', u'ipauniqueid', u'randompassword',
@@ -184,7 +184,12 @@ class StageUserTracker(Tracker):
                     (self.kwargs[key].split('@'))[0].lower(),
                     (self.kwargs[key].split('@'))[1])]
             elif key == u'manager':
-                self.attrs[key] = [unicode(get_user_dn(self.kwargs[key]))]
+                if isinstance(self.kwargs[key], list):
+                    self.attrs[key] = [
+                        unicode(get_user_dn(item))
+                        for item in self.kwargs[key]]
+                else:
+                    self.attrs[key] = [unicode(get_user_dn(self.kwargs[key]))]
             elif key == u'ipasshpubkey':
                 self.attrs[u'sshpubkeyfp'] = [sshpubkeyfp]
                 self.attrs[key] = [self.kwargs[key]]
@@ -891,3 +896,44 @@ class TestGroups(XMLRPC_test):
         command = group.make_add_member_command(options={u'user': user.uid})
         result = command()
         group.check_add_member_negative(result)
+
+
+@pytest.fixture(scope='class')
+def manager1(request):
+    t = UserTracker(u"manager1", u"manager", u"manager1")
+    return t.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def manager2(request):
+    t = UserTracker(u"manager2", u"manager", u"manager2")
+    return t.make_fixture(request)
+
+
+class TestMultipleManagers(XMLRPC_test):
+    """Tests for: https://fedorahosted.org/freeipa/ticket/5344""";
+    def test_multiple_managers_per_stageduser(self, manager1, manager2,
+                                              stageduser):
+        manager1.create()
+        manager2.create()
+        stageduser.create()
+
+        stageduser.update(
+            updates={u"manager": [manager1.name, manager2.name]},
+            expected_updates={u"manager": [manager1.name, manager2.name]})
+
+    def test_find_stageuser_with_multiple_managers(self, manager1, manager2,
+                                                   stageduser):
+        command = stageduser.make_find_command(
+            manager=[manager1.name, manager2.name])
+        result = command()
+        stageduser.check_find(result)
+
+    def test_create_new_stageduser_with_multiple_managers(
+            self, manager1, manager2, stageduser):
+        stageduser.ensure_missing()
+        command = stageduser.make_create_command(
+            options={u'manager': [manager1.name, manager2.name]})
+        result = command()
+        stageduser.track_create()
+        stageduser.check_create(result)
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 3d7b5e6ba14e3ccb144575f52e4e503e6638037d..2e525c28afbc16dea737efaa30887cc48089b7f9 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -27,6 +27,7 @@ import functools
 import datetime
 import ldap
 import re
+import pytest
 
 from ipalib import api, errors
 from ipatests.test_xmlrpc import objectclasses
@@ -1654,7 +1655,7 @@ class test_denied_bind_with_expired_principal(XMLRPC_test):
 
 
 class UserTracker(Tracker):
-    """ Class for host plugin like tests """
+    """ Class for user plugin like tests """
 
     retrieve_keys = {
         u'uid', u'givenname', u'sn', u'homedirectory',
@@ -1666,17 +1667,18 @@ class UserTracker(Tracker):
         u'has_password', u'street', u'postalcode', u'facsimiletelephonenumber',
         u'carlicense', u'ipasshpubkey', u'sshpubkeyfp', u'nsaccountlock',
         u'preserved', u'memberof_group', u'l', u'mobile', u'krbextradata',
-        u'krblastpwdchange', u'krbpasswordexpiration', u'pager', u'st'
+        u'krblastpwdchange', u'krbpasswordexpiration', u'pager', u'st',
+        u'manager'
         }
 
     retrieve_all_keys = retrieve_keys | {
         u'cn', u'ipauniqueid', u'objectclass', u'mepmanagedentry',
-        u'displayname', u'gecos', u'initials', u'krbprincipalname', u'manager'}
+        u'displayname', u'gecos', u'initials', u'krbprincipalname'}
 
     retrieve_preserved_keys = retrieve_keys - {u'memberof_group'}
     retrieve_preserved_all_keys = retrieve_all_keys - {u'memberof_group'}
 
-    create_keys = retrieve_all_keys | {
+    create_keys = (retrieve_all_keys - {u'nsaccountlock'}) | {
         u'randompassword', u'mepmanagedentry',
         u'krbextradata', u'krbpasswordexpiration', u'krblastpwdchange',
         u'krbprincipalkey', u'randompassword', u'userpassword'
@@ -1697,8 +1699,20 @@ class UserTracker(Tracker):
 
         self.kwargs = kwargs
 
-    def make_create_command(self, force=None):
+    def _fix_nsaccountlock_attr(self, result):
+        # small override because user-* commands returns different type
+        # of nsaccountlock value than DS, but overall the value fits
+        # expected result
+        if u'nsaccountlock' in result:
+            if result[u'nsaccountlock'] == [u'true']:
+                result[u'nsaccountlock'] = True
+            elif result[u'nsaccountlock'] == [u'false']:
+                result[u'nsaccountlock'] = False
+
+    def make_create_command(self, options=None, force=None):
         """ Make function that crates a user using user-add """
+        if options is not None:
+            self.kwargs = options
         return self.make_command(
             'user_add', self.uid,
             givenname=self.givenname,
@@ -1773,6 +1787,7 @@ class UserTracker(Tracker):
             has_password=False,
             mepmanagedentry=[get_group_dn(self.uid)],
             memberof_group=[u'ipausers'],
+            nsaccountlock=[u'false']
             )
 
         for key in self.kwargs:
@@ -1781,6 +1796,13 @@ class UserTracker(Tracker):
                     (self.kwargs[key].split('@'))[0].lower(),
                     (self.kwargs[key].split('@'))[1]
                     )]
+            if key == u'manager':
+                if isinstance(self.kwargs[key], list):
+                    self.attrs[key] = [
+                        unicode(get_user_dn(item))
+                        for item in self.kwargs[key]]
+                else:
+                    self.attrs[key] = [unicode(get_user_dn(self.kwargs[key]))]
             else:
                 self.attrs[key] = [self.kwargs[key]]
 
@@ -1816,14 +1838,7 @@ class UserTracker(Tracker):
         else:
             expected = self.filter_attrs(self.retrieve_keys)
 
-        # small override because stageuser-find returns different type
-        # of nsaccountlock value than DS, but overall the value fits
-        # expected result
-        if u'nsaccountlock' in expected:
-            if expected[u'nsaccountlock'] == [u'true']:
-                expected[u'nsaccountlock'] = True
-            elif expected[u'nsaccountlock'] == [u'false']:
-                expected[u'nsaccountlock'] = False
+        self._fix_nsaccountlock_attr(expected)
 
         assert_deepequal(dict(
             value=self.uid,
@@ -1833,14 +1848,13 @@ class UserTracker(Tracker):
 
     def check_find(self, result, all=False, raw=False):
         """ Check 'user-find' command result """
-        self.attrs[u'nsaccountlock'] = True
-        self.attrs[u'preserved'] = True
-
         if all:
             expected = self.filter_attrs(self.find_all_keys)
         else:
             expected = self.filter_attrs(self.find_keys)
 
+        self._fix_nsaccountlock_attr(expected)
+
         assert_deepequal(dict(
             count=1,
             truncated=False,
@@ -1859,10 +1873,14 @@ class UserTracker(Tracker):
 
     def check_update(self, result, extra_keys=()):
         """ Check 'user-mod' command result """
+        expected = self.filter_attrs(self.update_keys | set(extra_keys))
+
+        self._fix_nsaccountlock_attr(expected)
+
         assert_deepequal(dict(
             value=self.uid,
             summary=u'Modified user "%s"' % self.uid,
-            result=self.filter_attrs(self.update_keys | set(extra_keys))
+            result=expected,
         ), result)
 
     def create_from_staged(self, stageduser):
@@ -1920,7 +1938,7 @@ class UserTracker(Tracker):
             ), result)
 
     def track_delete(self, preserve=False):
-        """Update expected state for host deletion"""
+        """Update expected state for user deletion"""
         if preserve:
             self.exists = True
             if u'memberof_group' in self.attrs:
@@ -1980,3 +1998,52 @@ class UserTracker(Tracker):
         request.addfinalizer(finish)
 
         return self
+
+
+@pytest.fixture(scope='class')
+def manager1(request):
+    t = UserTracker(u"manager1", u"manager", u"manager1")
+    return t.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def manager2(request):
+    t = UserTracker(u"manager2", u"manager", u"manager2")
+    return t.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def testuser(request):
+    t = UserTracker(u"user", u"test", u"user")
+    return t.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def testuser2(request):
+    t = UserTracker(u"user2", u"test", u"user2")
+    return t.make_fixture(request)
+
+
+class TestMultipleManagers(XMLRPC_test):
+    """Tests for: https://fedorahosted.org/freeipa/ticket/5344""";
+    def test_multiple_managers_per_user(self, manager1, manager2, testuser):
+        manager1.create()
+        manager2.create()
+        testuser.create()
+
+        testuser.update({u"manager": [manager1.name, manager2.name]})
+
+    def test_find_user_with_multiple_managers(self, manager1, manager2,
+                                              testuser):
+        command = testuser.make_find_command(
+            manager=[manager1.name, manager2.name])
+        result = command()
+        testuser.check_find(result)
+
+    def test_create_new_user_with_multiple_managers(
+            self, manager1, manager2, testuser2):
+        command = testuser2.make_create_command(
+            options={u"manager": [manager1.name, manager2.name]})
+        result = command()
+        testuser2.track_create()
+        testuser2.check_create(result)
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to