Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-08-08 Thread Lenka Doudova 

On 07/20/2016 05:31 PM, Peter Lacko wrote:

Sorry for late reply, I was waiting how the discussion with tracker improvement 
will end, but since there's no progress and I'm leaving soon, I'm attaching new 
patch. I also created mapping between old and new tests [1], to make life of 
reviewer easier. Number in first column denotes line number in original file, 
followed by line number in new tests file and its name. Tests that are not 
mentioned in there extend previous coverage.

Regards,
Peter

[1] http://etherpad.corp.redhat.com/Vk3tRLaPSK


Hi,

review notes:

1) code related:

 * leftover PEP8 error:
   ./ipatests/test_xmlrpc/test_role_plugin.py:696:1: W391 blank line at
   end of file
 * in PrivilegeTracker:
 o creating privilege with description does not work properly,
   since there's a hardcoded value in track_create method
 o the check_retrieve method expects 'description' to be returned
   only when privilege is member of a role, but to the extent of my
   knowledge that value is returned always
 * in RoleTracker:
 o global variable 'member_types' is used only inside the class, so
   it should be a class variable rather than global
 o 'check_add_duplicate_member' method uses oneliner to create
   basis of expected value - suggest to use this more in other
   methods that do it 'literally' (e.g. check_add_member)
 o 'update_tracker' method - the main else statement should be
   investigated more, I'm not sure the try-except part is valid (if
   I expect the tracker to delete an attribute, but that attribute
   is not present, it's a pass? Doesn't sound right.)
 o 'update' method is needlessly simple - look at the same method
   in BaseTracker, which contains more method calls. Result of the
   simplicity is that in actual role tests, these other methods
   like 'check_update' etc. are called outside the 'update' method,
   when they can just as well be part of the method and save
   repeating same code over and over. This goes for 'retrieve' and
   'find' methods as well.
 * in role tests (ipatests/test_xmlrpc/test_role_plugin.py):
 o in class TestDeleredRole, there's unused method setUpClass

2) other:

 * I strongly suggest to go through all documentation comments, since
   some of them are vague, or even straight misleading (e.g. in
   RoleTracker, method 'find_tracker_roles': comment says, that it
   performs find command, but nothing like that happens!)
 * similarly as in previous note, please go through all parameter
   descriptions in the documentation - e.g. in RoleTracker, method
   'update_tracker': ":param expected_updates: Dictionary of other
   attributes" - such description is quite unsatisfactory)
 * when doing previous two, there are some typos that could be eliminated
 * some test cases in ipatests/test_xmlrpc/test_role_plugin.py seem
   incorrectly classified, e.g. method 'test_create_invalid' verifying
   attempt to create role with invalid name in TestNonexistentRole
   class that performes operations like role-show, role-delete on
   nonexistent entries
 * for future patches, it might be nice to have separate patches for
   each tracker and the tests

Also, since the author Peter Lacko left the company, fixing this patch 
will be a task for Ganna.


Lenka





- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Monday, June 6, 2016 12:29:29 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 02.06.2016 16:16, Peter Lacko wrote:

Rebased with updated tests.

Peter

- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Thursday, June 2, 2016 1:50:06 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

Your patch doesn't apply anymore on master, there were changes in role
test and patch have to be updated and rebased

Please look at this for new changes in test_role_plugin.py
https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1

Martin^2


On 02.06.2016 11:49, Peter Lacko wrote:

Sorry for late response, I wasn't working these days.
Fixed patch is in attachment.

Peter


- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>, freeipa-devel@redhat.com
Sent: Monday, May 9, 2016 1:06:08 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 09.05.2016 13:04, Martin Basti wrote:

On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus
you cannot use it directly, you need something like

if six.PY2:
   str = unicode

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-07-20 Thread Peter Lacko 
Sorry for late reply, I was waiting how the discussion with tracker improvement 
will end, but since there's no progress and I'm leaving soon, I'm attaching new 
patch. I also created mapping between old and new tests [1], to make life of 
reviewer easier. Number in first column denotes line number in original file, 
followed by line number in new tests file and its name. Tests that are not 
mentioned in there extend previous coverage.

Regards,
Peter

[1] http://etherpad.corp.redhat.com/Vk3tRLaPSK


- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Monday, June 6, 2016 12:29:29 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 02.06.2016 16:16, Peter Lacko wrote:
> Rebased with updated tests.
>
> Peter
>
> - Original Message -
> From: "Martin Basti" <mba...@redhat.com>
> To: "Peter Lacko" <pla...@redhat.com>
> Cc: freeipa-devel@redhat.com
> Sent: Thursday, June 2, 2016 1:50:06 PM
> Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests
>
> Your patch doesn't apply anymore on master, there were changes in role
> test and patch have to be updated and rebased
>
> Please look at this for new changes in test_role_plugin.py
> https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1
>
> Martin^2
>
>
> On 02.06.2016 11:49, Peter Lacko wrote:
>> Sorry for late response, I wasn't working these days.
>> Fixed patch is in attachment.
>>
>> Peter
>>
>>
>> - Original Message -
>> From: "Martin Basti" <mba...@redhat.com>
>> To: "Peter Lacko" <pla...@redhat.com>, freeipa-devel@redhat.com
>> Sent: Monday, May 9, 2016 1:06:08 PM
>> Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests
>>
>>
>>
>> On 09.05.2016 13:04, Martin Basti wrote:
>>> On 09.05.2016 12:19, Peter Lacko wrote:
>>>> +# pylint: disable=unicode-builtin
>>> I'm not doing complete review, just the line above hit my eyes.
>>>
>>> unicode() is not in Py3 because all strings there are unicode, thus
>>> you cannot use it directly, you need something like
>>>
>>> if six.PY2:
>>>   str = unicode
>>>
>>> and use str() everywhere and remove that #pylint line
>>>
>>> FYI all enabled pylint checks are there for a good reason, be careful
>>> with disabling it (mainly disabling it for a whole module) rather ask
>>> before if you are not sure.
>>>
>>> Martin^2
>>>
>> Nope, sorry, I temporarily forgot how to python
>>
>> instead of pylint disable use this
>>
>> if six.PY3:
>>unicode =str
>>
>> and keep unicode there. Sorry
>>
>> Martin^2
Hi,

I don't have time to continue with full review, maybe somebody else can 
continue instead of me, but anyway NACK, because using str(unicode()) or 
unicode(str()) is bad in both ways, it should be just unicode() in case 
of strings (with correct mapping unicode=str in Py3). I just  I noticed 
this by reading code, but I didn't do deeper review, so there might be 
more mistakes.

Martin^2
From a5192241e29181e68a480b2b2e802a4267cea4d6 Mon Sep 17 00:00:00 2001
From: Peter Lacko <pla...@redhat.com>
Date: Tue, 31 May 2016 14:34:19 +0200
Subject: [PATCH] New User Role tests.

Commit contains set of User Role tests, implementation of UserTracker
and basic implementation of PrivilegeTracker necessary for Role testing.
Tests replace previous declarative tests for User Role plugin and extends
their coverage.

Commit contains 74 tests split into classes as follows:

TestNonexistentRole
Test that no operation can be performed on non-existing role.
TestDeletedRole
Test that no operation can be performed on deleted role.
TestRoleBasic
Test basic role commands: create, update, delete.
TestRoleUpdate
Test role-mod command.
TestRolePrivileges
Test correct functionality role-privilege-* command.
TestRoleMembers
Test correct functionality of role-member-* command.
TestRoleFind
Test correct functionality of role-find command.

Other than previously mentiond functionality has not beed added, removed or modified.
---
 ipatests/test_xmlrpc/test_role_plugin.py | 1334 +++---
 ipatests/test_xmlrpc/tracker/privilege_plugin.py |   96 ++
 ipatests/test_xmlrpc/tracker/role_plugin.py  |  583 ++
 3 files changed, 1343 insertions(+), 670 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/privilege_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/role_plugin.py

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-06-06 Thread Martin Basti 



On 02.06.2016 16:16, Peter Lacko wrote:

Rebased with updated tests.

Peter

- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Thursday, June 2, 2016 1:50:06 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

Your patch doesn't apply anymore on master, there were changes in role
test and patch have to be updated and rebased

Please look at this for new changes in test_role_plugin.py
https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1

Martin^2


On 02.06.2016 11:49, Peter Lacko wrote:

Sorry for late response, I wasn't working these days.
Fixed patch is in attachment.

Peter


- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>, freeipa-devel@redhat.com
Sent: Monday, May 9, 2016 1:06:08 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 09.05.2016 13:04, Martin Basti wrote:

On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus
you cannot use it directly, you need something like

if six.PY2:
  str = unicode

and use str() everywhere and remove that #pylint line

FYI all enabled pylint checks are there for a good reason, be careful
with disabling it (mainly disabling it for a whole module) rather ask
before if you are not sure.

Martin^2


Nope, sorry, I temporarily forgot how to python

instead of pylint disable use this

if six.PY3:
   unicode =str

and keep unicode there. Sorry

Martin^2

Hi,

I don't have time to continue with full review, maybe somebody else can 
continue instead of me, but anyway NACK, because using str(unicode()) or 
unicode(str()) is bad in both ways, it should be just unicode() in case 
of strings (with correct mapping unicode=str in Py3). I just  I noticed 
this by reading code, but I didn't do deeper review, so there might be 
more mistakes.


Martin^2

--
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

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-06-02 Thread Peter Lacko 
Rebased with updated tests.

Peter

- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Thursday, June 2, 2016 1:50:06 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

Your patch doesn't apply anymore on master, there were changes in role 
test and patch have to be updated and rebased

Please look at this for new changes in test_role_plugin.py 
https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1

Martin^2


On 02.06.2016 11:49, Peter Lacko wrote:
> Sorry for late response, I wasn't working these days.
> Fixed patch is in attachment.
>
> Peter
>
>
> - Original Message -
> From: "Martin Basti" <mba...@redhat.com>
> To: "Peter Lacko" <pla...@redhat.com>, freeipa-devel@redhat.com
> Sent: Monday, May 9, 2016 1:06:08 PM
> Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests
>
>
>
> On 09.05.2016 13:04, Martin Basti wrote:
>>
>> On 09.05.2016 12:19, Peter Lacko wrote:
>>> +# pylint: disable=unicode-builtin
>> I'm not doing complete review, just the line above hit my eyes.
>>
>> unicode() is not in Py3 because all strings there are unicode, thus
>> you cannot use it directly, you need something like
>>
>> if six.PY2:
>>  str = unicode
>>
>> and use str() everywhere and remove that #pylint line
>>
>> FYI all enabled pylint checks are there for a good reason, be careful
>> with disabling it (mainly disabling it for a whole module) rather ask
>> before if you are not sure.
>>
>> Martin^2
>>
> Nope, sorry, I temporarily forgot how to python
>
> instead of pylint disable use this
>
> if six.PY3:
>   unicode =str
>
> and keep unicode there. Sorry
>
> Martin^2

From 63289d67190af1da3fe76c03be9cebd85dc1e0a8 Mon Sep 17 00:00:00 2001
From: Peter Lacko <pla...@redhat.com>
Date: Tue, 31 May 2016 14:34:19 +0200
Subject: [PATCH] New User Role tests.

Commit contains set of User Role tests, implementation of UserTracker
and basic implementation of PrivilegeTracker necessary for Role testing.
Tests replace previous declarative tests for User Role plugin and extends
their coverage.

Commit contains 65 tests split into classes as follows:

TestNonexistentRole
Test that no operation can be performed on non-existing role.
TestDeletedRole
Test that no operation can be performed on deleted role.
TestRoleBasic
Test basic role commands: create, update, delete.
TestRoleUpdate
Test role-mod command.
TestRolePrivileges
Test correct functionality role-privilege-* command.
TestRoleMembers
Test correct functionality of role-member-* command.
TestRoleFind
Test correct functionality of role-find command.

Other than previously mentiond functionality has not beed added, removed or modified.
---
 ipatests/test_xmlrpc/test_role_plugin.py | 1264 ++
 ipatests/test_xmlrpc/tracker/privilege_plugin.py |   96 ++
 ipatests/test_xmlrpc/tracker/role_plugin.py  |  583 ++
 3 files changed, 1268 insertions(+), 675 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/privilege_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/role_plugin.py

diff --git a/ipatests/test_xmlrpc/test_role_plugin.py b/ipatests/test_xmlrpc/test_role_plugin.py
index 2c368b3518cda3d11fadce69f8bdfc02edfcf719..00dcdf0ab3dfff10eca34f4b26f3bc2a19840456 100644
--- a/ipatests/test_xmlrpc/test_role_plugin.py
+++ b/ipatests/test_xmlrpc/test_role_plugin.py
@@ -2,8 +2,9 @@
 #   Rob Crittenden <rcrit...@redhat.com>
 #   Pavel Zuna <pz...@redhat.com>
 #   John Dennis <jden...@redhat.com>
+#   Peter Lacko <pla...@redhat.com>
 #
-# Copyright (C) 2009  Red Hat
+# Copyright (C) 2009, 2016  Red Hat
 # see file 'COPYING' for use and warranty information
 #
 # This program is free software; you can redistribute it and/or modify
@@ -22,686 +23,599 @@
 Test the `ipalib/plugins/role.py` module.
 """
 
-from ipalib import api, errors
-from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid
-from ipapython.dn import DN
+
 import pytest
+import six
+from uuid import uuid4
 
-search = u'test-role'
+from ipalib import errors
+from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+from ipatests.test_xmlrpc.tracker.hostgroup_plugin import HostGroupTracker
+from ipatests.test_xmlrpc.tracker.privilege_plugin import PrivilegeTracker
+from ipatests.test_xmlrpc.tracker.role_plugin import RoleTracker
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
+fr

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-06-02 Thread Martin Basti 
Your patch doesn't apply anymore on master, there were changes in role 
test and patch have to be updated and rebased


Please look at this for new changes in test_role_plugin.py 
https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1


Martin^2


On 02.06.2016 11:49, Peter Lacko wrote:

Sorry for late response, I wasn't working these days.
Fixed patch is in attachment.

Peter


- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>, freeipa-devel@redhat.com
Sent: Monday, May 9, 2016 1:06:08 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 09.05.2016 13:04, Martin Basti wrote:


On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus
you cannot use it directly, you need something like

if six.PY2:
 str = unicode

and use str() everywhere and remove that #pylint line

FYI all enabled pylint checks are there for a good reason, be careful
with disabling it (mainly disabling it for a whole module) rather ask
before if you are not sure.

Martin^2


Nope, sorry, I temporarily forgot how to python

instead of pylint disable use this

if six.PY3:
  unicode =str

and keep unicode there. Sorry

Martin^2


--
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

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-06-02 Thread Peter Lacko 
Sorry for late response, I wasn't working these days.
Fixed patch is in attachment.

Peter


- Original Message -
From: "Martin Basti" <mba...@redhat.com>
To: "Peter Lacko" <pla...@redhat.com>, freeipa-devel@redhat.com
Sent: Monday, May 9, 2016 1:06:08 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 09.05.2016 13:04, Martin Basti wrote:
>
>
> On 09.05.2016 12:19, Peter Lacko wrote:
>> +# pylint: disable=unicode-builtin
> I'm not doing complete review, just the line above hit my eyes.
>
> unicode() is not in Py3 because all strings there are unicode, thus 
> you cannot use it directly, you need something like
>
> if six.PY2:
> str = unicode
>
> and use str() everywhere and remove that #pylint line
>
> FYI all enabled pylint checks are there for a good reason, be careful 
> with disabling it (mainly disabling it for a whole module) rather ask 
> before if you are not sure.
>
> Martin^2
>

Nope, sorry, I temporarily forgot how to python

instead of pylint disable use this

if six.PY3:
 unicode =str

and keep unicode there. Sorry

Martin^2
From 14ce232be99d5a5f1dd5257340d932c86c222ac3 Mon Sep 17 00:00:00 2001
From: Peter Lacko <pla...@redhat.com>
Date: Tue, 31 May 2016 14:34:19 +0200
Subject: [PATCH] New User Role tests.

Commit contains set of User Role tests, implementation of UserTracker
and basic implementation of PrivilegeTracker necessary for Role testing.
Tests replace previous declarative tests for User Role plugin and extends
their coverage.

Commit contains 62 tests split into classes as follows:

TestNonexistentRole
Test that no operation can be performed on non-existing role.
TestDeletedRole
Test that no operation can be performed on deleted role.
TestRoleBasic
Test basic role commands: create, update, delete.
TestRoleUpdate
Test role-mod command.
TestRolePrivileges
Test correct functionality role-privilege-* command.
TestRoleMembers
Test correct functionality of role-member-* command.
TestRoleFind
Test correct functionality of role-find command.

Other than previously mentiond functionality has not beed added, removed or modified.
---
 ipatests/test_xmlrpc/test_role_plugin.py | 1164 +++---
 ipatests/test_xmlrpc/tracker/privilege_plugin.py |   96 ++
 ipatests/test_xmlrpc/tracker/role_plugin.py  |  566 +++
 3 files changed, 1228 insertions(+), 598 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/privilege_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/role_plugin.py

diff --git a/ipatests/test_xmlrpc/test_role_plugin.py b/ipatests/test_xmlrpc/test_role_plugin.py
index d06daac6902cdf569920f908d99e5ee5c0cdfa43..e38397303da85db15baa275e8907514b1b127300 100644
--- a/ipatests/test_xmlrpc/test_role_plugin.py
+++ b/ipatests/test_xmlrpc/test_role_plugin.py
@@ -2,8 +2,9 @@
 #   Rob Crittenden <rcrit...@redhat.com>
 #   Pavel Zuna <pz...@redhat.com>
 #   John Dennis <jden...@redhat.com>
+#   Peter Lacko <pla...@redhat.com>
 #
-# Copyright (C) 2009  Red Hat
+# Copyright (C) 2009, 2016  Red Hat
 # see file 'COPYING' for use and warranty information
 #
 # This program is free software; you can redistribute it and/or modify
@@ -22,609 +23,576 @@
 Test the `ipalib/plugins/role.py` module.
 """
 
-from ipalib import api, errors
-from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid
-from ipapython.dn import DN
+
 import pytest
+import six
+from uuid import uuid4
 
-search = u'test-role'
+from ipalib import errors
+from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+from ipatests.test_xmlrpc.tracker.hostgroup_plugin import HostGroupTracker
+from ipatests.test_xmlrpc.tracker.privilege_plugin import PrivilegeTracker
+from ipatests.test_xmlrpc.tracker.role_plugin import RoleTracker
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
+from ipatests.test_xmlrpc.xmlrpc_test import (
+raises_exact,
+XMLRPC_test,
+)
+from ipatests.util import assert_deepequal
 
-role1 = u'test-role-1'
-role1_dn = DN(('cn',role1),api.env.container_rolegroup,
-  api.env.basedn)
-renamedrole1 = u'test-role'
-invalidrole1 = u' whitespace '
+_DESCRIPTION = u'Description string'
+_NEW_DESC = u'New description'
+_INVALID_NAME = u'  Invalid Name  '
 
-role2 = u'test-role-2'
-role2_dn = DN(('cn',role2),api.env.container_rolegroup,
-  api.env.basedn)
+if six.PY3:
+unicode = str
 
-group1 = u'testgroup1'
-group1_dn = DN(('cn',group1),api.env.container_group,
-   api.env.basedn)
 
-privilege1 = u'r,w privilege 1'
-privilege1_dn = DN(('cn', privilege1), DN(api.env.container_privilege),
-   api.env.basedn)
+@p

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-06-01 Thread Martin Basti 



On 09.05.2016 13:06, Martin Basti wrote:




On 09.05.2016 13:04, Martin Basti wrote:



On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus 
you cannot use it directly, you need something like


if six.PY2:
str = unicode

and use str() everywhere and remove that #pylint line

FYI all enabled pylint checks are there for a good reason, be careful 
with disabling it (mainly disabling it for a whole module) rather ask 
before if you are not sure.


Martin^2



Nope, sorry, I temporarily forgot how to python

instead of pylint disable use this
if six.PY3:
 unicode =str
and keep unicode there. Sorry

Martin^2



Hello,
it is 3 weeks since last message, any updates?

Martin^2
-- 
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

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-05-09 Thread Martin Basti 



On 09.05.2016 13:04, Martin Basti wrote:



On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus 
you cannot use it directly, you need something like


if six.PY2:
str = unicode

and use str() everywhere and remove that #pylint line

FYI all enabled pylint checks are there for a good reason, be careful 
with disabling it (mainly disabling it for a whole module) rather ask 
before if you are not sure.


Martin^2



Nope, sorry, I temporarily forgot how to python

instead of pylint disable use this

if six.PY3:
unicode =str

and keep unicode there. Sorry

Martin^2
-- 
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

Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-05-09 Thread Martin Basti 



On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus you 
cannot use it directly, you need something like


if six.PY2:
str = unicode

and use str() everywhere and remove that #pylint line

FYI all enabled pylint checks are there for a good reason, be careful 
with disabling it (mainly disabling it for a whole module) rather ask 
before if you are not sure.


Martin^2

--
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

[Freeipa-devel] [PATCH] 0002 New User Role Tests

2016-05-09 Thread Peter Lacko 
Hi!

New User Role Tests, extending coverage of previous declarative tests are ready 
for review.

Peter Lacko
pla...@redhat.com

From fcbf0b389041e7051c70b15d6799a446c1f42456 Mon Sep 17 00:00:00 2001
From: placko 
Date: Fri, 8 Apr 2016 17:01:15 +0200
Subject: [PATCH] New User Role tests.

Commit contains set of User Role tests, implementeation of UserTracker
and basic implementation of PrivilegeTracker for needs of Role testing.
Tests replace previous declarative tests for User Role plugin and extends
their coverage.

Commit contains 62 tests split into classes as follows:

TestNonexistentRole
Test that no operation can be performed on non-existing role.
TestDeletedRole
Test that no operation can be performed on deleted role.
TestRoleBasic
Test basic role commands: create, update, delete.
TestRoleUpdate
Test for role-mod command.
TestRolePrivileges
Test correct functionality role-privilege-* command.
TestRoleMembers
Test correct functionality of role-member-* command.
TestRoleFind
Test correct functionality of role-find command.

Other than previously mentiond functionality has not beed added, removed or modified.
---
 ipatests/test_xmlrpc/test_role_plugin.py | 1180 +++---
 ipatests/test_xmlrpc/tracker/privilege_plugin.py |   96 ++
 ipatests/test_xmlrpc/tracker/role_plugin.py  |  565 +++
 3 files changed, 1223 insertions(+), 618 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/privilege_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/role_plugin.py

diff --git a/ipatests/test_xmlrpc/test_role_plugin.py b/ipatests/test_xmlrpc/test_role_plugin.py
index d06daac6902cdf569920f908d99e5ee5c0cdfa43..5117a2ec3f00c61b8ecdc591f2dc0a396c0cac98 100644
--- a/ipatests/test_xmlrpc/test_role_plugin.py
+++ b/ipatests/test_xmlrpc/test_role_plugin.py
@@ -1,630 +1,574 @@
-# Authors:
-#   Rob Crittenden 
-#   Pavel Zuna 
-#   John Dennis 
 #
-# Copyright (C) 2009  Red Hat
-# see file 'COPYING' for use and warranty information
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
 #
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see .
-"""
-Test the `ipalib/plugins/role.py` module.
-"""
 
-from ipalib import api, errors
-from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid
-from ipapython.dn import DN
+# pylint: disable=unicode-builtin
+
 import pytest
+from uuid import uuid4
 
-search = u'test-role'
+from ipalib import errors
+from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+from ipatests.test_xmlrpc.tracker.hostgroup_plugin import HostGroupTracker
+from ipatests.test_xmlrpc.tracker.privilege_plugin import PrivilegeTracker
+from ipatests.test_xmlrpc.tracker.role_plugin import RoleTracker
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
+from ipatests.util import assert_deepequal
+from xmlrpc_test import (
+raises_exact,
+XMLRPC_test,
+)
 
-role1 = u'test-role-1'
-role1_dn = DN(('cn',role1),api.env.container_rolegroup,
-  api.env.basedn)
-renamedrole1 = u'test-role'
-invalidrole1 = u' whitespace '
+_DESCRIPTION = u'Description string'
+_NEW_DESC = u'New description'
+_INVALID_NAME = u'  Invalid Name  '
 
-role2 = u'test-role-2'
-role2_dn = DN(('cn',role2),api.env.container_rolegroup,
-  api.env.basedn)
 
-group1 = u'testgroup1'
-group1_dn = DN(('cn',group1),api.env.container_group,
-   api.env.basedn)
+@pytest.fixture(scope='class')
+def role(request):
+"""Default testing role."""
+tracker = RoleTracker(name=u'test-role1', description=_DESCRIPTION)
+return tracker.make_fixture(request)
 
-privilege1 = u'r,w privilege 1'
-privilege1_dn = DN(('cn', privilege1), DN(api.env.container_privilege),
-   api.env.basedn)
+
+@pytest.fixture(scope='class')
+def role2(request):
+"""Additional testing role."""
+tracker = RoleTracker(name=u'test-role2', description=_DESCRIPTION)
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def random_role(request):
+"""Testing role with random name."""
+tracker = RoleTracker(
+name=unicode(uuid4()),