Hi,

attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387.

Lenka

On 11/20/2015 12:13 PM, Martin Babinsky wrote:
On 11/19/2015 10:34 AM, Petr Viktorin wrote:
On 11/19/2015 09:30 AM, Lenka Doudova wrote:
On 11/18/2015 04:51 PM, Martin Babinsky wrote:
On 11/18/2015 02:16 PM, Lenka Doudova wrote:
Hi,

here's a patch that adds a few comments to stageuser tests in order to
allow easier determining of a problem when tests fail.

Lenka



Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names anyway.

I am also not sure if this patch is worth reviewing and pushing as it
IMHO doesn't help in the identification of failed tests at all.

This should be solved at more fundamental level.

Ouch, good point, I didn't realize. Sorry.

Anyway, the issue is that even if tests are run in verbose mode, you get
output like this:

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27]
PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28]
PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29]
PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210]
PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211]
PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212]
PASSED


If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and
finding which parameters were put into the command, which was not much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test cases, one
for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I don't know
of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.

Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would make
the output even more confusing than the numbers.)

Please try the attached patch.
Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture



This looks like a better approach IMHO, you can then see which attribute/value was being checked.

I would very much favor more descriptive test/fixture names in the beginning.


From 163bbd04cf02d278b3d4ef8d0ee91878270e4377 Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Mon, 23 Nov 2015 10:27:07 +0100
Subject: [PATCH] Adding descriptive IDs to stageuser tests

Adding descriptive IDs to parametrized stageuser test for better identification of test cases.
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py    | 30 ++++++++++++++++++------
 ipatests/test_xmlrpc/tracker/stageuser_plugin.py |  2 +-
 ipatests/test_xmlrpc/tracker/user_plugin.py      |  9 +++++--
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..bda738b786f99296deaeb192e84965bb3e97cfea 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -83,6 +83,16 @@ options_ok = [
     {u'random': True},
     ]
 
+options_ids = [
+    'full name', 'initials', 'display name', 'home directory', 'GECOS',
+    'shell', 'email address', 'job title', 'kerberos principal',
+    'uppercase kerberos principal', 'street address', 'city', 'state',
+    'zip code', 'telephone number', 'fax number', 'mobile tel. number',
+    'pager number', 'organizational unit', 'car license', 'SSH key',
+    'manager', 'user ID number', 'group ID number', 'UID and GID numbers',
+    'password', 'random password'
+    ]
+
 
 @pytest.fixture(scope='class')
 def stageduser(request):
@@ -90,13 +100,19 @@ def stageduser(request):
     return tracker.make_fixture(request)
 
 
-@pytest.fixture(scope='class', params=options_ok)
+@pytest.fixture(scope='class', params=options_ok, ids=options_ids)
 def stageduser2(request):
     tracker = StageUserTracker(u'suser2', u'staged', u'user', **request.param)
     return tracker.make_fixture_activate(request)
 
 
 @pytest.fixture(scope='class')
+def user_activated(request):
+    tracker = UserTracker(u'suser2', u'staged', u'user')
+    return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
 def stageduser3(request):
     tracker = StageUserTracker(name=u'suser3', givenname=u'staged', sn=u'user')
     return tracker.make_fixture_activate(request)
@@ -220,7 +236,7 @@ class TestStagedUser(XMLRPC_test):
     def test_showall_stageduser(self, stageduser):
         stageduser.retrieve(all=True)
 
-    def test_create_attr(self, stageduser2, user, user6):
+    def test_create_with_attr(self, stageduser2, user, user_activated):
         """ Tests creating a user with various valid attributes listed
         in 'options_ok' list"""
         # create staged user with specified parameters
@@ -233,14 +249,14 @@ class TestStagedUser(XMLRPC_test):
 
         # activate user, verify that specified values were preserved
         # after activation
-        user6.ensure_missing()
-        user6 = UserTracker(
+        user_activated.ensure_missing()
+        user_activated = UserTracker(
             stageduser2.uid, stageduser2.givenname,
             stageduser2.sn, **stageduser2.kwargs)
-        user6.create_from_staged(stageduser2)
+        user_activated.create_from_staged(stageduser2)
         command = stageduser2.make_activate_command()
         result = command()
-        user6.check_activate(result)
+        user_activated.check_activate(result)
 
         # verify the staged user does not exist after activation
         command = stageduser2.make_retrieve_command()
@@ -248,7 +264,7 @@ class TestStagedUser(XMLRPC_test):
                 reason=u'%s: stage user not found' % stageduser2.uid)):
             command()
 
-        user6.delete()
+        user_activated.delete()
 
     def test_delete_stageduser(self, stageduser):
         stageduser.delete()
diff --git a/ipatests/test_xmlrpc/tracker/stageuser_plugin.py b/ipatests/test_xmlrpc/tracker/stageuser_plugin.py
index 09ad282e417058f40f1e7d0b9ae36e90c78af8e1..0f7eadd040d5fd12958a0651ddf00cbc4055fa1f 100644
--- a/ipatests/test_xmlrpc/tracker/stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/stageuser_plugin.py
@@ -128,7 +128,7 @@ 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]))]
+                self.attrs[key] = [self.kwargs[key]]
             elif key == u'ipasshpubkey':
                 self.attrs[u'sshpubkeyfp'] = [sshpubkeyfp]
                 self.attrs[key] = [self.kwargs[key]]
diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py
index af7d85836a5c514c4e208696398a68ab341e825f..bcae2ec787b3d3a54e34917f125be006094e07b0 100644
--- a/ipatests/test_xmlrpc/tracker/user_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/user_plugin.py
@@ -5,9 +5,10 @@
 from ipalib import api, errors
 from ipapython.dn import DN
 
-from ipatests.util import assert_deepequal, get_group_dn
+from ipatests.util import assert_deepequal, get_group_dn, get_user_dn
 from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_uuid, raises_exact
+from ipatests.test_xmlrpc.xmlrpc_test import (
+    fuzzy_digits, fuzzy_uuid, raises_exact)
 from ipatests.test_xmlrpc.tracker.base import Tracker
 
 
@@ -258,6 +259,10 @@ class UserTracker(Tracker):
             summary=u'Stage user %s activated' % self.uid,
             result=self.filter_attrs(self.activate_keys))
 
+        if 'manager' in expected['result']:
+            expected['result']['manager'] = [
+                unicode(get_user_dn(expected['result']['manager'][0]))]
+
         # work around to eliminate inconsistency in returned objectclass
         # (case sensitive assertion)
         expected['result']['objectclass'] = [item.lower() for item in
-- 
2.4.3

From 7aaf118d09fd35a47840e4fe69ebb544a97f6ca5 Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Wed, 9 Dec 2015 08:49:53 +0100
Subject: [PATCH] Adding descriptive IDs to stageuser tests

Adding descriptive IDs to parametrized stageuser test for better identification of test cases.
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py | 32 ++++++++++++++++++++-------
 ipatests/test_xmlrpc/test_user_plugin.py      |  4 ++++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index d645c4470d28c845bc609bdc6cb3a2ca7d4ca004..123da2b6b70433a74ede7ec8dd08cbf83e84441a 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -78,6 +78,16 @@ options_ok = [
     {u'random': True},
     ]
 
+options_ids = [
+    'full name', 'initials', 'display name', 'home directory', 'GECOS',
+    'shell', 'email address', 'job title', 'kerberos principal',
+    'uppercase kerberos principal', 'street address', 'city', 'state',
+    'zip code', 'telephone number', 'fax number', 'mobile tel. number',
+    'pager number', 'organizational unit', 'car license', 'SSH key',
+    'manager', 'user ID number', 'group ID number', 'UID and GID numbers',
+    'password', 'random password'
+    ]
+
 
 class StageUserTracker(Tracker):
     """ Tracker class for staged user LDAP object
@@ -179,7 +189,7 @@ 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]))]
+                self.attrs[key] = [self.kwargs[key]]
             elif key == u'ipasshpubkey':
                 self.attrs[u'sshpubkeyfp'] = [sshpubkeyfp]
                 self.attrs[key] = [self.kwargs[key]]
@@ -324,13 +334,19 @@ def stageduser(request):
     return tracker.make_fixture(request)
 
 
-@pytest.fixture(scope='class', params=options_ok)
+@pytest.fixture(scope='class', params=options_ok, ids=options_ids)
 def stageduser2(request):
     tracker = StageUserTracker(u'suser2', u'staged', u'user', **request.param)
     return tracker.make_fixture_activate(request)
 
 
 @pytest.fixture(scope='class')
+def user_activated(request):
+    tracker = UserTracker(u'suser2', u'staged', u'user')
+    return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
 def stageduser3(request):
     tracker = StageUserTracker(name=u'suser3', givenname=u'staged', sn=u'user')
     return tracker.make_fixture_activate(request)
@@ -454,7 +470,7 @@ class TestStagedUser(XMLRPC_test):
     def test_showall_stageduser(self, stageduser):
         stageduser.retrieve(all=True)
 
-    def test_create_attr(self, stageduser2, user, user6):
+    def test_create_with_attr(self, stageduser2, user, user_activated):
         """ Tests creating a user with various valid attributes listed
         in 'options_ok' list"""
         # create staged user with specified parameters
@@ -467,14 +483,14 @@ class TestStagedUser(XMLRPC_test):
 
         # activate user, verify that specified values were preserved
         # after activation
-        user6.ensure_missing()
-        user6 = UserTracker(
+        user_activated.ensure_missing()
+        user_activated = UserTracker(
             stageduser2.uid, stageduser2.givenname,
             stageduser2.sn, **stageduser2.kwargs)
-        user6.create_from_staged(stageduser2)
+        user_activated.create_from_staged(stageduser2)
         command = stageduser2.make_activate_command()
         result = command()
-        user6.check_activate(result)
+        user_activated.check_activate(result)
 
         # verify the staged user does not exist after activation
         command = stageduser2.make_retrieve_command()
@@ -482,7 +498,7 @@ class TestStagedUser(XMLRPC_test):
                 reason=u'%s: stage user not found' % stageduser2.uid)):
             command()
 
-        user6.delete()
+        user_activated.delete()
 
     def test_delete_stageduser(self, stageduser):
         stageduser.delete()
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 81185e449acaa127aa9429fff9587d39a2be81e6..cce769f8f31fc05df218fd4f5afa8978bce091be 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -1903,6 +1903,10 @@ class UserTracker(Tracker):
             summary=u'Stage user %s activated' % self.uid,
             result=self.filter_attrs(self.activate_keys))
 
+        if 'manager' in expected['result']:
+            expected['result']['manager'] = [
+                unicode(get_user_dn(expected['result']['manager'][0]))]
+
         # work around to eliminate inconsistency in returned objectclass
         # (case sensitive assertion)
         expected['result']['objectclass'] = [item.lower() for item in
-- 
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