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 _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel