[Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

[Milan Kubík]

On 01/15/2016 03:38 PM, Filip Skola wrote:
> Hi,
>
> sending rebased patch.
>
> F.
>
> ----- Original Message -----
>> Hello,
>>
>> sorry for delays. The patch no longer applies to master. Rebase it, please.
>>
>> Milan
>>
>> ----- Original Message -----
>> From: "Filip Škola" <fskola at redhat.com>
>> To: "Milan Kubík" <mkubik at redhat.com>
>> Cc: freeipa-devel at redhat.com
>> Sent: Wednesday, 9 December, 2015 7:01:02 PM
>> Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
>>
>> On Mon, 7 Dec 2015 17:49:18 +0100
>> Milan Kubík <mkubik at redhat.com> wrote:
>>
>>> On 12/03/2015 08:15 PM, Filip Škola wrote:
>>>> On Mon, 30 Nov 2015 17:18:30 +0100
>>>> Milan Kubík <mkubik at redhat.com> wrote:
>>>>
>>>>> On 11/23/2015 04:42 PM, Filip Škola wrote:
>>>>>> Sending updated patch.
>>>>>>
>>>>>> F.
>>>>>>
>>>>>> On Mon, 23 Nov 2015 14:59:34 +0100
>>>>>> Filip Škola <fskola at redhat.com> wrote:
>>>>>>
>>>>>>> Found couple of issues (broke some dependencies).
>>>>>>>
>>>>>>> NACK
>>>>>>>
>>>>>>> F.
>>>>>>>
>>>>>>> On Fri, 20 Nov 2015 13:56:36 +0100
>>>>>>> Filip Škola <fskola at redhat.com> wrote:
>>>>>>>
>>>>>>>> Another one.
>>>>>>>>
>>>>>>>> F.
>>>>> Hi, the tests look good. Few remarks, though.
>>>>>
>>>>> 1. Please, use the shortes copyright notice in new modules.
>>>>>
>>>>>        #
>>>>>        # Copyright (C) 2015  FreeIPA Contributors see COPYING for
>>>>> license #
>>>>>
>>>>> 2. The tests `test_group_remove_group_from_protected_group` and
>>>>> `test_group_full_set_of_objectclass_not_available_post_detach`
>>>>> were not ported. Please, include them in the patch.
>>>>>
>>>>> Also, for less hassle, please rebase your patches on top of
>>>>> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
>>>>> Which changes the location of tracker implementations and prevents
>>>>> circular imports.
>>>>>
>>>>> Thanks.
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> these cases are there, in corresponding classes. They are marked
>>>> with the original comments. (However I can move them to separate
>>>> class if desirable.)
>>>>
>>>> The copyright notice is changed. Also included a few changes in the
>>>> test with user without private group.
>>>>
>>>> Filip
>>> NACK
>>>
>>> linter:
>>> ************* Module tracker.group_plugin
>>> ipatests/test_xmlrpc/tracker/group_plugin.py:257:
>>> [E0102(function-redefined), GroupTracker.check_remove_member] method
>>> already defined line 253)
>>>
>>> Probably a leftover after the rebase made on top of my patch. Please
>>> fix it. You can check youch changes by make-lint script before
>>> sending them.
>>>
>>> Thanks
>>>
>>
>> Hi,
>>
>> I learned to use make-lint!
>>
>> Thanks,
>> F.
>>
Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported 
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this would be 
a good idea in our environment. I suggest to create the fixtures on per 
module basis.


[1]: http://fpaste.org/311949/53118942/
[2]: 
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects

-- 
Milan Kubik