[Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

[Filip Skola]

----- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-fskola-0002-7-Refactor-test_group_plugin.patch
Type: text/x-patch
Size: 74872 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160128/121b19bf/attachment.bin>