On 05/30/2013 04:35 PM, Tomas Babej wrote: > On 05/29/2013 12:25 PM, Martin Kosek wrote: >> On 05/28/2013 03:48 PM, Alexander Bokovoy wrote: >>> On Tue, 28 May 2013, Tomas Babej wrote: >>>> On 05/28/2013 02:35 PM, Alexander Bokovoy wrote: >>>>> On Mon, 27 May 2013, Tomas Babej wrote: >>>>>>>>> We got rid of openldap utilities now. While using python.ldap module, >>>>>>>>> I >>>>>>>>> also made the tests much more robust and added a new test case. >>>>>>>> In general patches look fine, there is one small nitpick. >>>>>>>> I'll run tests on Monday and then will provide final ACK. >>>>>>>> >>>>>>>>> --- a/tests/test_xmlrpc/test_range_plugin.py >>>>>>>>> +++ b/tests/test_xmlrpc/test_range_plugin.py >>>>>>>>> @@ -22,66 +22,171 @@ Test the `ipalib/plugins/idrange.py` module, and >>>>>>>>> XML-RPC in general. >>>>>>>>> """ >>>>>>>>> >>>>>>>>> from ipalib import api, errors, _ >>>>>>>>> +from ipapython.ipautil import run >>>>>>>> This import is unused, can be removed. >>>>>>>> >>>>>>> Fixed, thanks for catching that. >>>>>>> >>>>>>> Updated patch attached. >>>>> So I tried to run this test on a machine where there is already trust >>>>> established and I think there should be done some changes. >>>> I perused the log. Seems that the failures you're experiencing are not >>>> relevant to the patch itself, >>>> since the newly added tests passed. >>>> >>>> This is problem with test_range_plugin.py tests that has been there for >>>> quite >>>> a while, the parameters >>>> of the ranges such as size, and base ID/RID/secondary RID are hardcoded in >>>> the test case. >>> Yep. >>> >>> >>>>> Probably it would be wise to add pre-start procedure to pull existing >>>>> ranges and define constants for the ranges so that they don't overlap >>>>> with existing ones. Perhaps selecting something from a top of the range >>>>> space... >>>>> >>>>> Attached is the log >>>> I agree. This has not been relevant until now, since we did not do much >>>> testing on IPA instances >>>> with trusts set up, and even then there's random factor in having the >>>> overlap >>>> with the already created >>>> trust range. >>>> >>>> I'd propose fixing this in a separate effort as a part of continouous >>>> integration improvements. I see it >>>> as a separate issue of its own. >>>> >>>> What do you think? >>> Please file a separate ticket then. >>> >>> ACK for this one. >>> >> May-be-NACK. >> >> Would it make sense to replace the error with DependentEntry error? We use in >> cases like this elsewhere and I think it makes more sense in this case too. >> >> Martin > > Sure, I changed the error class in idrange.py and tests accordingly. > > I ran the unit tests again to verify the changes. > > Here is the updated patch. > > Tomas
ACK. Pushed to master, ipa-3-2. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
