[Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

[Filip Skola]

----- Original Message -----
> On 02/10/2016 09:17 AM, Milan Kubík wrote:
> > On 02/09/2016 04:19 PM, Milan Kubík wrote:
> >> On 01/28/2016 10:42 AM, Filip Skola wrote:
> >>>
> >>> ----- Original Message -----
> >>>> On 01/25/2016 11:11 AM, Filip Skola wrote:
> >>>>> ----- Original Message -----
> >>>>>> 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
> >>>>>>
> >>>>>>
> >>>>> Hi,
> >>>>>
> >>>>> the fixtures were copied into corresponding module. Please note
> >>>>> that this
> >>>>> patch has a dependence on my patch 0001 (user plugin).
> >>>>>
> >>>>> Filip
> >>>> Linter:
> >>>> ************* Module ipatests.test_xmlrpc.tracker.group_plugin
> >>>> W:100,26: Calling a dict.iter*() method (dict-iter-method)
> >>>>
> >>>> please use dict.items
> >>>>
> >>>> --
> >>>> Milan Kubik
> >>>>
> >>>>
> >>> Hi, sorry. This has been fixed in this patch.
> >>>
> >>> Filip
> >> ACK, thanks for the patience. :)
> >>
> > Sorry, there are some other things I need clarified. NACK.
> > Mail will follow later.
> >
> What is the purpose of `make_fixture_detach` in your patches? They are
> not used anywhere and the finalizer does nothing.
> 
> --
> Milan Kubik
> 
> 

Hi,

none, I guess, probably a leftover copied from the tracker in the early days. Deleting the function.

Filip
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-fskola-0002-8-Refactor-test_group_plugin.patch
Type: text/x-patch
Size: 75337 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160222/3486762b/attachment.bin>