[Freeipa-devel] [PATCH 0002] TEST: Stageuser plugin
Martin Basti
mbasti at redhat.com
Thu Aug 13 12:09:23 UTC 2015
On 08/11/2015 10:57 AM, Lenka Doudova wrote:
>
>
> On 08/11/2015 10:06 AM, thierry bordaz wrote:
>> On 08/04/2015 01:37 PM, Lenka Doudova wrote:
>>>
>>>
>>> Dne 30.7.2015 v 16:10 Martin Basti napsal(a):
>>>> On 30/07/15 16:09, Martin Basti wrote:
>>>>> On 29/07/15 16:10, Martin Basti wrote:
>>>>>> On 29/07/15 15:29, Lenka Doudova wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> thanks a lot for the comments, will work on it tomorrow.
>>>>>>>
>>>>>>> Lenka
>>>>>>>
>>>>>>> Dne 29.7.2015 v 15:27 Martin Basti napsal(a):
>>>>>>>> On 27/07/15 16:47, Lenka Doudova wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I'm attaching a patch with automated tests for stageuser
>>>>>>>>> plugin (https://fedorahosted.org/freeipa/ticket/3813). The
>>>>>>>>> user plugin test is affected as well (one class was added).
>>>>>>>>> The tests seem a bit of a mess even to myself, but what with
>>>>>>>>> the way freeipa behaves I didn't know how else to implement
>>>>>>>>> them, but I'm eager to learn how to do it in a nicer way, if
>>>>>>>>> someone has a better idea.
>>>>>>>>>
>>>>>>>>> Lenka
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> I just applied patches:
>>>>>>>>
>>>>>>>> 1) Please remove whitespace errors
>>>>>>>> $ git am
>>>>>>>> freeipa-lryznaro-0002-Automated-test-for-stageuser-plugin.patch
>>>>>>>> Applying: Automated test for stageuser plugin
>>>>>>>> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:110:
>>>>>>>> trailing whitespace.
>>>>>>>> """ Tracker class for staged user LDAP object
>>>>>>>> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:113:
>>>>>>>> trailing whitespace.
>>>>>>>> StageUserTracker object stores information about the user.
>>>>>>>> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:121:
>>>>>>>> trailing whitespace.
>>>>>>>> u'krbprincipalexpiration', u'usercertificate', u'dn',
>>>>>>>> u'has_keytab', u'has_password',
>>>>>>>> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:122:
>>>>>>>> trailing whitespace.
>>>>>>>> u'street', u'postalcode', u'facsimiletelephonenumber',
>>>>>>>> u'carlicense',
>>>>>>>> /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:125:
>>>>>>>> trailing whitespace.
>>>>>>>> u'cn', u'ipauniqueid', u'objectclass', u'description',
>>>>>>>> warning: squelched 50 whitespace errors
>>>>>>>> warning: 55 lines add whitespace errors.
>>>>>>>>
>>>>>>>> 2)
>>>>>>>> Please use new shorter format of license header
>>>>>>>>
>>>>>>>> 3) can you fix some of the most serious PEP8 errors
>>>>>>>> $ git show -U0 | pep8 --diff | wc -l
>>>>>>>> 198
>>>>>>>>
>>>>>>>> 4)
>>>>>>>> if options != None:
>>>>>>>>
>>>>>>>> Please use "options *is not* None"
>>>>>>>>
>>>>>>>> 5)
>>>>>>>> For consistency it should be u'random'
>>>>>>>> if key == 'random':
>>>>>>>> self.attrs[u'randompassword'] = fuzzy_string
>>>>>>>>
>>>>>>>> Otherwise it looks good
>>>>>>>> Martin^2
>>>>>>>> --
>>>>>>>> Martin Basti
>>>>>>>
>>>>>> And also fix this please
>>>>>>
>>>>>> ./make-lint
>>>>>> ************* Module ipatests.test_xmlrpc.test_stageuser_plugin
>>>>>> ipatests/test_xmlrpc/test_stageuser_plugin.py:337:
>>>>>> [E0102(function-redefined), user2] function already defined line 44)
>>>>>>
>>>>>> --
>>>>>> Martin Basti
>>>>>>
>>>>>>
>>>>> Ahoj, v patchi mas este uvedene svoje stare meno, mala by si v
>>>>> gite nastavit redhat email
>>>>>
>>>>> --
>>>>> Martin Basti
>>>>>
>>>>>
>>>> Sorry for spam, you can safely ignore this. :)
>>>>
>>>> --
>>>> Martin Basti
>>>>
>>>>
>>> Attaching new patch - (hopefully) fixed the errors from the old one
>>> + few test cases were added.
>>>
>>> Lenka
>>>
>>>
>>
>>
>> Hello Lenka,
>>
>> This is a very impressive work and test framework. I have not
>> understood all the details of the implementation so I just focus on
>> the reading the tests body.
>> The patch is looking great to me and I have really few minors comments.
>>
>> * About non existing stage user, you may try to activate a non
>> existing one. Is it what TestStagedUser.test_activate does ?
>>
> No, it didn't occur to me to test activation of nonexistent user, I
> can add it.
>>
>> * In test_create_attr, I can see that user6 is activated. How is
>> checked that the specified values are preserved ? (sorry my
>> python skill is still very low)
>>
> The values are checked - in the test itself, using
> 'create_from_staged' I first copy attributes of stage user object to a
> new active user object (just object, not the ipa user itself) and in
> 'check_activate' I verify that values of ipa user correspond with
> those taken from staged user
>>
>> * Many testcases
>> (http://www.freeipa.org/page/V4/User_Life-Cycle_Management/Test_Plan#Test_case:_Try_to_search_for_a_nonexistent_user)
>> are about creating a stage user with various attributes
>> (initial,shell, homedir...). I found uid/gid, are the others
>> implemented ?
>>
> Yes, all the options are implemented using parametrized fixture
> 'stageduser2' + 'options_ok' list, tests are performed by
> 'test_create_attr' method. These tests also contain activation check
> (users can be activated and values are preserved).
>>
>> * In TestActive.test_delete_preserve, does check_delete check the
>> active container or the stageuser container ?
>>
> check_delete just checks the result of 'user-del' command (it just
> says that a certain user has been deleted). But after that, there is a
> verification that the user is no longer in _active_ container
> performed by running 'user-show' command with 'not found' as expected
> result.
>>
>> * Does test_delete_preserved checks that the deleted entry has been
>> permanently removed ?
>>
> If you mean whether there is a check like running 'user-show' command
> and expecting 'not found', no, there is not. I haven't thought about
> it. Can add.
>
>> * Is test_preserved_membership the test case for
>> https://fedorahosted.org/freeipa/ticket/5170 ?
>>
> I'd say that partly - it just verifies if membership can be added to a
> preserved user (expects failure of such attempt).
> It does not check whether a user with thus successfully assigned
> membership preserves it after 'user-undel' (but that is part of
> TestPreserved.test_reactivate_preserved - the
> 'check_attr_preservation' method verifies that reactivated user is
> member of 'ipausers' group only).
>
> Lenka
>>
>> Thanks
>> thierry
>>
>
>
>
NACK
syntax error, missing ')'
-from ipatests.util import assert_equal, assert_not_equal, raises
+from ipatests.util import (
+ assert_equal, assert_not_equal, raises, assert_deepequal
I cannot apply this patch, please check it
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150813/937da5c5/attachment.htm>
More information about the Freeipa-devel
mailing list