On 01/15/2016 03:41 PM, Filip Skola wrote:
Hi,
sending rebased patch on top of 58c42ddac0964a8cce7c1e1faa7516da53f028ad.
Includes a "fix" for the rename-to-invalid-username issue for the new version.
F.
----- Original Message -----
Hi,
I don't know what is causing the \r\n issue. I use vim and than send each
email with claws-mail. Didn't spot this issue when trying emailing the patch
to my other address. I'm trying to send it from zimbra now, let me know if
that helped pls.
Fix for the stageuser plugin issues caused by this patch should have been
included in the last update; I think the remaining issue is not caused by
UserTracker changes. Please correct me, if I'm wrong.
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 -
This has been changed. This test still fails, though.
Filip
----- Original Message -----
From: "Aleš Mareček" <amare...@redhat.com>
To: "Filip Škola" <fsk...@redhat.com>
Cc: freeipa-devel@redhat.com, "Milan Kubík" <mku...@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" <fsk...@redhat.com>
To: "Aleš Mareček" <amare...@redhat.com>
Cc: freeipa-devel@redhat.com, "Milan Kubík" <mku...@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 <amare...@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" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@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 <mku...@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 <mku...@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 <mku...@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
--
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
NACK, there are errors occuring that do not appear in the respective
test cases in the declarative test.
In the original module the ` Test a login name that is too long` and
`Try to rename to a username that is too long` do not use {add,set}attr.
Why do you use them?
I'm also postponing the review of your other patches as they depend on
changes in this one.
--
Milan Kubik
--
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
