[Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin

[Aleš Mareček]

There is some issue with "test_rename_to_too_long_login" test. It fails but actually this is false positive because it is possible to create login upto 255 characters. I don't know why test mentions 32 characters without any other modified setup.
NACK for now.
 - alich -


----- Original Message -----
> From: "Aleš Mareček" <amarecek at redhat.com>
> To: "Filip Škola" <fskola at redhat.com>
> Cc: freeipa-devel at redhat.com, "Milan Kubík" <mkubik at redhat.com>
> Sent: Thursday, December 10, 2015 4:11:47 PM
> Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin
> 
> Ah, sorry, haven't realized there had been devel list attached.
> Ok, there is some problem with \r\n in the patch.
> Filip, please take a look at it...
> Thanks...
>  - alich -
> 
> ----- Original Message -----
> > From: "Filip Škola" <fskola at redhat.com>
> > To: "Aleš Mareček" <amarecek at redhat.com>
> > Cc: freeipa-devel at redhat.com, "Milan Kubík" <mkubik at redhat.com>
> > Sent: Thursday, December 10, 2015 11:29:52 AM
> > Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin
> > 
> > Hi,
> > 
> > this if fixed. Also issues with test_stageuser_plugin caused by
> > UserTracker changes should be fixed here.
> > 
> > Filip
> > 
> > 
> > On Mon, 7 Dec 2015 09:29:31 -0500 (EST)
> > Aleš Mareček <amarecek at redhat.com> wrote:
> > 
> > > NACK.
> > > 
> > > $ ./make-lint
> > > ************* Module ipatests.test_xmlrpc.test_user_plugin
> > > ipatests/test_xmlrpc/test_user_plugin.py:42:
> > > [E0611(no-name-in-module), ] No name 'ldaptracker' in module
> > > 'ipatests.test_xmlrpc')
> > > 
> > > $ grep ldaptracker ipatests/test_xmlrpc/test_user_plugin.py
> > > from ipatests.test_xmlrpc.ldaptracker import Tracker
> > > $ ls ipatests/test_xmlrpc/ldaptracker*
> > > ls: cannot access ipatests/test_xmlrpc/ldaptracker*: No such file or
> > > directory
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Filip Škola" <fskola at redhat.com>
> > > > To: "Milan Kubík" <mkubik at redhat.com>
> > > > Cc: freeipa-devel at redhat.com
> > > > Sent: Thursday, December 3, 2015 5:38:43 PM
> > > > Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin
> > > > 
> > > > Hi,
> > > > 
> > > > sending corrected version.
> > > > 
> > > > F.
> > > > 
> > > > On Thu, 12 Nov 2015 14:03:19 +0100
> > > > Milan Kubík <mkubik at redhat.com> wrote:
> > > > 
> > > > > On 11/10/2015 12:13 PM, Filip Škola wrote:
> > > > > > Hi,
> > > > > >
> > > > > > fixed.
> > > > > >
> > > > > > F.
> > > > > >
> > > > > > On Tue, 10 Nov 2015 10:52:45 +0100
> > > > > > Milan Kubík <mkubik at redhat.com> wrote:
> > > > > >
> > > > > >> On 11/09/2015 04:35 PM, Filip Škola wrote:
> > > > > >>> Another patch was applied in the meantime.
> > > > > >>>
> > > > > >>> Attaching an updated version.
> > > > > >>>
> > > > > >>> F.
> > > > > >>>
> > > > > >>> On Mon, 9 Nov 2015 13:35:02 +0100
> > > > > >>> Milan Kubík <mkubik at redhat.com> wrote:
> > > > > >>>
> > > > > >>>> On 11/06/2015 11:32 AM, Filip Škola wrote:
> > > > > >>>> Hi,
> > > > > >>>> the patch doesn't apply.
> > > > > >>>>
> > > > > >> Please fix this.
> > > > > >>
> > > > > >>       ipatests/test_xmlrpc/test_user_plugin.py:1419:
> > > > > >> [E0602(undefined-variable),
> > > > > >> TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined
> > > > > >> variable 'user1')
> > > > > >>
> > > > > >> Also, use the version numbers for your changed patches.
> > > > > >>
> > > > > >
> > > > > >
> > > > > Thanks for the patch. Several issues:
> > > > > 
> > > > > 1. Use dict.items instead of dict.iteritems, for python3
> > > > > compatibility
> > > > > 
> > > > > 2. What is the purpose of TestPrepare class? The 'purge' methods
> > > > > do not call any ipa commands.
> > > > > Tracker.make_fixture should be used to make the Tracked resources
> > > > > clean themselves up when they're out of scope.
> > > > > 
> > > > > 3. Why reference the resources by hardcoded name if they have a
> > > > > fixture representation?
> > > > > 
> > > > > 4. Rewrite {create,delete}_test_group to a fixture. You may want
> > > > > to use different scope (or not).
> > > > > 
> > > > > 5. In `def atest_rename_to_invalid_login(self, user):` - use
> > > > > pytest.skipif decorator and provide a reason if you must,
> > > > > do not obfuscate method name in order not to run it.
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > --
> > > > 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
> > 
> > 
>