Re: [Freeipa-devel] [PATCH] 0108 cert-request: raise error when request fails
On 09/09/2016 01:53 AM, Fraser Tweedale wrote: On Thu, Sep 08, 2016 at 01:15:03PM +0200, Martin Babinsky wrote: On 09/08/2016 04:00 AM, Fraser Tweedale wrote: The attached patch fixes regression in cert-request: https://fedorahosted.org/freeipa/ticket/6309 Thanks, Fraser ACK. Does this patch also fix the (reopened) https://fedorahosted.org/freeipa/ticket/3473 ? It does not. There's much more work to do on #3473. It has only been a little bit done because I needed to switch ra.request_certificate to REST API so we can properly detect failure due to CA-disabled condition. Thanks, Fraser Hi, just a note - this needs to be pushed to both master and ipa-4-4 branches. Thanks, Lenka -- 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
[Freeipa-devel] [PATCH 0039][Tests] ID views tests do not recognize 'krbcanonicalname' attribute
Hi, ID views tests still do not recognize 'krbcanonicalname' attribute - fix attached. Lenka From 6610280a0bae7116c0d790ffa75b6f1a2208365a Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 22 Aug 2016 15:41:01 +0200 Subject: [PATCH] Tests: ID views tests do not recognize krbcanonicalname attribute https://fedorahosted.org/freeipa/ticket/6242 --- ipatests/test_xmlrpc/test_idviews_plugin.py | 8 1 file changed, 8 insertions(+) diff --git a/ipatests/test_xmlrpc/test_idviews_plugin.py b/ipatests/test_xmlrpc/test_idviews_plugin.py index eeadc7ceca6c81afe5004898f67e0727f480efe3..591d002882dee75568baa6b39eeec476961a91e8 100644 --- a/ipatests/test_xmlrpc/test_idviews_plugin.py +++ b/ipatests/test_xmlrpc/test_idviews_plugin.py @@ -804,6 +804,8 @@ class test_idviews(Declarative): l=[u'Undisclosed location 1'], krbprincipalname=[ u'host/%s@%s' % (get_fqdn(host1), api.env.realm)], +krbcanonicalname=[ +u'host/%s@%s' % (get_fqdn(host1), api.env.realm)], objectclass=objectclasses.host, ipauniqueid=[fuzzy_uuid], managedby_host=[get_fqdn(host1)], @@ -832,6 +834,8 @@ class test_idviews(Declarative): l=[u'Undisclosed location 2'], krbprincipalname=[ u'host/%s@%s' % (get_fqdn(host2), api.env.realm)], +krbcanonicalname=[ +u'host/%s@%s' % (get_fqdn(host2), api.env.realm)], objectclass=objectclasses.host, ipauniqueid=[fuzzy_uuid], managedby_host=[get_fqdn(host2)], @@ -860,6 +864,8 @@ class test_idviews(Declarative): l=[u'Undisclosed location 3'], krbprincipalname=[ u'host/%s@%s' % (get_fqdn(host3), api.env.realm)], +krbcanonicalname=[ +u'host/%s@%s' % (get_fqdn(host3), api.env.realm)], objectclass=objectclasses.host, ipauniqueid=[fuzzy_uuid], managedby_host=[get_fqdn(host3)], @@ -1453,6 +1459,8 @@ class test_idviews(Declarative): l=[u'Undisclosed location 4'], krbprincipalname=[ u'host/%s@%s' % (get_fqdn(host4), api.env.realm)], +krbcanonicalname=[ +u'host/%s@%s' % (get_fqdn(host4), api.env.realm)], objectclass=objectclasses.host, ipauniqueid=[fuzzy_uuid], managedby_host=[get_fqdn(host4)], -- 2.7.4 -- 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
[Freeipa-devel] [PATCHES 0038][Tests] ID views does not recognize ipakrboktoauthasdelegate attribute
Hi, due to implementation of [1] some ID views tests fail because they do not recognize ipakrboktoauthasdelegate attribute. Providing fix for this. Ticket: https://fedorahosted.org/freeipa/ticket/6241 Lenka [1] https://fedorahosted.org/freeipa/ticket/5764 From 8b4fb5f85f81bc1f414a4e95731ca0f763c2fe18 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 22 Aug 2016 13:39:32 +0200 Subject: [PATCH] Tests: ID views tests do not recognize ipakrboktoauthasdelegate sttribute Due to implementation of [1], new attribute 'ipakrboktoauthasdelegate' was presented, but is not recognized by ID views tests, thus causing them to fail. [1] https://fedorahosted.org/freeipa/ticket/5764 https://fedorahosted.org/freeipa/ticket/6241 --- ipatests/test_xmlrpc/test_idviews_plugin.py | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ipatests/test_xmlrpc/test_idviews_plugin.py b/ipatests/test_xmlrpc/test_idviews_plugin.py index 9cd44fe2f8263c14015001af6a79e10ff9801903..eeadc7ceca6c81afe5004898f67e0727f480efe3 100644 --- a/ipatests/test_xmlrpc/test_idviews_plugin.py +++ b/ipatests/test_xmlrpc/test_idviews_plugin.py @@ -1027,6 +1027,7 @@ class test_idviews(Declarative): objectclass=objectclasses.host, serverhostname=[host3], ipaassignedidview=[idview1], +ipakrboktoauthasdelegate=False, ), ), ), @@ -1056,6 +1057,7 @@ class test_idviews(Declarative): serverhostname=[host2], memberof_hostgroup=[hostgroup2], memberofindirect_hostgroup=[hostgroup1], +ipakrboktoauthasdelegate=False, ), ), ), @@ -1109,6 +,7 @@ class test_idviews(Declarative): memberof_hostgroup=[hostgroup2], memberofindirect_hostgroup=[hostgroup1], ipaassignedidview=[idview1], +ipakrboktoauthasdelegate=False, ), ), ), @@ -1138,6 +1141,7 @@ class test_idviews(Declarative): serverhostname=[host1], memberof_hostgroup=[hostgroup1], ipaassignedidview=[idview1], +ipakrboktoauthasdelegate=False, ), ), ), @@ -1210,6 +1214,7 @@ class test_idviews(Declarative): objectclass=objectclasses.host, serverhostname=[host1], memberof_hostgroup=[hostgroup1], +ipakrboktoauthasdelegate=False, ), ), ), @@ -1237,6 +1242,7 @@ class test_idviews(Declarative): managing_host=[get_fqdn(host3)], objectclass=objectclasses.host, serverhostname=[host3], +ipakrboktoauthasdelegate=False, ), ), ), @@ -1489,6 +1495,7 @@ class test_idviews(Declarative): managing_host=[get_fqdn(host4)], objectclass=objectclasses.host, serverhostname=[host4], +ipakrboktoauthasdelegate=False, ), ), ), -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0036, 0037][Tests] Host/service tests do not recognize newly added attribute
Hi, attached patches fix test fails occuring since patch for [1] was pushed. Ticket for tests: https://fedorahosted.org/freeipa/ticket/6240 Lenka [1] https://fedorahosted.org/freeipa/ticket/5764 From 4e152c92008ebcab69aa07d2a1f50649e71563ab Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 22 Aug 2016 10:32:50 +0200 Subject: [PATCH 1/2] Tests: Host tracker does not recognize 'ipakrboktoauthasdelegate' attribute Due to [1] being implemented, retrieve and search tests with --all option specified fail due to extra attribute. [1] https://fedorahosted.org/freeipa/ticket/5764 Ticket: https://fedorahosted.org/freeipa/ticket/6240 --- ipatests/test_xmlrpc/tracker/host_plugin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py index 45be169e0c9567f8c7d7a73f2eb8155e9c3d6cfc..756190667bdfb56db970a3914814b07fc4c8b9dc 100644 --- a/ipatests/test_xmlrpc/tracker/host_plugin.py +++ b/ipatests/test_xmlrpc/tracker/host_plugin.py @@ -40,7 +40,7 @@ class HostTracker(KerberosAliasMixin, Tracker): retrieve_all_keys = retrieve_keys | { u'cn', u'ipakrbokasdelegate', u'ipakrbrequirespreauth', u'ipauniqueid', u'krbcanonicalname', u'managing_host', u'objectclass', -u'serverhostname'} +u'serverhostname', u'ipakrboktoauthasdelegate'} create_keys = retrieve_keys | {'objectclass', 'ipauniqueid', 'randompassword'} update_keys = retrieve_keys - {'dn'} @@ -112,6 +112,7 @@ class HostTracker(KerberosAliasMixin, Tracker): ipakrbrequirespreauth=True, managing_host=[self.fqdn], serverhostname=[self.shortname], +ipakrboktoauthasdelegate=False, ) self.exists = True -- 2.7.4 From 36695099f1c2b500c7bb1633db387775f1e2ff3d Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 22 Aug 2016 12:08:04 +0200 Subject: [PATCH 2/2] Tests: Service tracker and tests don't recognize 'ipakrboktoauthasdelegate' attribute Due to [1] being implemented, retrieve and search tests with --all option specified fail due to extra attribute. [1] https://fedorahosted.org/freeipa/ticket/5764 Ticket: https://fedorahosted.org/freeipa/ticket/6240 --- ipatests/test_xmlrpc/test_service_plugin.py| 2 ++ ipatests/test_xmlrpc/tracker/service_plugin.py | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py index 56e2c7a7a0570cb15dc5d27dcdba488da089350f..0e8c8ea30aa48a4e47b5a6f816995f6a7c81f258 100644 --- a/ipatests/test_xmlrpc/test_service_plugin.py +++ b/ipatests/test_xmlrpc/test_service_plugin.py @@ -256,6 +256,7 @@ class test_service(Declarative): has_keytab=False, ipakrbrequirespreauth=True, ipakrbokasdelegate=False, +ipakrboktoauthasdelegate=False, ), ), ), @@ -319,6 +320,7 @@ class test_service(Declarative): managedby_host=[fqdn1], ipakrbrequirespreauth=True, ipakrbokasdelegate=False, +ipakrboktoauthasdelegate=False, ), ], ), diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py index 3b970b98985f6d0528aba369064f889256853b79..fe34390e268b53cc4717ce75aff139c510a7 100644 --- a/ipatests/test_xmlrpc/tracker/service_plugin.py +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -44,7 +44,7 @@ class ServiceTracker(KerberosAliasMixin, Tracker): u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData', u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof', u'objectClass', u'ipakrbrequirespreauth', -u'ipakrbokasdelegate'} +u'ipakrbokasdelegate', u'ipakrboktoauthasdelegate'} create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - { u'usercertificate', u'has_keytab'} @@ -94,7 +94,8 @@ class ServiceTracker(KerberosAliasMixin, Tracker): u'ipauniqueid': [fuzzy_uuid], u'managedby_host': [self.host_fqdn], u'krbcanonicalname': [u'{0}'.format(self.name)], -u'has_keytab': False +u'has_keytab': False, +u'ipakrboktoauthasdelegate': False, } for key in self.options: -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0035][Tests] Fix failing tests in test_ipalib/test_frontend
On 08/17/2016 04:57 PM, Milan Kubík wrote: On 08/17/2016 04:45 PM, Lenka Doudova wrote: Hi, attached patch provides fix for 2 out of three failing tests in ipatests/test_ipalib/test_frontend.py. Failures were caused by changes related to thin client implementation. Fix for the third failure will be provided later (after my PTO), as it will be more complicated fix. Lenka NACK: * Module ipatests.test_ipalib.test_frontend ipatests/test_ipalib/test_frontend.py:944: [E1120(no-value-for-parameter), test_Object.test_init.FakeAPI] No value for argument 'base' in constructor call) -- Milan Kubik Oh sorry, I attached non-updated patch... Correct one attached now. Lenka From 6838b13e2eec3aab2aab5f67dbc273a3801f352c Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 17 Aug 2016 16:37:29 +0200 Subject: [PATCH] Tests: Fix failing tests in test_ipalib/test_frontend Some tests in ipatests/test_ipalib/test_frontend.py are failing due to changes related to thin client implementation. Providing fix for: ipa.test_ipalib.test_frontend.test_Attribute.test_init ipa.test_ipalib.test_frontend.test_LocalOrRemote.test_run https://fedorahosted.org/freeipa/ticket/6188 --- ipalib/frontend.py| 2 +- ipatests/test_ipalib/test_frontend.py | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ipalib/frontend.py b/ipalib/frontend.py index 455b222d4d7fcbb65b43c4d8e1ffbbaf3e131d22..554d899d97539ab551abaa9983d68450e86454b1 100644 --- a/ipalib/frontend.py +++ b/ipalib/frontend.py @@ -1181,7 +1181,7 @@ class LocalOrRemote(Command): When running in a server context, this command is always executed locally and the value of ``options['server']`` is ignored. """ -if options['server'] and not self.env.in_server: +if options.get('server', False) and not self.env.in_server: return self.forward(*args, **options) return self.execute(*args, **options) diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py index c3dd9104c152dc9e9da774a53e0c6a42b9a89bf8..892328a37a8c923fbcf0236ccabb1c8b2d3183f3 100644 --- a/ipatests/test_ipalib/test_frontend.py +++ b/ipatests/test_ipalib/test_frontend.py @@ -862,13 +862,13 @@ class test_LocalOrRemote(ClassChecker): api.finalize() cmd = api.Command.example assert cmd(version=u'2.47') == dict( -result=('execute', (None,), dict(version=u'2.47', server=False)) +result=('execute', (), dict(version=u'2.47')) ) assert cmd(u'var', version=u'2.47') == dict( -result=('execute', (u'var',), dict(version=u'2.47', server=False)) +result=('execute', (u'var',), dict(version=u'2.47')) ) assert cmd(server=True, version=u'2.47') == dict( -result=('forward', (None,), dict(version=u'2.47', server=True)) +result=('forward', (), dict(version=u'2.47', server=True)) ) assert cmd(u'var', server=True, version=u'2.47') == dict( result=('forward', (u'var',), dict(version=u'2.47', server=True)) @@ -880,13 +880,13 @@ class test_LocalOrRemote(ClassChecker): api.finalize() cmd = api.Command.example assert cmd(version=u'2.47') == dict( -result=('execute', (None,), dict(version=u'2.47', server=False)) +result=('execute', (), dict(version=u'2.47', server=False)) ) assert cmd(u'var', version=u'2.47') == dict( result=('execute', (u'var',), dict(version=u'2.47', server=False)) ) assert cmd(server=True, version=u'2.47') == dict( -result=('execute', (None,), dict(version=u'2.47', server=True)) +result=('execute', (), dict(version=u'2.47', server=True)) ) assert cmd(u'var', server=True, version=u'2.47') == dict( result=('execute', (u'var',), dict(version=u'2.47', server=True)) @@ -1106,7 +1106,7 @@ class test_Attribute(ClassChecker): """ user_obj = 'The user frontend.Object instance' class api(object): -Object = dict(user=user_obj) +Object = {("user", "1"): user_obj} @staticmethod def is_production_mode(): return False -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0035][Tests] Fix failing tests in test_ipalib/test_frontend
Hi, attached patch provides fix for 2 out of three failing tests in ipatests/test_ipalib/test_frontend.py. Failures were caused by changes related to thin client implementation. Fix for the third failure will be provided later (after my PTO), as it will be more complicated fix. Lenka From 0c98433e0604fd7056899e4aec47f561cbc4b0bb Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 17 Aug 2016 16:37:29 +0200 Subject: [PATCH] Tests: Fix failing tests in test_ipalib/test_frontend Some tests in ipatests/test_ipalib/test_frontend.py are failing due to changes related to thin client implementation. Providing fix for: ipa.test_ipalib.test_frontend.test_Attribute.test_init ipa.test_ipalib.test_frontend.test_LocalOrRemote.test_run https://fedorahosted.org/freeipa/ticket/6188 --- ipalib/frontend.py| 2 +- ipatests/test_ipalib/test_frontend.py | 15 --- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ipalib/frontend.py b/ipalib/frontend.py index 455b222d4d7fcbb65b43c4d8e1ffbbaf3e131d22..554d899d97539ab551abaa9983d68450e86454b1 100644 --- a/ipalib/frontend.py +++ b/ipalib/frontend.py @@ -1181,7 +1181,7 @@ class LocalOrRemote(Command): When running in a server context, this command is always executed locally and the value of ``options['server']`` is ignored. """ -if options['server'] and not self.env.in_server: +if options.get('server', False) and not self.env.in_server: return self.forward(*args, **options) return self.execute(*args, **options) diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py index c3dd9104c152dc9e9da774a53e0c6a42b9a89bf8..9ca2475030685dbcda6b4ef3cdeded7bb11e0706 100644 --- a/ipatests/test_ipalib/test_frontend.py +++ b/ipatests/test_ipalib/test_frontend.py @@ -35,6 +35,7 @@ from ipalib import frontend, backend, plugable, errors, parameters, config from ipalib import output, messages from ipalib.parameters import Str from ipapython.version import API_VERSION +from ipalib.plugable import APINameSpace if six.PY3: unicode = str @@ -862,13 +863,13 @@ class test_LocalOrRemote(ClassChecker): api.finalize() cmd = api.Command.example assert cmd(version=u'2.47') == dict( -result=('execute', (None,), dict(version=u'2.47', server=False)) +result=('execute', (), dict(version=u'2.47')) ) assert cmd(u'var', version=u'2.47') == dict( -result=('execute', (u'var',), dict(version=u'2.47', server=False)) +result=('execute', (u'var',), dict(version=u'2.47')) ) assert cmd(server=True, version=u'2.47') == dict( -result=('forward', (None,), dict(version=u'2.47', server=True)) +result=('forward', (), dict(version=u'2.47', server=True)) ) assert cmd(u'var', server=True, version=u'2.47') == dict( result=('forward', (u'var',), dict(version=u'2.47', server=True)) @@ -880,13 +881,13 @@ class test_LocalOrRemote(ClassChecker): api.finalize() cmd = api.Command.example assert cmd(version=u'2.47') == dict( -result=('execute', (None,), dict(version=u'2.47', server=False)) +result=('execute', (), dict(version=u'2.47', server=False)) ) assert cmd(u'var', version=u'2.47') == dict( result=('execute', (u'var',), dict(version=u'2.47', server=False)) ) assert cmd(server=True, version=u'2.47') == dict( -result=('execute', (None,), dict(version=u'2.47', server=True)) +result=('execute', (), dict(version=u'2.47', server=True)) ) assert cmd(u'var', server=True, version=u'2.47') == dict( result=('execute', (u'var',), dict(version=u'2.47', server=True)) @@ -940,7 +941,7 @@ class test_Object(ClassChecker): methods_format = 'method_%d' class FakeAPI(object): -Method = NameSpace( +Method = APINameSpace( get_attributes(cnt, methods_format) ) def __contains__(self, key): @@ -1106,7 +1107,7 @@ class test_Attribute(ClassChecker): """ user_obj = 'The user frontend.Object instance' class api(object): -Object = dict(user=user_obj) +Object = {("user", "1"): user_obj} @staticmethod def is_production_mode(): return False -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0034][Tests] Fix failing tests in test_ipalib/test_parameters
Hi, attached patch fixes part of failing tests in ipatests/test_ipalib/test_parameters.py. Failures were caused mainly by thin client feature, sometimes by usage of unicode, which tests did not reflect. Issues were discussed with Honza. Remaining failing tests should be fixed in scope of one of Martin3's future patches (namely failures described in https://fedorahosted.org/freeipa/ticket/6190) Lenka From 8a58bb5996658e0d58621f0e88278ffaf4818dea Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 17 Aug 2016 15:39:54 +0200 Subject: [PATCH] Tests: Fix failing tests in test_ipalib/test_parameters Some of the tests are failing due to changes introduced because of thin client feature. https://fedorahosted.org/freeipa/ticket/6187 https://fedorahosted.org/freeipa/ticket/6224 --- ipatests/test_ipalib/test_parameters.py | 37 ++--- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/ipatests/test_ipalib/test_parameters.py b/ipatests/test_ipalib/test_parameters.py index aa14ef955c2351feffef4a57bd49b195a0bbc064..621dace59d2edf911af531017d7e9a7283f7c5cc 100644 --- a/ipatests/test_ipalib/test_parameters.py +++ b/ipatests/test_ipalib/test_parameters.py @@ -98,18 +98,18 @@ class test_DefaultFrom(ClassChecker): pass o = self.cls(stuff) -assert repr(o) == "DefaultFrom(stuff, 'one', 'two')" +assert repr(o) == "DefaultFrom('one', 'two')" o = self.cls(stuff, 'aye', 'bee', 'see') -assert repr(o) == "DefaultFrom(stuff, 'aye', 'bee', 'see')" +assert repr(o) == "DefaultFrom('aye', 'bee', 'see')" cb = lambda first, last: first[0] + last o = self.cls(cb) -assert repr(o) == "DefaultFrom(, 'first', 'last')" +assert repr(o) == "DefaultFrom('first', 'last')" o = self.cls(cb, 'aye', 'bee', 'see') -assert repr(o) == "DefaultFrom(, 'aye', 'bee', 'see')" +assert repr(o) == "DefaultFrom('aye', 'bee', 'see')" def test_call(self): """ @@ -301,9 +301,9 @@ class test_Param(ClassChecker): o = self.cls(name) assert repr(o) == 'Param(%r)' % name o = self.cls('name', required=False) -assert repr(o) == "Param('name', required=False)" +assert repr(o) == "Param('name?')" o = self.cls('name', multivalue=True) -assert repr(o) == "Param('name', multivalue=True)" +assert repr(o) == "Param('name+')" def test_use_in_context(self): """ @@ -352,17 +352,17 @@ class test_Param(ClassChecker): assert type(clone) is self.cls assert clone.name is orig.name for (key, kind, default) in self.cls.kwargs: -assert getattr(clone, key) is getattr(orig, key) +assert getattr(clone, key) == getattr(orig, key) # Test with a param spec: orig = self.cls('my_param*') assert orig.param_spec == 'my_param*' clone = orig.clone() -assert clone.param_spec == 'my_param' +assert clone.param_spec == 'my_param*' assert clone is not orig assert type(clone) is self.cls for (key, kind, default) in self.cls.kwargs: -assert getattr(clone, key) is getattr(orig, key) +assert getattr(clone, key) == getattr(orig, key) # Test with overrides: orig = self.cls('my_param*') @@ -373,7 +373,7 @@ class test_Param(ClassChecker): assert type(clone) is self.cls assert clone.required is True assert clone.multivalue is True -assert clone.param_spec == 'my_param' +assert clone.param_spec == 'my_param+' assert clone.name == 'my_param' def test_clone_rename(self): @@ -389,6 +389,8 @@ class test_Param(ClassChecker): assert type(clone) is self.cls assert clone.name == new_name for (key, kind, default) in self.cls.kwargs: +if key in ('cli_name', 'label', 'doc', 'cli_metavar'): +continue assert getattr(clone, key) is getattr(orig, key) # Test with overrides: @@ -400,7 +402,7 @@ class test_Param(ClassChecker): assert type(clone) is self.cls assert clone.required is True assert clone.multivalue is True -assert clone.param_spec == new_name +assert clone.param_spec == "{0}+"
[Freeipa-devel] [PATCH 0033][Tests] Fix test_ipalib/test_output failing due to change of Output class behaviour
Hi, attaching patch for https://fedorahosted.org/freeipa/ticket/6189 Lenka From 9e0591f6099c587218aa8155113d3e7bd85fe9f4 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 15 Aug 2016 14:24:11 +0200 Subject: [PATCH] Tests: test_ipalib/test_output fails due to change of Output behaviour https://fedorahosted.org/freeipa/ticket/6189 --- ipatests/test_ipalib/test_output.py | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ipatests/test_ipalib/test_output.py b/ipatests/test_ipalib/test_output.py index 5741637f9734e33be1e1a7e1f44099c53c8789c8..ed734ffd4b9ad99657ada0d3bcfc35db6c4b8c73 100644 --- a/ipatests/test_ipalib/test_output.py +++ b/ipatests/test_ipalib/test_output.py @@ -51,16 +51,16 @@ class test_Output(ClassChecker): Test the `ipalib.output.Output.__repr__` method. """ o = self.cls('aye') -assert repr(o) == "Output('aye', None, None)" +assert repr(o) == "Output('aye')" o = self.cls('aye', type=int, doc='An A, aye?') -assert repr(o) == "Output('aye', %r, 'An A, aye?')" % int +assert repr(o) == "Output('aye', type=[%r], doc='An A, aye?')" % int class Entry(self.cls): pass o = Entry('aye') -assert repr(o) == "Entry('aye', None, None)" +assert repr(o) == "Entry('aye')" o = Entry('aye', type=int, doc='An A, aye?') -assert repr(o) == "Entry('aye', %r, 'An A, aye?')" % int +assert repr(o) == "Entry('aye', type=[%r], doc='An A, aye?')" % int class test_ListOfEntries(ClassChecker): -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0031, 0032][Tests] Fixes for failing test_ipalib/test_messages tests
Hi, attached are patches that are fixing 3 failing tests in test_ipalib/test_messages.py. Lenka From 11bd09d8b82630b959deebe265320221db815540 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 15 Aug 2016 11:10:57 +0200 Subject: [PATCH 1/2] Fix malformed or missing docstrings in ipalib/messages Some of the docstrings in ipalib/messages.py are malformed or missing entirely. This causes test_ipalib/test_messages to fail due to non-matching regex. https://fedorahosted.org/freeipa/ticket/6215 --- ipalib/messages.py | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ipalib/messages.py b/ipalib/messages.py index 6abad64a8259a8e164db60f63e75bbb9c230e7bf..02b0a0e1021fc2c59a139746eb35fb2d24b78945 100644 --- a/ipalib/messages.py +++ b/ipalib/messages.py @@ -397,7 +397,7 @@ class DNSForwardPolicyConflictWithEmptyZone(PublicMessage): class DNSUpdateOfSystemRecordFailed(PublicMessage): """ -** 13022 ** Update of a DNS system record failed +**13022** Update of a DNS system record failed """ errno = 13022 type = "warning" @@ -408,7 +408,7 @@ class DNSUpdateOfSystemRecordFailed(PublicMessage): class DNSUpdateNotIPAManagedZone(PublicMessage): """ -** 13023 ** Zone for system records is not managed by IPA +**13023** Zone for system records is not managed by IPA """ errno = 13023 type = "warning" @@ -419,6 +419,9 @@ class DNSUpdateNotIPAManagedZone(PublicMessage): class AutomaticDNSRecordsUpdateFailed(PublicMessage): +""" +**13024** Automatic update of DNS records failed +""" errno = 13024 type = "warning" format = _( @@ -429,6 +432,9 @@ class AutomaticDNSRecordsUpdateFailed(PublicMessage): class ServiceRestartRequired(PublicMessage): +""" +**13025** Service restart is required +""" errno = 13025 type = "warning" format = _( @@ -438,6 +444,9 @@ class ServiceRestartRequired(PublicMessage): class LocationWithoutDNSServer(PublicMessage): +""" +**13026** Location without DNS server +""" errno = 13026 type = "warning" format = _( @@ -464,7 +473,7 @@ class ServerRemovalWarning(PublicMessage): class CertificateInvalid(PublicMessage): """ -***13029 Failed to parse a certificate +**13029** Failed to parse a certificate """ errno = 13029 type = "error" -- 2.7.4 From 11f4236f73bcff46297b24c5fde5f30e637d9b0c Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 15 Aug 2016 11:19:38 +0200 Subject: [PATCH] Tests: Add data attribute to messages Tests test_ipalib/test_messages.py are failing because messages now contain also 'data' attribute, which is not yet reflected in tests. https://fedorahosted.org/freeipa/ticket/6185 --- ipatests/test_ipalib/test_messages.py | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/ipatests/test_ipalib/test_messages.py b/ipatests/test_ipalib/test_messages.py index dad0e988a6f0da1486e02af6c35e6657029e07ac..f6508b98a3cc2ae2d734fc92300fe858c59d6f58 100644 --- a/ipatests/test_ipalib/test_messages.py +++ b/ipatests/test_ipalib/test_messages.py @@ -55,10 +55,11 @@ class test_PublicMessages(test_errors.BaseMessagesTest): def test_to_dict(): expected = dict( -name='HelloMessage', -type='info', -message='Hello, world!', +name=u'HelloMessage', +type=u'info', +message=u'Hello, world!', code=1234, +data={'greeting': 'Hello', 'object': 'world'}, ) assert HelloMessage(greeting='Hello', object='world').to_dict() == expected @@ -78,15 +79,17 @@ def test_add_message(): assert result == {'messages': [ dict( -name='HelloMessage', -type='info', -message='Hello, world!', +name=u'HelloMessage', +type=u'info', +message=u'Hello, world!', code=1234, +data={'greeting': 'Hello', 'object': 'world'}, ), dict( -name='HelloMessage', -type='info', -message='Hi, version!', +name=u'HelloMessage', +type=u'info', +message=u'Hi, version!', code=1234, +data={'greeting': 'Hi', 'object': 'version'}, ) ]} -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 08/10/2016 05:48 PM, Martin Basti wrote: On 08.08.2016 10:30, Martin Basti wrote: On 02.08.2016 14:50, Lenka Doudova wrote: On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka Hello, 1) This is not needed +global sn + +result = api.Command.cert_show(sn, out=unicode(self.certfile)) you need the global statement only for write access. But sn is not assigned in this code block. 2) I prefer to use instance attributes (self.sn) instead of global variables As we figured out, pytest creates for each test new instance of class, so 2) will not fork. Please fix only 1), sorry. Martin^2 Hi, attached fixed patches for master and 4.3 branches. Lenka Martin^2 From 0c11503843cd4c69549751cbb3e2113e79f196d6 Mon Sep 17 00:00:00 2001 From: Peter Lacko Date: Fri, 15 Jul 2016 16:55:51 +0200 Subject: [PATCH] Test URIs in certificate. Test that CRL URI and OCSP URI are present and correct in generated certificate. https://fedorahosted.org/freeipa/ticket/5881 --- ipatests/test_xmlrpc/test_cert_plugin.py | 49 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index a3839d0f79af7208bc2e9ce54183dec288f79ff1..d6a0bca72a272cd51e7da7728fd1a2618a27ff7a 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -19,24 +19,26 @@ """ Test the `ipalib/plugins/cert.py` module against a RA. """ +from __future__ import print_function import sys +import base64 +import nose import os +import pytest import shutil -from nose.tools import raises, assert_raises # pylint: disable=E0611 -from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +import six +import tempfile from ipalib import api from ipalib import errors from ipalib import x509 -import tempfile -from ipapython import ipautil -import six -import nose -import base64 from ipaplatform.paths import paths +from ipapython import ipautil from ipapython.dn import DN -import pytest +from ipapython.ipautil import run +from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +from nose.tools import raises, assert_raises if six.PY3: unicode = str @@ -44,6 +46,11 @@ if six.PY3: # So we can save the cert from issuance and compare it later cert = None newcert = None +sn = None + +_DOMAIN = api.env.domain +_EXP_CRL_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ipa/crl/MasterCRL.bin']) +_EXP_OCSP_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ca/ocsp']) def is_db_configured(): """ @@ -82,6 +89,8 @@ class test_cert(XMLRPC_test): if 'cert_request' not in api.Command: raise nose.SkipTest('cert_request not registered') +if 'cert_show' not in api.Command: +raise nose.SkipTest('cert_show not registered') is_db_configured() @@ -94,6 +103,7 @@ class test_cert(XMLRPC_test): self.reqdir = tempfile.mkdtemp(prefix = "tmp-") self.reqfile = self.reqdir + "/test.csr" self.pwname = self.reqdir + "/pwd" +self.certfile = self.reqdir + "/cert.crt" # Create an empty password file fp = open(self.pwname, "w") @@ -144,13 +154,15 @@ class test_cert(XMLRPC_test): Test the `xmlrpc.cert_request` method with --add.
Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests
On 07/20/2016 05:31 PM, Peter Lacko wrote: Sorry for late reply, I was waiting how the discussion with tracker improvement will end, but since there's no progress and I'm leaving soon, I'm attaching new patch. I also created mapping between old and new tests [1], to make life of reviewer easier. Number in first column denotes line number in original file, followed by line number in new tests file and its name. Tests that are not mentioned in there extend previous coverage. Regards, Peter [1] http://etherpad.corp.redhat.com/Vk3tRLaPSK Hi, review notes: 1) code related: * leftover PEP8 error: ./ipatests/test_xmlrpc/test_role_plugin.py:696:1: W391 blank line at end of file * in PrivilegeTracker: o creating privilege with description does not work properly, since there's a hardcoded value in track_create method o the check_retrieve method expects 'description' to be returned only when privilege is member of a role, but to the extent of my knowledge that value is returned always * in RoleTracker: o global variable 'member_types' is used only inside the class, so it should be a class variable rather than global o 'check_add_duplicate_member' method uses oneliner to create basis of expected value - suggest to use this more in other methods that do it 'literally' (e.g. check_add_member) o 'update_tracker' method - the main else statement should be investigated more, I'm not sure the try-except part is valid (if I expect the tracker to delete an attribute, but that attribute is not present, it's a pass? Doesn't sound right.) o 'update' method is needlessly simple - look at the same method in BaseTracker, which contains more method calls. Result of the simplicity is that in actual role tests, these other methods like 'check_update' etc. are called outside the 'update' method, when they can just as well be part of the method and save repeating same code over and over. This goes for 'retrieve' and 'find' methods as well. * in role tests (ipatests/test_xmlrpc/test_role_plugin.py): o in class TestDeleredRole, there's unused method setUpClass 2) other: * I strongly suggest to go through all documentation comments, since some of them are vague, or even straight misleading (e.g. in RoleTracker, method 'find_tracker_roles': comment says, that it performs find command, but nothing like that happens!) * similarly as in previous note, please go through all parameter descriptions in the documentation - e.g. in RoleTracker, method 'update_tracker': ":param expected_updates: Dictionary of other attributes" - such description is quite unsatisfactory) * when doing previous two, there are some typos that could be eliminated * some test cases in ipatests/test_xmlrpc/test_role_plugin.py seem incorrectly classified, e.g. method 'test_create_invalid' verifying attempt to create role with invalid name in TestNonexistentRole class that performes operations like role-show, role-delete on nonexistent entries * for future patches, it might be nice to have separate patches for each tracker and the tests Also, since the author Peter Lacko left the company, fixing this patch will be a task for Ganna. Lenka - Original Message - From: "Martin Basti" To: "Peter Lacko" Cc: freeipa-devel@redhat.com Sent: Monday, June 6, 2016 12:29:29 PM Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests On 02.06.2016 16:16, Peter Lacko wrote: Rebased with updated tests. Peter - Original Message - From: "Martin Basti" To: "Peter Lacko" Cc: freeipa-devel@redhat.com Sent: Thursday, June 2, 2016 1:50:06 PM Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests Your patch doesn't apply anymore on master, there were changes in role test and patch have to be updated and rebased Please look at this for new changes in test_role_plugin.py https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1 Martin^2 On 02.06.2016 11:49, Peter Lacko wrote: Sorry for late response, I wasn't working these days. Fixed patch is in attachment. Peter - Original Message - From: "Martin Basti" To: "Peter Lacko" , freeipa-devel@redhat.com Sent: Monday, May 9, 2016 1:06:08 PM Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests On 09.05.2016 13:04, Martin Basti wrote: On 09.05.2016 12:19, Peter Lacko wrote: +# pylint: disable=unicode-builtin I'm not doing complete review, just the line above hit my eyes. unicode() is not in Py3 because all strings there are unicode, thus you cannot use it directly, you need something like if six.PY2: str = unicode and use str() everywhere and remove that #pylint line FYI all enabled pylint checks are there for a good reason, be careful with disabling it (mainly disabling it for a whole module) rather ask before if you are not sure
Re: [Freeipa-devel] [PATCH 0196] baseldap: Fix MidairCollision instantiation during entry modification
On 07/26/2016 05:22 PM, Alexander Bokovoy wrote: On Tue, 26 Jul 2016, Martin Babinsky wrote: Fix for https://fedorahosted.org/freeipa/ticket/6097 Since this issue was found during investigation of other ticket[1], you can test it by performing steps to reproduce #6041, but instead of internal error you should see the MidairCollision raised as public error with the right error message. [1] https://fedorahosted.org/freeipa/ticket/6041 I have a preliminary patch for slapi-nis to fix 6041 (attached). Bump for review on this. As for this fix -- ACK. -- Martin^3 Babinsky From 8f0d6dab08f61fe2fd1ad64a63f7ab91fc5227d4 Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Mon, 25 Jul 2016 14:05:08 +0200 Subject: [PATCH] baseldap: Fix MidairCollision instantiation during entry modification https://fedorahosted.org/freeipa/ticket/6097 --- ipaserver/plugins/baseldap.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py index 6107e43a6ee17d9b9a63d9dc109664d8b232069f..f7844e3e7c59c259b9c8367d135b2dbefc3f0016 100644 --- a/ipaserver/plugins/baseldap.py +++ b/ipaserver/plugins/baseldap.py @@ -1466,7 +1466,7 @@ class LDAPUpdate(LDAPQuery, crud.Update): entry_attrs.dn, attrs_list) except errors.NotFound: raise errors.MidairCollision( -format=_('the entry was deleted while being modified') +message=_('the entry was deleted while being modified') ) self.obj.get_indirect_members(entry_attrs, attrs_list) @@ -2344,7 +2344,7 @@ class BaseLDAPModAttribute(LDAPQuery): entry_attrs.dn, attrs_list) except errors.NotFound: raise errors.MidairCollision( -format=_('the entry was deleted while being modified') +message=_('the entry was deleted while being modified') ) for callback in self.get_callbacks('post'): -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka From 63f0efeaf16cff8cb30e6e8e7903722330ae883c Mon Sep 17 00:00:00 2001 From: Peter Lacko Date: Fri, 15 Jul 2016 16:55:51 +0200 Subject: [PATCH] Test URIs in certificate. Test that CRL URI and OCSP URI are present and correct in generated certificate. https://fedorahosted.org/freeipa/ticket/5881 --- ipatests/test_xmlrpc/test_cert_plugin.py | 52 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index a3839d0f79af7208bc2e9ce54183dec288f79ff1..b106c091edde68097c35907ea834b37b64407735 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -19,24 +19,25 @@ """ Test the `ipalib/plugins/cert.py` module against a RA. """ +from __future__ import print_function import sys +import base64 +import nose import os +import pytest import shutil -from nose.tools import raises, assert_raises # pylint: disable=E0611 - -from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +import six +import tempfile from ipalib import api from ipalib import errors from ipalib import x509 -import tempfile -from ipapython import ipautil -import six -import nose -import base64 from ipaplatform.paths import paths +from ipapython import ipautil from ipapython.dn import DN -import pytest +from ipapython.ipautil import run +from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +from nose.tools import raises, assert_raises # pylint: disable=E0611 if six.PY3: unicode = str @@ -44,6 +45,11 @@ if six.PY3: # So we can save the cert from issuance and compare it later cert = None newcert = None +sn = None + +_DOMAIN = api.env.domain +_EXP_CRL_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ipa/crl/MasterCRL.bin']) +_EXP_OCSP_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ca/ocsp']) def is_db_configured(): """ @@ -82,6 +88,8 @@ class test_cert(XMLRPC_test): if 'cert_request' not in api.Command: raise nose.SkipTest('cert_request not registered') +if 'cert_show' not in api.Command: +raise nose.SkipTest('cert_show not registered') is_db_configured() @@ -94,6 +102,7 @@ class test_cert(XMLRPC_test): self.reqdir = tempfile.mkdtemp(prefix = "tmp-") self.reqfile = self.reqdir + "/test.csr" self.pwname = self.reqdir + "/pwd" +self.certfile = self.reqdir + "/cert.crt" # Create an empty password file fp = open(self.pwname, "w") @@ -144,13 +153,15 @@ class test_cert(XMLRPC_test): Test the `xmlrpc.cert_request` method with --add. """ # Our host should exist from previous test -global cert +global cert, sn csr = unicode(self.generateCSR(str(self.subject))) res = api.Command['cert_request'](csr, principal=self.service_princ, add=True)['result'] assert DN(res['subject']) == self.subject # save the cert for the service_show/find tests cert = res['certificate'].encode('ascii') +# save cert's SN for URI test +sn = res['serial_number'] def test_0003_service_show(self): ""&qu
Re: [Freeipa-devel] [PATCH] 0078-82: webui tests: tests for new certificate widget
On 07/29/2016 03:00 PM, Pavel Vomacka wrote: On 07/28/2016 08:16 AM, Lenka Doudova wrote: On 07/20/2016 04:51 PM, Pavel Vomacka wrote: Please review attached patches, which add tests for new certificate widget in WebUI. https://fedorahosted.org/freeipa/ticket/6064 Hi, thanks for patches. Functionally ok, but you have lots of PEP8 errors in patches 78, 80, 81 and 82 -> NACK. Also in patch 82, method test_arbitrary_certificate, comment says user needs to have "arbitrary_cert" configured, but the property in config file is correctly "arbitrary_cert_path", so it's a bit misleading. Patch 79 is OK, ACK. Lenka Thank you for review. Attaching patches which have fixed all pep8 erros. Bad property of config file was also mentioned in patch 81. These are also fixed. -- Pavel^3 Vomacka Hi, all is fine now, ACK for all patches. Lenka -- 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
[Freeipa-devel] [PATCH 0030][Tests] Fix authentication indicators tests failing due to removal of has_keytab key from list of expected attributes of update command
Hi, solution for https://fedorahosted.org/freeipa/ticket/5281 has removed has_keytab attribute from list of expected keys of service-mod command. Attached patch provides a fix for tests that consequently started to fail. Lenka From eb6b692b484816084a48e64f969b28adae9fb967 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 1 Aug 2016 08:00:16 +0200 Subject: [PATCH] Tests: Remove has_keytab from list of expected keys of update command As part of https://fedorahosted.org/freeipa/ticket/5281, the has_keytab attribute was removed from results of service-mod command. Removing this attribute from list of expected keys to prevent failing tests. Ticket: https://fedorahosted.org/freeipa/ticket/6149 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py index e51232f8056908ef298c62db158c128e1d7436b4..3b970b98985f6d0528aba369064f889256853b79 100644 --- a/ipatests/test_xmlrpc/tracker/service_plugin.py +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -48,7 +48,7 @@ class ServiceTracker(KerberosAliasMixin, Tracker): create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - { u'usercertificate', u'has_keytab'} -update_keys = retrieve_keys - {u'dn'} +update_keys = retrieve_keys - {u'dn', u'has_keytab'} def __init__(self, name, host_fqdn, options=None): super(ServiceTracker, self).__init__(default_version=None) -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka -- 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
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To: freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka -- 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
Re: [Freeipa-devel] [PATCH 0027][Tests] Fix failing automember tests in 4.3
On 07/28/2016 06:08 PM, Ganna Kaihorodova wrote: Greetings! ACK Best regards, Ganna Kaihorodova Associate Software Quality Engineer - Original Message - From: "Lenka Doudova" To: "freeipa-devel" Sent: Wednesday, July 13, 2016 3:21:25 PM Subject: [Freeipa-devel] [PATCH 0027][Tests] Fix failing automember tests in 4.3 Hi, providing patch to fix two failing automember tests in 4.3 branch. The reason of the failure was the output normalization (specifically manager attribute for user). The patch is intended for ipa-4-3 branch only. Lenka Hi, minor fix to the patch - added link to ticket to commit message. Lenka From 94b5b6fa52ac8e5984792f47fec06241a383bb51 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 13 Jul 2016 15:14:11 +0200 Subject: [PATCH] Tests: Fix failing automember tests Two tests in xmlrpc/automember suite were failing as a result of manager data normalization in user attributes. Tests are fixed to reflect the change. https://fedorahosted.org/freeipa/ticket/6147 --- ipatests/test_xmlrpc/test_automember_plugin.py | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py index be0f7390565ed739aa66bc0c5c6d23d25d67df92..dde386286d26ddf4537fbc89647f1afecf51fcad 100644 --- a/ipatests/test_xmlrpc/test_automember_plugin.py +++ b/ipatests/test_xmlrpc/test_automember_plugin.py @@ -472,8 +472,7 @@ class test_automember(Declarative): summary=u'Added user "tuser1"', result=get_user_result( user1, u'Test', u'User1', 'add', -manager=[DN(('uid', 'mscott'), ('cn', 'users'), -('cn', 'accounts'), api.env.basedn)] +manager=[manager1] ) ), ), @@ -1394,8 +1393,7 @@ class test_automember(Declarative): result=get_user_result( user1, u'Test', u'User1', 'add', memberof_group=[group1, u'ipausers'], -manager=[DN(('uid', 'mscott'), ('cn', 'users'), -('cn', 'accounts'), api.env.basedn)] +manager=[manager1] ), ), ), -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter -- 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
Re: [Freeipa-devel] [PATCH] 0078-82: webui tests: tests for new certificate widget
On 07/20/2016 04:51 PM, Pavel Vomacka wrote: Please review attached patches, which add tests for new certificate widget in WebUI. https://fedorahosted.org/freeipa/ticket/6064 Hi, thanks for patches. Functionally ok, but you have lots of PEP8 errors in patches 78, 80, 81 and 82 -> NACK. Also in patch 82, method test_arbitrary_certificate, comment says user needs to have "arbitrary_cert" configured, but the property in config file is correctly "arbitrary_cert_path", so it's a bit misleading. Patch 79 is OK, ACK. Lenka -- 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
Re: [Freeipa-devel] [PATCH] webui test: bunch of patches which fix webui patches
On 07/27/2016 03:00 PM, Lenka Doudova wrote: On 07/20/2016 04:43 PM, Pavel Vomacka wrote: On 07/11/2016 06:33 PM, Pavel Vomacka wrote: Hello, please review these patches. First four of them fixes patches and the last one fixes small bug in WebUI which causes that some tests fail. https://fedorahosted.org/freeipa/ticket/6050 https://fedorahosted.org/freeipa/ticket/6052 https://fedorahosted.org/freeipa/ticket/6053 https://fedorahosted.org/freeipa/ticket/6054 4 patches have incorrect information about user who makes the patch. Sending new patches which correct it. -- Pavel^3 Vomacka Hi, thanks for patches, ACK on all. Lenka When pushing, please don't forget to push patch 0073 from the original email. Lenka -- 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
Re: [Freeipa-devel] [PATCH] webui test: bunch of patches which fix webui patches
On 07/20/2016 04:43 PM, Pavel Vomacka wrote: On 07/11/2016 06:33 PM, Pavel Vomacka wrote: Hello, please review these patches. First four of them fixes patches and the last one fixes small bug in WebUI which causes that some tests fail. https://fedorahosted.org/freeipa/ticket/6050 https://fedorahosted.org/freeipa/ticket/6052 https://fedorahosted.org/freeipa/ticket/6053 https://fedorahosted.org/freeipa/ticket/6054 4 patches have incorrect information about user who makes the patch. Sending new patches which correct it. -- Pavel^3 Vomacka Hi, thanks for patches, ACK on all. Lenka -- 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
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/30/2016 01:14 PM, Martin Basti wrote: On 30.06.2016 12:58, Lenka Doudova wrote: On 06/30/2016 12:51 PM, Petr Spacek wrote: On 30.6.2016 12:32, Lenka Doudova wrote: On 06/29/2016 07:49 PM, Petr Spacek wrote: On 29.6.2016 18:52, Lenka Doudova wrote: On 06/29/2016 06:51 PM, Petr Spacek wrote: On 29.6.2016 18:48, Lenka Doudova wrote: On 06/27/2016 11:05 AM, Lenka Doudova wrote: On 06/27/2016 10:33 AM, Martin Babinsky wrote: On 06/27/2016 10:28 AM, Petr Spacek wrote: On 27.6.2016 10:26, Petr Spacek wrote: On 27.6.2016 10:18, Martin Babinsky wrote: On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Btw, this is not a NACK. Well AFAIK the exit status on POSIX systems is encoded into a single byte so you cannot have the return value greater that 255. We would have to devise some mapping between our XMLRPC status codes and subprocess return codes. Some of our exceptions have defined return values outside plain '1', e.g. NotFound has return value of 2. It would be possible to extend this concept on other common errors. Even more importantly, the forward zone is completely unnecessary because DNS when DNS is set up properly. I would simply remove the dnsforwardzone-add. Grr, I meant this: Even more importantly, the forward zone is completely unnecessary when DNS is set up properly. I would simply remove the dnsforwardzone-add. +1, our tests should not fiddle with the provisioned environment as much as they sometimes do. Well, I have nothing against removing it completely, but left it there just because with previous AD machines with "wild" domains it was necessary. Looking at the code, your suggestion would probably mean to entirely remove method configure_dns_for_trust from ipatests/test_integration/tasks.py, right? I'll have to verify this won't break anything else. Lenka Hi, to get back to this issue: do we really want to have the DNS configuration method removed? I mean, we no longer need it for our CI tests, with new AD VMs it works without it, but should somebody else with different setup run these tests, they could experience failures because their AD domain would not be configured in DNS by default and the test would no longer provide that configuration. It doesn't feel right to delete something we needed before but don't need anymore, in case somebody else is depending on the same configuration. But of course, I'll abide by your counsel. In case the call on DNS configuration method really is removed, should I remove even it's definition? It's not used anywhere else, so it would be quite logical. Feel free to keep empty method around as a "hook" for other people. The important thing is that it should do nothing by default. So leave the method call, but erase method contents and let it just pass? Fine with me. (List re-added.) Ah, sorry for doing the wrong reply. Anyway, fixed patch attached. I decided to do as you suggested - the original DNS configuring function is now empty, I modified the comment to explain significance of this strange thing. I also changed patch title to better reflect proposed changes. ACK if it passes your tests. Yes, I've had no problems running the tests since I started to use this. Thanks. Pushed to master: 1d9e1521c59a5b43c2322892ce5cbe8cceff2790 Hi, I just realized the same problem occurs in 4.3 branch, but the original patch was pushed to master only, hence I ask for second review. The originally pushed patch attached, should not need any modifications for ipa-4-3 branch. Ticket: https://fedorahosted.org/freeipa/ticket/6133 Thanks, Lenka From a4772a1b8cc31f0020c86acabbeb944c6d5269b7 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 30 Jun 2016 12:26:28 +0200 Subject: [PATCH] Tests: Remove DNS configuration from trust tests Since DNS configuration is no longer needed for running trust tests, this method's contents are removed. Method is left empty as reference
Re: [Freeipa-devel] [PATCH 0029][Tests] Adding authentication test to trust test suite
On 07/20/2016 02:28 PM, Martin Babinsky wrote: On 07/19/2016 10:41 AM, Lenka Doudova wrote: Hi, this patch adds authentication test (specifically "kinit -E ipauser@IPADOMAIN") to basic trust test suite, as requested by Sumit. Intended to be applied after my patches 25.4 and 26.3 (already waiting to be pushed). Lenka Hi Lenka, Code review: 1.) You have several PEP8 transgressions in the patch, please fix them: """ $ git show -U0 | pep8 --diff ./ipatests/test_integration/test_trust.py:172:34: E127 continuation line over-indented for visual indent ./ipatests/test_integration/test_trust.py:176:35: E128 continuation line under-indented for visual indent ./ipatests/test_integration/test_trust.py:180:27: E231 missing whitespace after ',' """ 2.) + + +def unlock_principal_password(user, oldpw, newpw, master): +container_user = "cn=users,cn=accounts" +basedn = master.domain.basedn + +userdn = "uid={},{},{}".format(user, container_user, basedn) + +args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw, +'-s', newpw, '-x'] +return run(args) there is already a function with the same name in other module: """ git grep -ni 'def unlock_principal_password' ipatests ipatests/test_integration/util.py:82:def unlock_principal_password(user, oldpw, newpw, master): ipatests/util.py:676:def unlock_principal_password(user, oldpw, newpw): """ Having functions with the same names in different modules makes for poor coding practice IMHO. Please rename the function to something like "ldappasswd_user" or something like that so that we have a distinction. Also, you should call ldappasswd directly on master (since you pass it as an argument anyway) using "master.run_command(args)". You should *not* run any in-test code on the controller unless absolutely necessary. You can then remove the ipautil.run import from the beginning of the module. Commit message woes: 1.) vague summary is vague, I would rather see something like: """ test that IPA user can kinit using enterprise principal with IPA domain """ 2.) Commit message body is longer than 78 characters. 3.) there is no ticket URL, I think you can link https://fedorahosted.org/freeipa/ticket/6036 or create a new ticket for the change. Hi, thanks for review, fixed (and renamed) patch attached. Lenka From f93dafb858d55701f52b6f316a20818855a6e180 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 18 Jul 2016 14:38:18 +0200 Subject: [PATCH] Tests: IPA user can kinit using enterprise principal with IPA domain Providing missing test case verifying authentication as IPA user, namely: "kinit -E ipauser@IPADOMAIN". https://fedorahosted.org/freeipa/ticket/6036 --- ipatests/test_integration/test_trust.py | 20 ipatests/test_integration/util.py | 13 + 2 files changed, 33 insertions(+) diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index e3fe9c89e9cd1357c8913d8fcd98699906f07f85..d0b8e58b75551976bcd2739d6811ad7b4b6faad8 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -161,6 +161,26 @@ class TestBasicADTrust(ADTrustBase): assert re.search(testuser_regex, result.stdout_text) +def test_ipauser_authentication(self): +ipauser = u'tuser' +original_passwd = 'Secret123' +new_passwd = 'userPasswd123' + +# create an ipauser for this test +self.master.run_command(['ipa', 'user-add', ipauser, '--first=Test', + '--last=User', '--password'], +stdin_text=original_passwd) + +# change password for the user to be able to kinit +util.ldappasswd_user_change(ipauser, original_passwd, new_passwd, +self.master) + +# try to kinit as ipauser +self.master.run_command( +['kinit', '-E', '{0}@{1}'.format(ipauser, + self.master.domain.name)], +stdin_text=new_passwd) + def test_remove_nonposix_trust(self): tasks.remove_trust_with_ad(self.master, self.ad_domain) tasks.clear_sssd_cache(self.master) diff --git a/ipatests/test_integration/util.py b/ipatests/test_integration/util.py index 594737b6d753d476cd06aeb0d5cd376b7ca46467..179f6727eae2d17f81f6204a504481784c4fa33c 100644 --- a/ipatests/test_integration/util.py +++ b/ipatests/test_integration/util.py @@ -20,6 +20,8 @@ import time import re +from ipaplatform.paths import paths +from ipalib.constants import DEFAULT_CONFIG def run
Re: [Freeipa-devel] [PATCH 0028][Tests] Fix failing user tests
On 07/20/2016 02:04 PM, Martin Babinsky wrote: On 07/15/2016 06:10 PM, Lenka Doudova wrote: Hi, here's patch with fix for failing user tests, specifically tests with renaming users. Failures were caused by RFE Kerberos principal aliases. As part of the fix, I had to rewrite few of the tests themselves, since they used "--setattr" option rather than "--rename" option, which produces different results. Lenka Hi Lenka, I get nice green *user tests with this patch, so functionally it seems ok. However, the commit message does not meet our project standards[1]: 1.) The message summary is very vague, I would rather see something more specific like: """ Tests: improve the handling of rename operations by user tracker """ 2.) The message itself should be wrapped at the maximum of 78 character width (IIRC) but your lines are way too long. A nice way to automate this in vim is to highlight the text, enter command mode and run 'gq', that should format the text for you. 3.) You did not add upstream ticket URL to the end of the message, please do so. There is also an extraneous whitespace here: """ else: if type(value) is list: self.attrs[key] = value else: self.attrs[key] = [value] + for key, value in expected_updates.items(): if value is None or value is '' or value is u'': del self.attrs[key] """ that has nothing to do with the scope of the patch. Please remove it. [1] http://www.freeipa.org/page/Contribute/Patch_Format Hi, thanks for review, fixed patch attached. Lenka From b76139052fa290111642464d3256b4ff15b184eb Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 15 Jul 2016 17:57:53 +0200 Subject: [PATCH] Tests: Improve handling of rename operation by user tracker Improving handling of rename operation by user tracker, together with fixes for user tests, that failed as consequence. Failures were caused by RFE Kerberos principal alias. Some tests were rewritten, since they used "--setattr" option instead of "--rename", and hence didn't reflect proper behaviour of the principal aliases feature. https://fedorahosted.org/freeipa/ticket/6024 --- ipatests/test_xmlrpc/test_user_plugin.py| 31 ++--- ipatests/test_xmlrpc/tracker/user_plugin.py | 9 + 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index def522814f6c0a894f0bd8f352e110a95e5aa09a..7c27abc56cb859eb4fb710f1ff384793dfbe453c 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -316,24 +316,10 @@ class TestUpdate(XMLRPC_test): renameduser.ensure_missing() olduid = user.uid -# using user.update(dict(uid=value)) results in -# OverlapError: overlapping arguments and options: ['uid'] -user.attrs.update(uid=[renameduser.uid]) -command = user.make_update_command( -updates=dict(setattr=(u'uid=%s' % renameduser.uid)) -) -result = command() -user.check_update(result) -user.uid = renameduser.uid +user.update(updates=dict(rename=renameduser.uid)) # rename the test user back so it gets properly deleted -user.attrs.update(uid=[olduid]) -command = user.make_update_command( -updates=dict(setattr=(u'uid=%s' % olduid)) -) -result = command() -user.check_update(result) -user.uid = olduid +user.update(updates=dict(rename=olduid)) def test_rename_to_the_same_value(self, user): """ Try to rename user to the same value """ @@ -640,18 +626,13 @@ class TestUserWithGroup(XMLRPC_test): if its manager is also renamed """ renamed_name = u'renamed_npg2' old_name = user_npg2.uid -command = user_npg2.make_update_command(dict(rename=renamed_name)) -result = command() -user_npg2.attrs.update(uid=[renamed_name]) -user_npg2.check_update(result) + +user_npg2.update(updates=dict(rename=renamed_name)) + user_npg.attrs.update(manager=[renamed_name]) user_npg.retrieve(all=True) -command = user_npg2.make_command( -'user_mod', renamed_name, **dict(rename=old_name) -) -# we rename the user back otherwise the tracker is too confused -result = command() +user_npg2.update(updates=dict(rename=old_name)) def test_check_if_manager_gets_removed(self, user_npg, user_npg2): """ Delete manager and check if it's gone from user's attributes "&
Re: [Freeipa-devel] [PATCH 0024][Tests] Fix integration tests not to produce incorrect /etc/hosts file
On 06/29/2016 06:49 PM, Petr Spacek wrote: On 29.6.2016 18:39, Oleg Fayans wrote: In fact, I believe /etc/hosts file should not be touched at all. Hostname resolution is usually governed by the DNS system of the lab in which tests are running. We do not modify it when perform tests manually, so I'd rather remove this method at all. +1, it should not be need. Let me know if it is needed for some reason and I will have a look. Petr^2 Spacek Hi, providing new (and renamed) patch as was suggested in the discussion above - removing manipulation with /etc/hosts file from the tests. The "fix_etc_hosts" function was completely removed from the tasks file. Verification that nothing is broken by this change was done by running some random integration test (trust tests), and also on Milan's suggestion by running a test requiring two replicas (replica promotion tests). Lenka On 06/29/2016 06:27 PM, Lenka Doudova wrote: Hi all, a function 'fix_etc_hosts' in ipatests/test_integration/tasks.py produces incorrect /etc/hosts file (solitary IPv6 address), and currently parser is not able to resolve the issue, causing ipa-server-install to fail with 'list index out of range' error. Hence I'm attaching patch to fix this issue before parser is fixed (related ticket to it #6014). The fix is just change of regexs responsible for creating incorrect file so that all the lines containing defined hostname are removed, not just specific IP/hostname/shortname strings. Lenka From eb73a40d4294c062a646b8d6cf9fe495382896ed Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 19 Jul 2016 13:12:05 +0200 Subject: [PATCH] Tests: Removing manipulation with /etc/hosts file from integration tests --- ipatests/test_integration/tasks.py | 19 --- 1 file changed, 19 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index cee6c15ccdc260f6ebe4cdebbc2cf6ec84931f27..55cc902b490c57b8741a644ed691254c7d83dc24 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -96,7 +96,6 @@ def allow_sync_ptr(host): def apply_common_fixes(host): -fix_etc_hosts(host) fix_hostname(host) prepare_host(host) @@ -118,24 +117,6 @@ def backup_file(host, filename): return False -def fix_etc_hosts(host): -backup_file(host, paths.HOSTS) -contents = host.get_file_contents(paths.HOSTS) -# Remove existing mentions of the host's FQDN, short name, and IP -# Removing of IP must be done as first, otherwise hosts file may be -# corrupted -contents = re.sub('^%s.*' % re.escape(host.ip), '', contents, - flags=re.MULTILINE) -contents = re.sub('\s%s(\s|$)' % re.escape(host.hostname), ' ', contents, - flags=re.MULTILINE) -contents = re.sub('\s%s(\s|$)' % re.escape(host.shortname), ' ', contents, - flags=re.MULTILINE) -# Add the host's info again -contents += '\n%s %s %s\n' % (host.ip, host.hostname, host.shortname) -log.debug('Writing the following to /etc/hosts:\n%s', contents) -host.put_file_contents(paths.HOSTS, contents) - - def fix_hostname(host): backup_file(host, paths.ETC_HOSTNAME) host.put_file_contents(paths.ETC_HOSTNAME, host.hostname + '\n') -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0029][Tests] Adding authentication test to trust test suite
Hi, this patch adds authentication test (specifically "kinit -E ipauser@IPADOMAIN") to basic trust test suite, as requested by Sumit. Intended to be applied after my patches 25.4 and 26.3 (already waiting to be pushed). Lenka From 394304d23ef752c30cf1f4d69d5e6116fd41ad2d Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 18 Jul 2016 14:38:18 +0200 Subject: [PATCH] Tests: Adding authentication test to basic trust test suite Providing missing test case verifying authentication as IPA user, namely "kinit -E ipauser@IPADOMAIN". --- ipatests/test_integration/test_trust.py | 19 +++ ipatests/test_integration/util.py | 13 + 2 files changed, 32 insertions(+) diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index e3fe9c89e9cd1357c8913d8fcd98699906f07f85..3dc1056dea82d0fe21a983ffe5354ff33e8ad8a3 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -161,6 +161,25 @@ class TestBasicADTrust(ADTrustBase): assert re.search(testuser_regex, result.stdout_text) +def test_ipauser_authentication(self): +ipauser = u'tuser' +original_passwd = 'Secret123' +new_passwd = 'userPasswd123' + +# create an ipauser for this test +self.master.run_command(['ipa', 'user-add', ipauser, '--first=Test', + '--last=User', '--password'], + stdin_text=original_passwd) + +# change password for the user to be able to kinit +util.unlock_principal_password(ipauser, original_passwd, new_passwd, + self.master) + +# try to kinit as ipauser +self.master.run_command( +['kinit', '-E','{0}@{1}'.format(ipauser, self.master.domain.name)], +stdin_text=new_passwd) + def test_remove_nonposix_trust(self): tasks.remove_trust_with_ad(self.master, self.ad_domain) tasks.clear_sssd_cache(self.master) diff --git a/ipatests/test_integration/util.py b/ipatests/test_integration/util.py index 594737b6d753d476cd06aeb0d5cd376b7ca46467..b317beed7756bdc50c6a17c199f4970b5af15303 100644 --- a/ipatests/test_integration/util.py +++ b/ipatests/test_integration/util.py @@ -20,6 +20,8 @@ import time import re +from ipaplatform.paths import paths +from ipapython.ipautil import run def run_repeatedly(host, command, assert_zero_rc=True, test=None, timeout=30, **kwargs): @@ -75,3 +77,14 @@ def get_host_ip_with_hostmask(host): if match: return match.group('full_ip') + + +def unlock_principal_password(user, oldpw, newpw, master): +container_user = "cn=users,cn=accounts" +basedn = master.domain.basedn + +userdn = "uid={},{},{}".format(user, container_user, basedn) + +args = [paths.LDAPPASSWD, '-D', userdn, '-w', oldpw, '-a', oldpw, +'-s', newpw, '-x'] +return run(args) -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0025][Tests] RFE: External trust
On 07/18/2016 04:55 PM, Martin Babinsky wrote: On 07/14/2016 11:42 AM, Lenka Doudova wrote: On 07/13/2016 05:40 PM, Martin Babinsky wrote: On 07/01/2016 04:15 PM, Lenka Doudova wrote: On 07/01/2016 02:38 PM, Martin Babinsky wrote: On 07/01/2016 06:36 AM, Lenka Doudova wrote: On 06/30/2016 05:01 PM, Martin Babinsky wrote: On 06/30/2016 03:47 PM, Lenka Doudova wrote: Hi, attaching patch with some basic coverage for external trust feature. Bit more detailed info in commit message. Since the feature requires me to run commands previously used only for forest root domains even for subdomains, I made some changes in ipatests/test_integration/tasks.py file, so that it would enable me to reuse existing function without copy-pasting them for one variable change. Lenka Hi Lenka, I have a few comments: 1.) -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False): This modification seems extremely kludgy to me. I would rather change the function signature to: -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad_domain, extra_args=()): and pass the domain name itself to the function instead of doing magic inside. The same goes with `remove trust_with_ad`. You may want to fix the calls to them in existing code so that the domain is passed instead of the whole trust object. 2.) +def sync_time_hostname(host, srv_hostname): +""" +Syncs time with the remote server given by its hostname. +Please note that this function leaves ntpd stopped. +""" +host.run_command(['systemctl', 'stop', 'ntpd']) +host.run_command(['ntpdate', srv_hostname]) + + this looks like a duplicate of the function above it, why is it even needed? 3.) +class TestExternalTrustWithSubdomain(ADTrustBase): +""" +Test establishing external trust with subdomain +""" + +@classmethod +def install(cls, mh): +super(ADTrustBase, cls).install(mh) +cls.ad = cls.ad_domains[0].ads[0] +cls.install_adtrust() +cls.check_sid_generation() + +# Determine whether the subdomain AD is available +try: +child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +cls.ad_subdomain = None + +cls.configure_dns_and_time() Please use proper OOP practices when overriding the behavior of the base test class. For starters you could rewrite the install method like this: +@classmethod +def install(cls, mh): +# Determine whether the subdomain AD is available +try: +cls.child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +raise nose.SkipTest("AFAIK this will skip the whole test class") +super(ADTrustBase, cls).install(mh) with child_ad stored as class attribute, you can override `configure_dns_and_time`: +def configure_dns_and_time(cls): +tasks.configure_dns_for_trust(cls.master, cls.ad_subdomain) # no need to re-implement the function just to get to the child AD DC hostname now +tasks.sync_time(cls.master, cls.child_ad.hostname) You can then use this class as an intermediary in the hierarchy and inherit the other external/non-external trust test classes from it, since most setup method in them are just copy-pasted from this one. Hi, thanks for review, fixed patch attached. Lenka Hi Lenka, I am still not happy about the patch underutilizing the potential for a proper inheritance hierarchy and instead relying on a staggering amounts of copy-pasting for implementation. I am attaching a quick untested patch illustrating how the implementation should look like. Also you have some leftovers from previous revision, please fix them: Pylint is running, please wait ... * Module ipatests.test_integration.test_trust ipatests/test_integration/test_trust.py:236: [E1101(no-member), TestExternalTrustWithSubdomain.configure_dns_and_time] Module 'ipatests.test_integration.tasks' has no 'sync_time_hostname' member) ipatests/test_integration/test_trust.py:243: [E1123(unexpected-keyword-arg), TestExternalTrustWithSubdomain.test_establish_trust] Unexpected keyword argument 'subdomain' in function call) ipatests/test_integration/test_trust.py:279: [E1123(unexpected-keyword-arg), TestExternalTrustWithSubdomain.test_remove_nonposix_trust] Unexpected keyword argument 'subdomain' in function call) ipatests/test_integra
[Freeipa-devel] [PATCH 0028][Tests] Fix failing user tests
Hi, here's patch with fix for failing user tests, specifically tests with renaming users. Failures were caused by RFE Kerberos principal aliases. As part of the fix, I had to rewrite few of the tests themselves, since they used "--setattr" option rather than "--rename" option, which produces different results. Lenka From 18968819c48088c77786736cc52271d973e123cd Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 15 Jul 2016 17:57:53 +0200 Subject: [PATCH] Tests: Fix for failing user tests Providing fix for failing user rename tests. Failures were caused by RFE Kerberos principal alias. Some tests were rewritten, since they used "--setattr" option instead of "--rename", and hence didn't reflect proper behaviour of the principal aliases feature. --- ipatests/test_xmlrpc/test_user_plugin.py| 31 ++--- ipatests/test_xmlrpc/tracker/user_plugin.py | 10 ++ 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index def522814f6c0a894f0bd8f352e110a95e5aa09a..7c27abc56cb859eb4fb710f1ff384793dfbe453c 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -316,24 +316,10 @@ class TestUpdate(XMLRPC_test): renameduser.ensure_missing() olduid = user.uid -# using user.update(dict(uid=value)) results in -# OverlapError: overlapping arguments and options: ['uid'] -user.attrs.update(uid=[renameduser.uid]) -command = user.make_update_command( -updates=dict(setattr=(u'uid=%s' % renameduser.uid)) -) -result = command() -user.check_update(result) -user.uid = renameduser.uid +user.update(updates=dict(rename=renameduser.uid)) # rename the test user back so it gets properly deleted -user.attrs.update(uid=[olduid]) -command = user.make_update_command( -updates=dict(setattr=(u'uid=%s' % olduid)) -) -result = command() -user.check_update(result) -user.uid = olduid +user.update(updates=dict(rename=olduid)) def test_rename_to_the_same_value(self, user): """ Try to rename user to the same value """ @@ -640,18 +626,13 @@ class TestUserWithGroup(XMLRPC_test): if its manager is also renamed """ renamed_name = u'renamed_npg2' old_name = user_npg2.uid -command = user_npg2.make_update_command(dict(rename=renamed_name)) -result = command() -user_npg2.attrs.update(uid=[renamed_name]) -user_npg2.check_update(result) + +user_npg2.update(updates=dict(rename=renamed_name)) + user_npg.attrs.update(manager=[renamed_name]) user_npg.retrieve(all=True) -command = user_npg2.make_command( -'user_mod', renamed_name, **dict(rename=old_name) -) -# we rename the user back otherwise the tracker is too confused -result = command() +user_npg2.update(updates=dict(rename=old_name)) def test_check_if_manager_gets_removed(self, user_npg, user_npg2): """ Delete manager and check if it's gone from user's attributes """ diff --git a/ipatests/test_xmlrpc/tracker/user_plugin.py b/ipatests/test_xmlrpc/tracker/user_plugin.py index 1a85e93327e5d517249fd67e208e83a922509002..fca0ab9c46b18ed145990bf04c2444701b8a3675 100644 --- a/ipatests/test_xmlrpc/tracker/user_plugin.py +++ b/ipatests/test_xmlrpc/tracker/user_plugin.py @@ -196,11 +196,18 @@ class UserTracker(Tracker): for key, value in updates.items(): if value is None or value is '' or value is u'': del self.attrs[key] +elif key == 'rename': +new_principal = u'{0}@{1}'.format(value, self.api.env.realm) +self.attrs['uid'] = [value] +self.attrs['krbcanonicalname'] = [new_principal] +if new_principal not in self.attrs['krbprincipalname']: +self.attrs['krbprincipalname'].append(new_principal) else: if type(value) is list: self.attrs[key] = value else: self.attrs[key] = [value] + for key, value in expected_updates.items(): if value is None or value is '' or value is u'': del self.attrs[key] @@ -212,6 +219,9 @@ class UserTracker(Tracker): extra_keys=set(updates.keys()) | set(expected_updates.keys()) ) +if 'rename' in updates: +self.uid = self.attrs['uid'][0] + def check_create
Re: [Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains
On 07/13/2016 06:04 PM, Martin Babinsky wrote: On 07/01/2016 04:45 PM, Lenka Doudova wrote: On 07/01/2016 03:04 PM, Martin Babinsky wrote: On 07/01/2016 11:13 AM, Lenka Doudova wrote: And, of course, a patch file :) On 07/01/2016 11:09 AM, Lenka Doudova wrote: Hi all, here's patch with basic test suite for support of UPN. Note: it needs to be applied on top of my patch 0025.2 (or later, if there's will be more fixes to that patch). Lenka Hi Lenka, test data such as usernames, etc. should be stored either in separate resource files or at least as class attributes like this: diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index e8fdc6b..86ba7cc 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase): """ Test support of UPN for trusted domains """ +upn_suffix = 'UPNsuffix.com' +upn_username = 'upnuser' +upn_princ = '{}@{}'.format(upn_username, upn_suffix) +upn_password = 'Secret123456' + def test_upn_in_nonposix_trust(self): """ Check that UPN is listed as trust attribute """ result = self.master.run_command(['ipa', 'trust-show', self.ad_domain, '--all', '--raw']) -assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text +assert ("ipantadditionalsuffixes: {}".format(self.upn_suffix) in +result.stdout_text) def test_upn_user_resolution_in_nonposix_trust(self): """ Check that user with UPN can be resolved """ -upnuser = 'upnu...@upnsuffix.com' -result = self.master.run_command(['getent', 'passwd', upnuser]) +result = self.master.run_command(['getent', 'passwd', self.upn_princ]) # result will contain AD domain, not UPN -upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN User:/:$".format( -self.ad_domain) +upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format( +self.upn_username, self.ad_domain) assert re.search(upnuser_regex, result.stdout_text) def test_upn_user_authentication(self): """ Check that AD user with UPN can authenticate in IPA """ self.master.run_command(['systemctl', 'restart', 'krb5kdc']) -self.master.run_command(['kinit', '-C', '-E', 'upnu...@upnsuffix.com'], -stdin_text='Secret123456') +self.master.run_command(['kinit', '-C', '-E', self.upn_princ], +stdin_text=self.upn_password) otherwise LGTM. Thanks for review, fixed patch attached. Few notes: 1. mbabinsky's suggestion to store testdata as class attributes or separate resource file: I decided to use the class attribute approach. The separate resource file is a nice idea, which I have already put on my "to do" list - there's a lot of hardcoded stuff in the trust tests, even in the original ones (before my patches), so when there's time I'll work on a way how to dynamically provide this data as test configuration 2. previous discussion about getent vs. pwd.getpwnam(): I'll leave the getent command, since according to mbasti the alternative would not work in CI. Lenka Hi Lenka, I am not sure 'test_all_trustdomains_found' should be run as a part of this test suite. Maybe yes, I'm not sure. Also I would add a 60 second sleep after KDC restart in 'test_upn_user_authentication' so that MS-PAC cache gets refreshed before trying to kinit as enterprise principal. Two of the tests fail on my setup but that is probably due to https://fedorahosted.org/freeipa/ticket/6082 . Hi, the "test_all_trustdomains_found" method is inherited from parent class, and I believe it cannot hurt to have it there. I added the sleep as you requested. I also tried to run the tests with two way trust and since everything was fine then, the failures you experienced are really most likely due to the oddjob issue you linked. Of course the patch still contains tests with one way trusts, so until the oddjob issue is solved, the tests will probably fail, and should be fine once the fix is provided. Fixed patch attached. Lenka From 985bfe7ff3d87f3c3a73543ea5cc625687099db7 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 1 Jul 2016 11:00:57 +0200 Subject: [PATCH 1/2] Tests: Support of UPN for trusted domains Basic set of tests to verify support of UPN functi
Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
On 07/14/2016 11:25 AM, Lenka Doudova wrote: On 07/14/2016 09:20 AM, Lenka Doudova wrote: On 07/13/2016 04:48 PM, Milan Kubík wrote: On 07/11/2016 01:34 PM, Lenka Doudova wrote: On 07/08/2016 02:24 PM, Milan Kubík wrote: On 07/01/2016 05:13 PM, Lenka Doudova wrote: On 07/01/2016 02:42 PM, Milan Kubík wrote: On 06/16/2016 03:23 PM, Lenka Doudova wrote: Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka patch 0014: In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary? Example: ipa service-mod SRV --addattr="authind=radius" Result: The way the tracker works, this adds /u'addattr="authind=radius"'/ to the list of expected results (result of /self.attrs.update(updates)/. Of course nothing like that appears anywhere, so in case there's the /--addattr/ option, it's necessary to ensure it won't get to the /self.attrs/ atribute. patch 0015: host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case? Renamed. patch 0016: Per offline discussion, admin user has no special privileges here, LGTM. -- Milan Kubik Thanks for review, fixed patches (14.2 and 15.2) attached. Lenka NACK, the update method of ServiceTracker creates the entry if it doesn't exist. Why? I know the base class has this problem also [1], though. Given this will be addressed, the fixtures in the xmlrpc test will fail since the fixture scope is wrong - function instead of class. [1]: https://fedorahosted.org/freeipa/ticket/6045 -- Milan Kubik Hi, fixed patch attached. I chose to leave the scope as it was (in case one test breaks and entry is not even created, the other tests can still be successful), but I removed the creation command from ServiceTracker update method and called it directly in the tests, so there shouldn't be hidden actions. Lenka Thanks for the updated patches. There are still some issues I haven't noticed before: service tracker: track_create method doesn't set self.exists flag which is needed In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' to set the indicator, why? -- Milan Kubik Hi, fixes for comments in attached patches. After offline discussion, I removed the usage of "addattr" and replaced it with test to add two indicators in one command. Lenka One more problem was pointed out to me offline, attaching changed patch 0014. Lenka Resending the complete patch set. L. From aaf2afa849b3156603e057d99fe7f31247b08725 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 15 Jun 2016 13:40:00 +0200 Subject: [PATCH] Tests: Tracker class for services Provides basic service tracker, so far for purposes of [1]. Tracker is not complete, some methods will need to be added in case of service test refactoring. [1] https://fedorahosted.org/freeipa/ticket/433 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 152 + 1 file changed, 152 insertions(+) create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py new file mode 100644 index ..89c27e82161b70a77ac4e45ffd8b60d3da657163 --- /dev/null +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -0,0 +1,152 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import six + +from ipalib import api +from ipatests.test_xmlrpc.tracker.base import Tracker +from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid +from ipatests.test_xmlrpc import objectclasses +from ipatests.util import assert_deepequal +from ipapython.dn import DN + +if six.PY3: +unicode = str + + +class ServiceTracker(Tracker): +""" +Tracker class for service plugin + +So far does not include methods for these commands: +service-add-host +service-remove-host +service-allow-retrieve-keytab +service-disallow-retrieve-keytab +service-allow-create-keytab +service-disallow-create-keytab +service-disable +service-add-cert +service-remove-cert +""" + +retrieve_keys = { +u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab', +u'ipakrbauthzdata', u'ipaallowedtoperform'
Re: [Freeipa-devel] [PATCH 0025][Tests] RFE: External trust
On 07/13/2016 05:40 PM, Martin Babinsky wrote: On 07/01/2016 04:15 PM, Lenka Doudova wrote: On 07/01/2016 02:38 PM, Martin Babinsky wrote: On 07/01/2016 06:36 AM, Lenka Doudova wrote: On 06/30/2016 05:01 PM, Martin Babinsky wrote: On 06/30/2016 03:47 PM, Lenka Doudova wrote: Hi, attaching patch with some basic coverage for external trust feature. Bit more detailed info in commit message. Since the feature requires me to run commands previously used only for forest root domains even for subdomains, I made some changes in ipatests/test_integration/tasks.py file, so that it would enable me to reuse existing function without copy-pasting them for one variable change. Lenka Hi Lenka, I have a few comments: 1.) -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False): This modification seems extremely kludgy to me. I would rather change the function signature to: -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad_domain, extra_args=()): and pass the domain name itself to the function instead of doing magic inside. The same goes with `remove trust_with_ad`. You may want to fix the calls to them in existing code so that the domain is passed instead of the whole trust object. 2.) +def sync_time_hostname(host, srv_hostname): +""" +Syncs time with the remote server given by its hostname. +Please note that this function leaves ntpd stopped. +""" +host.run_command(['systemctl', 'stop', 'ntpd']) +host.run_command(['ntpdate', srv_hostname]) + + this looks like a duplicate of the function above it, why is it even needed? 3.) +class TestExternalTrustWithSubdomain(ADTrustBase): +""" +Test establishing external trust with subdomain +""" + +@classmethod +def install(cls, mh): +super(ADTrustBase, cls).install(mh) +cls.ad = cls.ad_domains[0].ads[0] +cls.install_adtrust() +cls.check_sid_generation() + +# Determine whether the subdomain AD is available +try: +child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +cls.ad_subdomain = None + +cls.configure_dns_and_time() Please use proper OOP practices when overriding the behavior of the base test class. For starters you could rewrite the install method like this: +@classmethod +def install(cls, mh): +# Determine whether the subdomain AD is available +try: +cls.child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +raise nose.SkipTest("AFAIK this will skip the whole test class") +super(ADTrustBase, cls).install(mh) with child_ad stored as class attribute, you can override `configure_dns_and_time`: +def configure_dns_and_time(cls): +tasks.configure_dns_for_trust(cls.master, cls.ad_subdomain) # no need to re-implement the function just to get to the child AD DC hostname now +tasks.sync_time(cls.master, cls.child_ad.hostname) You can then use this class as an intermediary in the hierarchy and inherit the other external/non-external trust test classes from it, since most setup method in them are just copy-pasted from this one. Hi, thanks for review, fixed patch attached. Lenka Hi Lenka, I am still not happy about the patch underutilizing the potential for a proper inheritance hierarchy and instead relying on a staggering amounts of copy-pasting for implementation. I am attaching a quick untested patch illustrating how the implementation should look like. Also you have some leftovers from previous revision, please fix them: Pylint is running, please wait ... * Module ipatests.test_integration.test_trust ipatests/test_integration/test_trust.py:236: [E1101(no-member), TestExternalTrustWithSubdomain.configure_dns_and_time] Module 'ipatests.test_integration.tasks' has no 'sync_time_hostname' member) ipatests/test_integration/test_trust.py:243: [E1123(unexpected-keyword-arg), TestExternalTrustWithSubdomain.test_establish_trust] Unexpected keyword argument 'subdomain' in function call) ipatests/test_integration/test_trust.py:279: [E1123(unexpected-keyword-arg), TestExternalTrustWithSubdomain.test_remove_nonposix_trust] Unexpected keyword argument 'subdomain' in function call) ipatests/test_integration/test_trust.py:330: [E1101(no-member), TestNonexternalTrustWithSubdomain.configure_dns_and_tim
Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
On 07/14/2016 09:20 AM, Lenka Doudova wrote: On 07/13/2016 04:48 PM, Milan Kubík wrote: On 07/11/2016 01:34 PM, Lenka Doudova wrote: On 07/08/2016 02:24 PM, Milan Kubík wrote: On 07/01/2016 05:13 PM, Lenka Doudova wrote: On 07/01/2016 02:42 PM, Milan Kubík wrote: On 06/16/2016 03:23 PM, Lenka Doudova wrote: Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka patch 0014: In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary? Example: ipa service-mod SRV --addattr="authind=radius" Result: The way the tracker works, this adds /u'addattr="authind=radius"'/ to the list of expected results (result of /self.attrs.update(updates)/. Of course nothing like that appears anywhere, so in case there's the /--addattr/ option, it's necessary to ensure it won't get to the /self.attrs/ atribute. patch 0015: host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case? Renamed. patch 0016: Per offline discussion, admin user has no special privileges here, LGTM. -- Milan Kubik Thanks for review, fixed patches (14.2 and 15.2) attached. Lenka NACK, the update method of ServiceTracker creates the entry if it doesn't exist. Why? I know the base class has this problem also [1], though. Given this will be addressed, the fixtures in the xmlrpc test will fail since the fixture scope is wrong - function instead of class. [1]: https://fedorahosted.org/freeipa/ticket/6045 -- Milan Kubik Hi, fixed patch attached. I chose to leave the scope as it was (in case one test breaks and entry is not even created, the other tests can still be successful), but I removed the creation command from ServiceTracker update method and called it directly in the tests, so there shouldn't be hidden actions. Lenka Thanks for the updated patches. There are still some issues I haven't noticed before: service tracker: track_create method doesn't set self.exists flag which is needed In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' to set the indicator, why? -- Milan Kubik Hi, fixes for comments in attached patches. After offline discussion, I removed the usage of "addattr" and replaced it with test to add two indicators in one command. Lenka One more problem was pointed out to me offline, attaching changed patch 0014. Lenka From aaf2afa849b3156603e057d99fe7f31247b08725 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 15 Jun 2016 13:40:00 +0200 Subject: [PATCH] Tests: Tracker class for services Provides basic service tracker, so far for purposes of [1]. Tracker is not complete, some methods will need to be added in case of service test refactoring. [1] https://fedorahosted.org/freeipa/ticket/433 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 152 + 1 file changed, 152 insertions(+) create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py new file mode 100644 index ..89c27e82161b70a77ac4e45ffd8b60d3da657163 --- /dev/null +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -0,0 +1,152 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import six + +from ipalib import api +from ipatests.test_xmlrpc.tracker.base import Tracker +from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid +from ipatests.test_xmlrpc import objectclasses +from ipatests.util import assert_deepequal +from ipapython.dn import DN + +if six.PY3: +unicode = str + + +class ServiceTracker(Tracker): +""" +Tracker class for service plugin + +So far does not include methods for these commands: +service-add-host +service-remove-host +service-allow-retrieve-keytab +service-disallow-retrieve-keytab +service-allow-create-keytab +service-disallow-create-keytab +service-disable +service-add-cert +service-remove-cert +""" + +retrieve_keys = { +u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab', +u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject', +u'managedby', u'serial_nu
Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
On 07/13/2016 04:48 PM, Milan Kubík wrote: On 07/11/2016 01:34 PM, Lenka Doudova wrote: On 07/08/2016 02:24 PM, Milan Kubík wrote: On 07/01/2016 05:13 PM, Lenka Doudova wrote: On 07/01/2016 02:42 PM, Milan Kubík wrote: On 06/16/2016 03:23 PM, Lenka Doudova wrote: Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka patch 0014: In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary? Example: ipa service-mod SRV --addattr="authind=radius" Result: The way the tracker works, this adds /u'addattr="authind=radius"'/ to the list of expected results (result of /self.attrs.update(updates)/. Of course nothing like that appears anywhere, so in case there's the /--addattr/ option, it's necessary to ensure it won't get to the /self.attrs/ atribute. patch 0015: host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case? Renamed. patch 0016: Per offline discussion, admin user has no special privileges here, LGTM. -- Milan Kubik Thanks for review, fixed patches (14.2 and 15.2) attached. Lenka NACK, the update method of ServiceTracker creates the entry if it doesn't exist. Why? I know the base class has this problem also [1], though. Given this will be addressed, the fixtures in the xmlrpc test will fail since the fixture scope is wrong - function instead of class. [1]: https://fedorahosted.org/freeipa/ticket/6045 -- Milan Kubik Hi, fixed patch attached. I chose to leave the scope as it was (in case one test breaks and entry is not even created, the other tests can still be successful), but I removed the creation command from ServiceTracker update method and called it directly in the tests, so there shouldn't be hidden actions. Lenka Thanks for the updated patches. There are still some issues I haven't noticed before: service tracker: track_create method doesn't set self.exists flag which is needed In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' to set the indicator, why? -- Milan Kubik Hi, fixes for comments in attached patches. After offline discussion, I removed the usage of "addattr" and replaced it with test to add two indicators in one command. Lenka From 1e69ce92786668d80507a02b03a3fae40d7bbbd2 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 15 Jun 2016 13:40:00 +0200 Subject: [PATCH 1/2] Tests: Tracker class for services Provides basic service tracker, so far for purposes of [1]. Tracker is not complete, some methods will need to be added in case of service test refactoring. [1] https://fedorahosted.org/freeipa/ticket/433 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 173 + 1 file changed, 173 insertions(+) create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py new file mode 100644 index ..a7eb4303241e136ec88c51642f51fe703b16de7e --- /dev/null +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -0,0 +1,173 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import six + +from ipalib import api +from ipatests.test_xmlrpc.tracker.base import Tracker +from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid +from ipatests.test_xmlrpc import objectclasses +from ipatests.util import assert_deepequal +from ipapython.dn import DN + +if six.PY3: +unicode = str + + +class ServiceTracker(Tracker): +""" +Tracker class for service plugin + +So far does not include methods for these commands: +service-add-host +service-remove-host +service-allow-retrieve-keytab +service-disallow-retrieve-keytab +service-allow-create-keytab +service-disallow-create-keytab +service-disable +service-add-cert +service-remove-cert +""" + +retrieve_keys = { +u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab', +u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject', +u'managedby', u'serial_number', u'serial_number_hex', u'issuer', +u'valid_not_before', u'valid_not_after', u
[Freeipa-devel] [PATCH 0027][Tests] Fix failing automember tests in 4.3
Hi, providing patch to fix two failing automember tests in 4.3 branch. The reason of the failure was the output normalization (specifically manager attribute for user). The patch is intended for ipa-4-3 branch only. Lenka From 5c03137b1ea0adf756c05438c8dcceae98f0748a Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 13 Jul 2016 15:14:11 +0200 Subject: [PATCH] Tests: Fix failing automember tests Two tests in xmlrpc/automember suite were failing as a result of manager data normalization in user attributes. Tests are fixed to reflect the change. --- ipatests/test_xmlrpc/test_automember_plugin.py | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py index be0f7390565ed739aa66bc0c5c6d23d25d67df92..dde386286d26ddf4537fbc89647f1afecf51fcad 100644 --- a/ipatests/test_xmlrpc/test_automember_plugin.py +++ b/ipatests/test_xmlrpc/test_automember_plugin.py @@ -472,8 +472,7 @@ class test_automember(Declarative): summary=u'Added user "tuser1"', result=get_user_result( user1, u'Test', u'User1', 'add', -manager=[DN(('uid', 'mscott'), ('cn', 'users'), -('cn', 'accounts'), api.env.basedn)] +manager=[manager1] ) ), ), @@ -1394,8 +1393,7 @@ class test_automember(Declarative): result=get_user_result( user1, u'Test', u'User1', 'add', memberof_group=[group1, u'ipausers'], -manager=[DN(('uid', 'mscott'), ('cn', 'users'), -('cn', 'accounts'), api.env.basedn)] +manager=[manager1] ), ), ), -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
On 07/08/2016 02:24 PM, Milan Kubík wrote: On 07/01/2016 05:13 PM, Lenka Doudova wrote: On 07/01/2016 02:42 PM, Milan Kubík wrote: On 06/16/2016 03:23 PM, Lenka Doudova wrote: Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka patch 0014: In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary? Example: ipa service-mod SRV --addattr="authind=radius" Result: The way the tracker works, this adds /u'addattr="authind=radius"'/ to the list of expected results (result of /self.attrs.update(updates)/. Of course nothing like that appears anywhere, so in case there's the /--addattr/ option, it's necessary to ensure it won't get to the /self.attrs/ atribute. patch 0015: host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case? Renamed. patch 0016: Per offline discussion, admin user has no special privileges here, LGTM. -- Milan Kubik Thanks for review, fixed patches (14.2 and 15.2) attached. Lenka NACK, the update method of ServiceTracker creates the entry if it doesn't exist. Why? I know the base class has this problem also [1], though. Given this will be addressed, the fixtures in the xmlrpc test will fail since the fixture scope is wrong - function instead of class. [1]: https://fedorahosted.org/freeipa/ticket/6045 -- Milan Kubik Hi, fixed patch attached. I chose to leave the scope as it was (in case one test breaks and entry is not even created, the other tests can still be successful), but I removed the creation command from ServiceTracker update method and called it directly in the tests, so there shouldn't be hidden actions. Lenka From 52c8cc1f6ebe6db519bfaaf7fa3c5863d1f4d6d8 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 15 Jun 2016 13:40:00 +0200 Subject: [PATCH 1/2] Tests: Tracker class for services Provides basic service tracker, so far for purposes of [1]. Tracker is not complete, some methods will need to be added in case of service test refactoring. [1] https://fedorahosted.org/freeipa/ticket/433 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 172 + 1 file changed, 172 insertions(+) create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py new file mode 100644 index ..54c5dfcf9df009048e376ac6fb41638f9299c06c --- /dev/null +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -0,0 +1,172 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import six + +from ipalib import api +from ipatests.test_xmlrpc.tracker.base import Tracker +from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid +from ipatests.test_xmlrpc import objectclasses +from ipatests.util import assert_deepequal +from ipapython.dn import DN + +if six.PY3: +unicode = str + + +class ServiceTracker(Tracker): +""" +Tracker class for service plugin + +So far does not include methods for these commands: +service-add-host +service-remove-host +service-allow-retrieve-keytab +service-disallow-retrieve-keytab +service-allow-create-keytab +service-disallow-create-keytab +service-disable +service-add-cert +service-remove-cert +""" + +retrieve_keys = { +u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab', +u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject', +u'managedby', u'serial_number', u'serial_number_hex', u'issuer', +u'valid_not_before', u'valid_not_after', u'md5_fingerprint', +u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host', +u'krbcanonicalname'} +retrieve_all_keys = retrieve_keys | { +u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData', +u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof', +u'objectClass', u'ipakrbrequirespreauth', +u'ipakrbokasdelegate'} + +create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'
Re: [Freeipa-devel] [Testplan] Support of UPN for trusted domains
On 07/07/2016 11:13 AM, Sumit Bose wrote: On Fri, May 27, 2016 at 11:24:24AM +0300, Alexander Bokovoy wrote: On Fri, 27 May 2016, Sumit Bose wrote: On Fri, May 27, 2016 at 09:57:37AM +0200, Lenka Doudova wrote: Hi all, here [1] is a draft of test plan for V4 RFE Support of UPN for trusted domains. Please review this and let me know if there's something missing or wrong. Hi Lenka, thank you for the test plan. About the TBD, Alexander and I agreed to store the alternative domain suffixes read from AD in a new attribute in the LDAP object of the forest root of the trusted domain. About the kinit tests. Please note that it is expected that the -E option of kinit must be used when alternative suffixes are used. I'm not sure if SSSD tests are in the scope here as well. If they are I would suggest to add authentication tests with SSSD where e.g. the name with an alternative domain suffix is used as login name. This in general already works with SSSD but is disabled by default for IPA because of the missing server-side support so far. Since SSSD must be able to work with old and new IPA server https://fedorahosted.org/sssd/ticket/3018 was created so that SSSD can detect at runtime if the server supports this or not. Right, I think we should make sure SSSD is tested against IPA UPN support because otherwise we might get regressions. Hi Lenka, I would like to ask you to add test where 'kinit -E' is used with an IPA user as well to avoid regression, because currently 'kinit -E ipauser@IPA.DOMAIN' does not work. Please note that the full principal must be used with kinit in this case because when just using kinit -E ipauser kinit is smart enough to see that it makes no sense to add the default_realm twice and internally just does 'kinit ipauser@IPA.DOMAIN'. If you think this test is better suited in a different test plan please let me know, then I'll ask there. bye, Sumit Hi Sumit, this test should be covered in basic trust test suite, but I think it's not in the code of the test (I was busy with providing coverage for new features and didn't manage to go through old coverage). I'll check this and update ASAP. Thanks for catching it! Lenka -- / Alexander Bokovoy -- 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
Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
On 07/01/2016 02:42 PM, Milan Kubík wrote: On 06/16/2016 03:23 PM, Lenka Doudova wrote: Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka patch 0014: In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary? Example: ipa service-mod SRV --addattr="authind=radius" Result: The way the tracker works, this adds /u'addattr="authind=radius"'/ to the list of expected results (result of /self.attrs.update(updates)/. Of course nothing like that appears anywhere, so in case there's the /--addattr/ option, it's necessary to ensure it won't get to the /self.attrs/ atribute. patch 0015: host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case? Renamed. patch 0016: Per offline discussion, admin user has no special privileges here, LGTM. -- Milan Kubik Thanks for review, fixed patches (14.2 and 15.2) attached. Lenka From 7f8626d9b0cb25f9fe12fdb853f5f3af213ce3ad Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 15 Jun 2016 13:40:00 +0200 Subject: [PATCH] Tests: Tracker class for services Provides basic service tracker, so far for purposes of [1]. Tracker is not complete, some methods will need to be added in case of service test refactoring. [1] https://fedorahosted.org/freeipa/ticket/433 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 173 + 1 file changed, 173 insertions(+) create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py new file mode 100644 index ..f56a95fd23438108b2afd5ce3dc8db72b8ba4e95 --- /dev/null +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -0,0 +1,173 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import six + +from ipalib import api +from ipatests.test_xmlrpc.tracker.base import Tracker +from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid +from ipatests.test_xmlrpc import objectclasses +from ipatests.util import assert_deepequal +from ipapython.dn import DN + +if six.PY3: +unicode = str + + +class ServiceTracker(Tracker): +""" +Tracker class for service plugin + +So far does not include methods for these commands: +service-add-host +service-remove-host +service-allow-retrieve-keytab +service-disallow-retrieve-keytab +service-allow-create-keytab +service-disallow-create-keytab +service-disable +service-add-cert +service-remove-cert +""" + +retrieve_keys = { +u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab', +u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject', +u'managedby', u'serial_number', u'serial_number_hex', u'issuer', +u'valid_not_before', u'valid_not_after', u'md5_fingerprint', +u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host', +u'krbcanonicalname'} +retrieve_all_keys = retrieve_keys | { +u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData', +u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof', +u'objectClass', u'ipakrbrequirespreauth', +u'ipakrbokasdelegate'} + +create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - { +u'usercertificate', u'has_keytab'} +update_keys = retrieve_keys - {u'dn'} + +def __init__(self, name, host_fqdn, options): +super(ServiceTracker, self).__init__(default_version=None) +self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm) +self.dn = DN( +('krbprincipalname', self.name), api.env.container_service, +api.env.basedn) +self.host_fqdn = host_fqdn +self.options = options + +@property +def name(self): +return self._name + +def make_create_command(self, force=True): +""" Make function that creates a service """ +return self.make_command('service_
Re: [Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains
On 07/01/2016 03:04 PM, Martin Babinsky wrote: On 07/01/2016 11:13 AM, Lenka Doudova wrote: And, of course, a patch file :) On 07/01/2016 11:09 AM, Lenka Doudova wrote: Hi all, here's patch with basic test suite for support of UPN. Note: it needs to be applied on top of my patch 0025.2 (or later, if there's will be more fixes to that patch). Lenka Hi Lenka, test data such as usernames, etc. should be stored either in separate resource files or at least as class attributes like this: diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index e8fdc6b..86ba7cc 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase): """ Test support of UPN for trusted domains """ +upn_suffix = 'UPNsuffix.com' +upn_username = 'upnuser' +upn_princ = '{}@{}'.format(upn_username, upn_suffix) +upn_password = 'Secret123456' + def test_upn_in_nonposix_trust(self): """ Check that UPN is listed as trust attribute """ result = self.master.run_command(['ipa', 'trust-show', self.ad_domain, '--all', '--raw']) -assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text +assert ("ipantadditionalsuffixes: {}".format(self.upn_suffix) in +result.stdout_text) def test_upn_user_resolution_in_nonposix_trust(self): """ Check that user with UPN can be resolved """ -upnuser = 'upnu...@upnsuffix.com' -result = self.master.run_command(['getent', 'passwd', upnuser]) +result = self.master.run_command(['getent', 'passwd', self.upn_princ]) # result will contain AD domain, not UPN -upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN User:/:$".format( -self.ad_domain) +upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format( +self.upn_username, self.ad_domain) assert re.search(upnuser_regex, result.stdout_text) def test_upn_user_authentication(self): """ Check that AD user with UPN can authenticate in IPA """ self.master.run_command(['systemctl', 'restart', 'krb5kdc']) -self.master.run_command(['kinit', '-C', '-E', 'upnu...@upnsuffix.com'], -stdin_text='Secret123456') +self.master.run_command(['kinit', '-C', '-E', self.upn_princ], +stdin_text=self.upn_password) otherwise LGTM. Thanks for review, fixed patch attached. Few notes: 1. mbabinsky's suggestion to store testdata as class attributes or separate resource file: I decided to use the class attribute approach. The separate resource file is a nice idea, which I have already put on my "to do" list - there's a lot of hardcoded stuff in the trust tests, even in the original ones (before my patches), so when there's time I'll work on a way how to dynamically provide this data as test configuration 2. previous discussion about getent vs. pwd.getpwnam(): I'll leave the getent command, since according to mbasti the alternative would not work in CI. Lenka From 997ae46d6ee2ab5a147e9f57ef17778cad943cdd Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 1 Jul 2016 11:00:57 +0200 Subject: [PATCH] Tests: Support of UPN for trusted domains Basic set of tests to verify support of UPN functionality. Test cases: - establish trust - verify the trust recognizes UPN - verify AD user with UPN can be resolved - verify AD user with UPN can authenticate - remove trust https://fedorahosted.org/freeipa/ticket/5354 --- ipatests/test_integration/test_trust.py | 40 + 1 file changed, 40 insertions(+) diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index ba7ab8fdc0703369d55302ae3c20e79bd1b01daa..2507bf1747bfcdfdda4ae269ea403aad66fa903a 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -345,3 +345,43 @@ class TestExternalTrustWithRootDomain(ADTrustSubdomainBase): def test_remove_nonposix_trust(self): tasks.remove_trust_with_ad(self.master, self.ad_domain) tasks.clear_sssd_cache(self.master) + + +class TestTrustWithUPN(ADTrustBase): +""" +Test support of UPN for trusted domains +""" + +upn_suffix = 'UPNsuffix.com' +upn_username = 'upnuser'
Re: [Freeipa-devel] [PATCH 0025][Tests] RFE: External trust
On 07/01/2016 02:38 PM, Martin Babinsky wrote: On 07/01/2016 06:36 AM, Lenka Doudova wrote: On 06/30/2016 05:01 PM, Martin Babinsky wrote: On 06/30/2016 03:47 PM, Lenka Doudova wrote: Hi, attaching patch with some basic coverage for external trust feature. Bit more detailed info in commit message. Since the feature requires me to run commands previously used only for forest root domains even for subdomains, I made some changes in ipatests/test_integration/tasks.py file, so that it would enable me to reuse existing function without copy-pasting them for one variable change. Lenka Hi Lenka, I have a few comments: 1.) -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False): This modification seems extremely kludgy to me. I would rather change the function signature to: -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad_domain, extra_args=()): and pass the domain name itself to the function instead of doing magic inside. The same goes with `remove trust_with_ad`. You may want to fix the calls to them in existing code so that the domain is passed instead of the whole trust object. 2.) +def sync_time_hostname(host, srv_hostname): +""" +Syncs time with the remote server given by its hostname. +Please note that this function leaves ntpd stopped. +""" +host.run_command(['systemctl', 'stop', 'ntpd']) +host.run_command(['ntpdate', srv_hostname]) + + this looks like a duplicate of the function above it, why is it even needed? 3.) +class TestExternalTrustWithSubdomain(ADTrustBase): +""" +Test establishing external trust with subdomain +""" + +@classmethod +def install(cls, mh): +super(ADTrustBase, cls).install(mh) +cls.ad = cls.ad_domains[0].ads[0] +cls.install_adtrust() +cls.check_sid_generation() + +# Determine whether the subdomain AD is available +try: +child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +cls.ad_subdomain = None + +cls.configure_dns_and_time() Please use proper OOP practices when overriding the behavior of the base test class. For starters you could rewrite the install method like this: +@classmethod +def install(cls, mh): +# Determine whether the subdomain AD is available +try: +cls.child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +raise nose.SkipTest("AFAIK this will skip the whole test class") +super(ADTrustBase, cls).install(mh) with child_ad stored as class attribute, you can override `configure_dns_and_time`: +def configure_dns_and_time(cls): +tasks.configure_dns_for_trust(cls.master, cls.ad_subdomain) # no need to re-implement the function just to get to the child AD DC hostname now +tasks.sync_time(cls.master, cls.child_ad.hostname) You can then use this class as an intermediary in the hierarchy and inherit the other external/non-external trust test classes from it, since most setup method in them are just copy-pasted from this one. Hi, thanks for review, fixed patch attached. Lenka Hi Lenka, I am still not happy about the patch underutilizing the potential for a proper inheritance hierarchy and instead relying on a staggering amounts of copy-pasting for implementation. I am attaching a quick untested patch illustrating how the implementation should look like. Also you have some leftovers from previous revision, please fix them: Pylint is running, please wait ... * Module ipatests.test_integration.test_trust ipatests/test_integration/test_trust.py:236: [E1101(no-member), TestExternalTrustWithSubdomain.configure_dns_and_time] Module 'ipatests.test_integration.tasks' has no 'sync_time_hostname' member) ipatests/test_integration/test_trust.py:243: [E1123(unexpected-keyword-arg), TestExternalTrustWithSubdomain.test_establish_trust] Unexpected keyword argument 'subdomain' in function call) ipatests/test_integration/test_trust.py:279: [E1123(unexpected-keyword-arg), TestExternalTrustWithSubdomain.test_remove_nonposix_trust] Unexpected keyword argument 'subdomain' in function call) ipatests/test_integration/test_trust.py:330: [E1101(no-member), TestNonexternalTrustWithSubdomain.configure_dns_and_time] Module 'ipatests.test_integration.tasks' has no 'sync_time_hostn
Re: [Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains
And, of course, a patch file :) On 07/01/2016 11:09 AM, Lenka Doudova wrote: Hi all, here's patch with basic test suite for support of UPN. Note: it needs to be applied on top of my patch 0025.2 (or later, if there's will be more fixes to that patch). Lenka From 5c8cb8727322371b7246f6d939b38ac1cbd61e4c Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 1 Jul 2016 11:00:57 +0200 Subject: [PATCH] Tests: Support of UPN for trusted domains Basic set of tests to verify support of UPN functionality. Test cases: - establish trust - verify the trust recognizes UPN - verify AD user with UPN can be resolved - verify AD user with UPN can authenticate - remove trust https://fedorahosted.org/freeipa/ticket/5354 --- ipatests/test_integration/test_trust.py | 32 1 file changed, 32 insertions(+) diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index d662e80727b6eab3df93166d35ddbaea6a0f6f7a..e8fdc6ba68fb6275a0d7920c76ca434ed830ed84 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -388,3 +388,35 @@ class TestExternalTrustWithRootDomain(ADTrustBase): tasks.remove_trust_with_ad(self.master, self.ad_domain) tasks.clear_sssd_cache(self.master) + + +class TestTrustWithUPN(ADTrustBase): +""" +Test support of UPN for trusted domains +""" +def test_upn_in_nonposix_trust(self): +""" Check that UPN is listed as trust attribute """ +result = self.master.run_command(['ipa', 'trust-show', self.ad_domain, + '--all', '--raw']) + +assert "ipantadditionalsuffixes: UPNsuffix.com" in result.stdout_text + +def test_upn_user_resolution_in_nonposix_trust(self): +""" Check that user with UPN can be resolved """ +upnuser = 'upnu...@upnsuffix.com' +result = self.master.run_command(['getent', 'passwd', upnuser]) + +# result will contain AD domain, not UPN +upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN User:/:$".format( +self.ad_domain) +assert re.search(upnuser_regex, result.stdout_text) + +def test_upn_user_authentication(self): +""" Check that AD user with UPN can authenticate in IPA """ +self.master.run_command(['systemctl', 'restart', 'krb5kdc']) +self.master.run_command(['kinit', '-C', '-E', 'upnu...@upnsuffix.com'], +stdin_text='Secret123456') + +def test_remove_nonposix_trust(self): +tasks.remove_trust_with_ad(self.master, self.ad_domain) +tasks.clear_sssd_cache(self.master) -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains
Hi all, here's patch with basic test suite for support of UPN. Note: it needs to be applied on top of my patch 0025.2 (or later, if there's will be more fixes to that patch). Lenka -- 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
Re: [Freeipa-devel] [PATCH 0025][Tests] RFE: External trust
On 06/30/2016 05:01 PM, Martin Babinsky wrote: On 06/30/2016 03:47 PM, Lenka Doudova wrote: Hi, attaching patch with some basic coverage for external trust feature. Bit more detailed info in commit message. Since the feature requires me to run commands previously used only for forest root domains even for subdomains, I made some changes in ipatests/test_integration/tasks.py file, so that it would enable me to reuse existing function without copy-pasting them for one variable change. Lenka Hi Lenka, I have a few comments: 1.) -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False): This modification seems extremely kludgy to me. I would rather change the function signature to: -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad_domain, extra_args=()): and pass the domain name itself to the function instead of doing magic inside. The same goes with `remove trust_with_ad`. You may want to fix the calls to them in existing code so that the domain is passed instead of the whole trust object. 2.) +def sync_time_hostname(host, srv_hostname): +""" +Syncs time with the remote server given by its hostname. +Please note that this function leaves ntpd stopped. +""" +host.run_command(['systemctl', 'stop', 'ntpd']) +host.run_command(['ntpdate', srv_hostname]) + + this looks like a duplicate of the function above it, why is it even needed? 3.) +class TestExternalTrustWithSubdomain(ADTrustBase): +""" +Test establishing external trust with subdomain +""" + +@classmethod +def install(cls, mh): +super(ADTrustBase, cls).install(mh) +cls.ad = cls.ad_domains[0].ads[0] +cls.install_adtrust() +cls.check_sid_generation() + +# Determine whether the subdomain AD is available +try: +child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +cls.ad_subdomain = None + +cls.configure_dns_and_time() Please use proper OOP practices when overriding the behavior of the base test class. For starters you could rewrite the install method like this: +@classmethod +def install(cls, mh): +# Determine whether the subdomain AD is available +try: +cls.child_ad = cls.host_by_role(cls.optional_extra_roles[0]) +cls.ad_subdomain = '.'.join(child_ad.hostname.split('.')[1:]) +cls.ad_subdomain_hostname = child_ad.hostname +except LookupError: +raise nose.SkipTest("AFAIK this will skip the whole test class") +super(ADTrustBase, cls).install(mh) with child_ad stored as class attribute, you can override `configure_dns_and_time`: +def configure_dns_and_time(cls): +tasks.configure_dns_for_trust(cls.master, cls.ad_subdomain) # no need to re-implement the function just to get to the child AD DC hostname now +tasks.sync_time(cls.master, cls.child_ad.hostname) You can then use this class as an intermediary in the hierarchy and inherit the other external/non-external trust test classes from it, since most setup method in them are just copy-pasted from this one. Hi, thanks for review, fixed patch attached. Lenka From ae860f97d701b296f52de1eb0c84dd43d1429f76 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 30 Jun 2016 12:23:02 +0200 Subject: [PATCH] Tests: External trust Provides basic coverage for external trust feature. Test cases: 1. verify an external trust with AD subdomain can be established - verify only one trustdomain is listed - verify subdomain users are resolvable - verify trust can be deleted 2. verify non-external trust with AD subdomain cannot be established 3. verify an external trust with AD forest root domain can be established - verify that even if AD subdomain is specified, it is not associated with the trust - verify trust can be deleted https://fedorahosted.org/freeipa/ticket/5743 --- ipatests/test_integration/tasks.py | 11 +- ipatests/test_integration/test_trust.py | 218 2 files changed, 197 insertions(+), 32 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 5be7cdae3ac777bbf0fc52e6c511969e9fabcd72..48ef6f1963e2405d7db9a18799e9796b7f10bfe5 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -443,7 +443,7 @@ def configure_dns_for_trust(master, ad): pass -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad_domain, extra_arg
Re: [Freeipa-devel] [PATCH 0025][Tests] RFE: External trust
On 06/30/2016 04:12 PM, Oleg Fayans wrote: Hi Lenka, The changes in test_trust.py are fine. As for tasks.py: 1. I'd rename sync_time_hostname to just sync_time and There's already one sync_time function with different contents, it seemed nicer to create a new function than adding some "switch" into the old one, making it three time longer. 2. I would start ntpd again in the same method: it's no good to keep this thing in mind each time you call it. Leaving ntpd stopped is "legacy" from original function, and as I am not sure which purpose it serves, I decided to leave it as it is. Besides, I would split the changes into 2 patches: one for tasks.py and other for test_trust.py No problem there, + adding ticket info into commit message, as I seem to have forgot again. On 06/30/2016 03:47 PM, Lenka Doudova wrote: Hi, attaching patch with some basic coverage for external trust feature. Bit more detailed info in commit message. Since the feature requires me to run commands previously used only for forest root domains even for subdomains, I made some changes in ipatests/test_integration/tasks.py file, so that it would enable me to reuse existing function without copy-pasting them for one variable change. Lenka -- 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
[Freeipa-devel] [PATCH 0025][Tests] RFE: External trust
Hi, attaching patch with some basic coverage for external trust feature. Bit more detailed info in commit message. Since the feature requires me to run commands previously used only for forest root domains even for subdomains, I made some changes in ipatests/test_integration/tasks.py file, so that it would enable me to reuse existing function without copy-pasting them for one variable change. Lenka From 71f4720c4b8b2d6ba2f3f7efc22ad75643fe7e53 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 30 Jun 2016 12:23:02 +0200 Subject: [PATCH] Tests: External trust Provides basic coverage for external trust feature. Test cases: 1. verify an external trust with AD subdomain can be established - verify only one trustdomain is listed - verify subdomain users are resolvable - verify trust can be deleted 2. verify non-external trust with AD subdomain cannot be established 3. verify an external trust with AD forest root domain can be established - verify that even if AD subdomain is specified, it is not associated with the trust - verify trust can be deleted --- ipatests/test_integration/tasks.py | 30 - ipatests/test_integration/test_trust.py | 208 2 files changed, 233 insertions(+), 5 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 5be7cdae3ac777bbf0fc52e6c511969e9fabcd72..9277a76c03805aa4dc354c3346ed8505499e4806 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -443,7 +443,7 @@ def configure_dns_for_trust(master, ad): pass -def establish_trust_with_ad(master, ad, extra_args=()): +def establish_trust_with_ad(master, ad, extra_args=(), subdomain=False): """ Establishes trust with Active Directory. Trust type is detected depending on the presence of SfU (Services for Unix) support on the AD. @@ -461,9 +461,15 @@ def establish_trust_with_ad(master, ad, extra_args=()): kinit_admin(master) master.run_command(['klist']) master.run_command(['smbcontrol', 'all', 'debug', '100']) + +if subdomain: +ad_domain = ad +else: +ad_domain = ad.domain.name + util.run_repeatedly(master, ['ipa', 'trust-add', - '--type', 'ad', ad.domain.name, + '--type', 'ad', ad_domain, '--admin', 'Administrator', '--password'] + list(extra_args), stdin_text=master.config.ad_admin_password) @@ -471,18 +477,23 @@ def establish_trust_with_ad(master, ad, extra_args=()): clear_sssd_cache(master) -def remove_trust_with_ad(master, ad): +def remove_trust_with_ad(master, ad, subdomain=False): """ Removes trust with Active Directory. Also removes the associated ID range. """ kinit_admin(master) +if subdomain: +ad_domain = ad +else: +ad_domain = ad.domain.name + # Remove the trust -master.run_command(['ipa', 'trust-del', ad.domain.name]) +master.run_command(['ipa', 'trust-del', ad_domain]) # Remove the range -range_name = ad.domain.name.upper() + '_id_range' +range_name = ad_domain.upper() + '_id_range' master.run_command(['ipa', 'idrange-del', range_name]) @@ -610,6 +621,15 @@ def sync_time(host, server): host.run_command(['ntpdate', server.hostname]) +def sync_time_hostname(host, srv_hostname): +""" +Syncs time with the remote server given by its hostname. +Please note that this function leaves ntpd stopped. +""" +host.run_command(['systemctl', 'stop', 'ntpd']) +host.run_command(['ntpdate', srv_hostname]) + + def connect_replica(master, replica, domain_level=None): if domain_level is None: domain_level = master.config.domain_level diff --git a/ipatests/test_integration/test_trust.py b/ipatests/test_integration/test_trust.py index 772a50842f7c251b78d68fd4bd3c91668f580d50..6074217e0c738623279cc191c1794f0ce3df0a3a 100644 --- a/ipatests/test_integration/test_trust.py +++ b/ipatests/test_integration/test_trust.py @@ -23,6 +23,7 @@ import re from ipatests.test_integration.base import IntegrationTest from ipatests.test_integration import tasks from ipatests.test_integration import util +from ipaplatform.paths import paths class ADTrustBase(IntegrationTest): @@ -224,3 +225,210 @@ class TestInvalidRangeTypes(ADTrustBase): # The trust-add command is supposed to fail assert result.returncode == 1 + + +class TestExternalTrustWithSubdomain(ADTrustBase): +
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/30/2016 12:51 PM, Petr Spacek wrote: On 30.6.2016 12:32, Lenka Doudova wrote: On 06/29/2016 07:49 PM, Petr Spacek wrote: On 29.6.2016 18:52, Lenka Doudova wrote: On 06/29/2016 06:51 PM, Petr Spacek wrote: On 29.6.2016 18:48, Lenka Doudova wrote: On 06/27/2016 11:05 AM, Lenka Doudova wrote: On 06/27/2016 10:33 AM, Martin Babinsky wrote: On 06/27/2016 10:28 AM, Petr Spacek wrote: On 27.6.2016 10:26, Petr Spacek wrote: On 27.6.2016 10:18, Martin Babinsky wrote: On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Btw, this is not a NACK. Well AFAIK the exit status on POSIX systems is encoded into a single byte so you cannot have the return value greater that 255. We would have to devise some mapping between our XMLRPC status codes and subprocess return codes. Some of our exceptions have defined return values outside plain '1', e.g. NotFound has return value of 2. It would be possible to extend this concept on other common errors. Even more importantly, the forward zone is completely unnecessary because DNS when DNS is set up properly. I would simply remove the dnsforwardzone-add. Grr, I meant this: Even more importantly, the forward zone is completely unnecessary when DNS is set up properly. I would simply remove the dnsforwardzone-add. +1, our tests should not fiddle with the provisioned environment as much as they sometimes do. Well, I have nothing against removing it completely, but left it there just because with previous AD machines with "wild" domains it was necessary. Looking at the code, your suggestion would probably mean to entirely remove method configure_dns_for_trust from ipatests/test_integration/tasks.py, right? I'll have to verify this won't break anything else. Lenka Hi, to get back to this issue: do we really want to have the DNS configuration method removed? I mean, we no longer need it for our CI tests, with new AD VMs it works without it, but should somebody else with different setup run these tests, they could experience failures because their AD domain would not be configured in DNS by default and the test would no longer provide that configuration. It doesn't feel right to delete something we needed before but don't need anymore, in case somebody else is depending on the same configuration. But of course, I'll abide by your counsel. In case the call on DNS configuration method really is removed, should I remove even it's definition? It's not used anywhere else, so it would be quite logical. Feel free to keep empty method around as a "hook" for other people. The important thing is that it should do nothing by default. So leave the method call, but erase method contents and let it just pass? Fine with me. (List re-added.) Ah, sorry for doing the wrong reply. Anyway, fixed patch attached. I decided to do as you suggested - the original DNS configuring function is now empty, I modified the comment to explain significance of this strange thing. I also changed patch title to better reflect proposed changes. ACK if it passes your tests. Yes, I've had no problems running the tests since I started to use this. Thanks. -- 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
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/30/2016 12:32 PM, Lenka Doudova wrote: On 06/29/2016 07:49 PM, Petr Spacek wrote: On 29.6.2016 18:52, Lenka Doudova wrote: On 06/29/2016 06:51 PM, Petr Spacek wrote: On 29.6.2016 18:48, Lenka Doudova wrote: On 06/27/2016 11:05 AM, Lenka Doudova wrote: On 06/27/2016 10:33 AM, Martin Babinsky wrote: On 06/27/2016 10:28 AM, Petr Spacek wrote: On 27.6.2016 10:26, Petr Spacek wrote: On 27.6.2016 10:18, Martin Babinsky wrote: On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Btw, this is not a NACK. Well AFAIK the exit status on POSIX systems is encoded into a single byte so you cannot have the return value greater that 255. We would have to devise some mapping between our XMLRPC status codes and subprocess return codes. Some of our exceptions have defined return values outside plain '1', e.g. NotFound has return value of 2. It would be possible to extend this concept on other common errors. Even more importantly, the forward zone is completely unnecessary because DNS when DNS is set up properly. I would simply remove the dnsforwardzone-add. Grr, I meant this: Even more importantly, the forward zone is completely unnecessary when DNS is set up properly. I would simply remove the dnsforwardzone-add. +1, our tests should not fiddle with the provisioned environment as much as they sometimes do. Well, I have nothing against removing it completely, but left it there just because with previous AD machines with "wild" domains it was necessary. Looking at the code, your suggestion would probably mean to entirely remove method configure_dns_for_trust from ipatests/test_integration/tasks.py, right? I'll have to verify this won't break anything else. Lenka Hi, to get back to this issue: do we really want to have the DNS configuration method removed? I mean, we no longer need it for our CI tests, with new AD VMs it works without it, but should somebody else with different setup run these tests, they could experience failures because their AD domain would not be configured in DNS by default and the test would no longer provide that configuration. It doesn't feel right to delete something we needed before but don't need anymore, in case somebody else is depending on the same configuration. But of course, I'll abide by your counsel. In case the call on DNS configuration method really is removed, should I remove even it's definition? It's not used anywhere else, so it would be quite logical. Feel free to keep empty method around as a "hook" for other people. The important thing is that it should do nothing by default. So leave the method call, but erase method contents and let it just pass? Fine with me. (List re-added.) Ah, sorry for doing the wrong reply. Anyway, fixed patch attached. I decided to do as you suggested - the original DNS configuring function is now empty, I modified the comment to explain significance of this strange thing. I also changed patch title to better reflect proposed changes. Lenka NACK the previous one, forgot PEP8. New patch attached. Lenka From a4772a1b8cc31f0020c86acabbeb944c6d5269b7 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 30 Jun 2016 12:26:28 +0200 Subject: [PATCH] Tests: Remove DNS configuration from trust tests Since DNS configuration is no longer needed for running trust tests, this method's contents are removed. Method is left empty as reference for others, should they have issues with DNS configuration. --- ipatests/test_integration/tasks.py | 44 -- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 38218fa709c2c220d5fea98a092b55e995d48d77..5be7cdae3ac777bbf0fc52e6c511969e9fabcd72 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -436,47 +436,11 @@ def install_adtrust(host): def configure_dns_for_trust(master, ad):
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/29/2016 07:49 PM, Petr Spacek wrote: On 29.6.2016 18:52, Lenka Doudova wrote: On 06/29/2016 06:51 PM, Petr Spacek wrote: On 29.6.2016 18:48, Lenka Doudova wrote: On 06/27/2016 11:05 AM, Lenka Doudova wrote: On 06/27/2016 10:33 AM, Martin Babinsky wrote: On 06/27/2016 10:28 AM, Petr Spacek wrote: On 27.6.2016 10:26, Petr Spacek wrote: On 27.6.2016 10:18, Martin Babinsky wrote: On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Btw, this is not a NACK. Well AFAIK the exit status on POSIX systems is encoded into a single byte so you cannot have the return value greater that 255. We would have to devise some mapping between our XMLRPC status codes and subprocess return codes. Some of our exceptions have defined return values outside plain '1', e.g. NotFound has return value of 2. It would be possible to extend this concept on other common errors. Even more importantly, the forward zone is completely unnecessary because DNS when DNS is set up properly. I would simply remove the dnsforwardzone-add. Grr, I meant this: Even more importantly, the forward zone is completely unnecessary when DNS is set up properly. I would simply remove the dnsforwardzone-add. +1, our tests should not fiddle with the provisioned environment as much as they sometimes do. Well, I have nothing against removing it completely, but left it there just because with previous AD machines with "wild" domains it was necessary. Looking at the code, your suggestion would probably mean to entirely remove method configure_dns_for_trust from ipatests/test_integration/tasks.py, right? I'll have to verify this won't break anything else. Lenka Hi, to get back to this issue: do we really want to have the DNS configuration method removed? I mean, we no longer need it for our CI tests, with new AD VMs it works without it, but should somebody else with different setup run these tests, they could experience failures because their AD domain would not be configured in DNS by default and the test would no longer provide that configuration. It doesn't feel right to delete something we needed before but don't need anymore, in case somebody else is depending on the same configuration. But of course, I'll abide by your counsel. In case the call on DNS configuration method really is removed, should I remove even it's definition? It's not used anywhere else, so it would be quite logical. Feel free to keep empty method around as a "hook" for other people. The important thing is that it should do nothing by default. So leave the method call, but erase method contents and let it just pass? Fine with me. (List re-added.) Ah, sorry for doing the wrong reply. Anyway, fixed patch attached. I decided to do as you suggested - the original DNS configuring function is now empty, I modified the comment to explain significance of this strange thing. I also changed patch title to better reflect proposed changes. Lenka From c931a57aa4a0162b7f28d2fe0e01d43c8c95c146 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 30 Jun 2016 12:26:28 +0200 Subject: [PATCH] Tests: Remove DNS configuration from trust tests Since DNS configuration is no longer needed for running trust tests, this method's contents are removed. Method is left empty as reference for others, should they have issues with DNS configuration. --- ipatests/test_integration/tasks.py | 44 -- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 38218fa709c2c220d5fea98a092b55e995d48d77..1aa7772a1b59b4ad78bd3b3f653c8782ae6041de 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -436,47 +436,11 @@ def install_adtrust(host): def configure_dns_for_trust(master, ad): """ -This configures DNS on IPA master according to the relationship of the -IPA's and AD's domains. +This method is
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/27/2016 11:05 AM, Lenka Doudova wrote: On 06/27/2016 10:33 AM, Martin Babinsky wrote: On 06/27/2016 10:28 AM, Petr Spacek wrote: On 27.6.2016 10:26, Petr Spacek wrote: On 27.6.2016 10:18, Martin Babinsky wrote: On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Btw, this is not a NACK. Well AFAIK the exit status on POSIX systems is encoded into a single byte so you cannot have the return value greater that 255. We would have to devise some mapping between our XMLRPC status codes and subprocess return codes. Some of our exceptions have defined return values outside plain '1', e.g. NotFound has return value of 2. It would be possible to extend this concept on other common errors. Even more importantly, the forward zone is completely unnecessary because DNS when DNS is set up properly. I would simply remove the dnsforwardzone-add. Grr, I meant this: Even more importantly, the forward zone is completely unnecessary when DNS is set up properly. I would simply remove the dnsforwardzone-add. +1, our tests should not fiddle with the provisioned environment as much as they sometimes do. Well, I have nothing against removing it completely, but left it there just because with previous AD machines with "wild" domains it was necessary. Looking at the code, your suggestion would probably mean to entirely remove method configure_dns_for_trust from ipatests/test_integration/tasks.py, right? I'll have to verify this won't break anything else. Lenka Hi, to get back to this issue: do we really want to have the DNS configuration method removed? I mean, we no longer need it for our CI tests, with new AD VMs it works without it, but should somebody else with different setup run these tests, they could experience failures because their AD domain would not be configured in DNS by default and the test would no longer provide that configuration. It doesn't feel right to delete something we needed before but don't need anymore, in case somebody else is depending on the same configuration. But of course, I'll abide by your counsel. In case the call on DNS configuration method really is removed, should I remove even it's definition? It's not used anywhere else, so it would be quite logical. Thanks, Lenka -- 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
[Freeipa-devel] [PATCH 0024][Tests] Fix integration tests not to produce incorrect /etc/hosts file
Hi all, a function 'fix_etc_hosts' in ipatests/test_integration/tasks.py produces incorrect /etc/hosts file (solitary IPv6 address), and currently parser is not able to resolve the issue, causing ipa-server-install to fail with 'list index out of range' error. Hence I'm attaching patch to fix this issue before parser is fixed (related ticket to it #6014). The fix is just change of regexs responsible for creating incorrect file so that all the lines containing defined hostname are removed, not just specific IP/hostname/shortname strings. Lenka From a2be2991041725d03d53d1cff04fc45290acd092 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 29 Jun 2016 18:13:23 +0200 Subject: [PATCH] Tests: Fix integration tests not to produce incorrect /etc/hosts file Function 'fix_etc_hosts' in ipatests/test_integration/tasks.py produces incorrectly formatted /etc/hosts file (solitary IPv6 address), which causes ipa-server-install failure. This provides fix of the function so that the regex changing /etc/hosts file remove entire lines with hostname entry in them. --- ipatests/test_integration/tasks.py | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 38218fa709c2c220d5fea98a092b55e995d48d77..61f772638b319a74e935d4962b5921e451e9bb27 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -124,11 +124,7 @@ def fix_etc_hosts(host): # Remove existing mentions of the host's FQDN, short name, and IP # Removing of IP must be done as first, otherwise hosts file may be # corrupted -contents = re.sub('^%s.*' % re.escape(host.ip), '', contents, - flags=re.MULTILINE) -contents = re.sub('\s%s(\s|$)' % re.escape(host.hostname), ' ', contents, - flags=re.MULTILINE) -contents = re.sub('\s%s(\s|$)' % re.escape(host.shortname), ' ', contents, +contents = re.sub('^.*%s.*$' % re.escape(host.hostname), '', contents, flags=re.MULTILINE) # Add the host's info again contents += '\n%s %s %s\n' % (host.ip, host.hostname, host.shortname) -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0023][Tests] Fix frontend tests - #5987
Hi, I've made patch to fix for https://fedorahosted.org/freeipa/ticket/5987. Please note, that this patch must be applied on top on my patch no. 0018, which provides other fixes on the same file (and same test). Lenka From ef04d8d013643fd5d2a0a7de32ab23686e46531d Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 28 Jun 2016 06:27:41 +0200 Subject: [PATCH] Tests: Fix frontend tests Test ipatests/test_ipalib/test_frontend.py::test_Command::test_validate fails due to attributes that are no longer present, therefore assertion for these values was removed. https://fedorahosted.org/freeipa/ticket/5987 --- ipatests/test_ipalib/test_frontend.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py index 0a5951159978706a2265bb688f08deca85de4c0d..c3dd9104c152dc9e9da774a53e0c6a42b9a89bf8 100644 --- a/ipatests/test_ipalib/test_frontend.py +++ b/ipatests/test_ipalib/test_frontend.py @@ -493,10 +493,7 @@ class test_Command(ClassChecker): fail['option0'] = u'whatever' e = raises(errors.ValidationError, sub.validate, **fail) assert_equal(e.name, u'option0') -assert_equal(e.value, u'whatever') assert_equal(e.error, u"must equal 'option0'") -assert e.rule.__class__.__name__ == 'Rule' -assert e.index is None # Check with a missing required arg fail = dict(okay) -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0167] test_serverroles: ensure that test API is initialized with correct ldap_uri
On 06/27/2016 02:04 PM, Martin Babinsky wrote: Makes the test suite play nice with others during CI. https://fedorahosted.org/freeipa/ticket/6000 ACK, thank you! Lenka -- 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
Re: [Freeipa-devel] [PATCH 0020][Tests] Make ID views test reflect new krbcanonicalname attribute
On 06/27/2016 10:26 AM, Martin Babinsky wrote: On 06/23/2016 03:51 PM, Lenka Doudova wrote: Patch attached. Lenka Thanks for catching this. conditional ACK if you add https://fedorahosted.org/freeipa/ticket/3864 to the commit message. Ah, yes. New patch with fixed commit message attached. Thanks, Lenka From a85b2cc58a90647440bebfd81bec3dad183d9c71 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 23 Jun 2016 15:47:03 +0200 Subject: [PATCH] Tests: Make ID views tests reflect new krbcanonicalname attribute https://fedorahosted.org/freeipa/ticket/3864 --- ipatests/test_xmlrpc/test_idviews_plugin.py | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ipatests/test_xmlrpc/test_idviews_plugin.py b/ipatests/test_xmlrpc/test_idviews_plugin.py index 205b0fc0e930f5db1cd2dc464cb24b9a1e1b13a5..9cd44fe2f8263c14015001af6a79e10ff9801903 100644 --- a/ipatests/test_xmlrpc/test_idviews_plugin.py +++ b/ipatests/test_xmlrpc/test_idviews_plugin.py @@ -1016,6 +1016,7 @@ class test_idviews(Declarative): description=[u'Test host 3'], l=[u'Undisclosed location 3'], krbprincipalname=[get_host_principal(host3)], +krbcanonicalname=[get_host_principal(host3)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host3)], @@ -1043,6 +1044,7 @@ class test_idviews(Declarative): description=[u'Test host 2'], l=[u'Undisclosed location 2'], krbprincipalname=[get_host_principal(host2)], +krbcanonicalname=[get_host_principal(host2)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host2)], @@ -1094,6 +1096,7 @@ class test_idviews(Declarative): description=[u'Test host 2'], l=[u'Undisclosed location 2'], krbprincipalname=[get_host_principal(host2)], +krbcanonicalname=[get_host_principal(host2)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host2)], @@ -1123,6 +1126,7 @@ class test_idviews(Declarative): description=[u'Test host 1'], l=[u'Undisclosed location 1'], krbprincipalname=[get_host_principal(host1)], +krbcanonicalname=[get_host_principal(host1)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host1)], @@ -1195,6 +1199,7 @@ class test_idviews(Declarative): description=[u'Test host 1'], l=[u'Undisclosed location 1'], krbprincipalname=[get_host_principal(host1)], +krbcanonicalname=[get_host_principal(host1)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host1)], @@ -1222,6 +1227,7 @@ class test_idviews(Declarative): description=[u'Test host 3'], l=[u'Undisclosed location 3'], krbprincipalname=[get_host_principal(host3)], +krbcanonicalname=[get_host_principal(host3)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host3)], @@ -1473,6 +1479,7 @@ class test_idviews(Declarative): description=[u'Test host 4'], l=[u'Undisclosed location 4'], krbprincipalname=[get_host_principal(host4)], +krbcanonicalname=[get_host_principal(host4)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host4)], -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/27/2016 10:33 AM, Martin Babinsky wrote: On 06/27/2016 10:28 AM, Petr Spacek wrote: On 27.6.2016 10:26, Petr Spacek wrote: On 27.6.2016 10:18, Martin Babinsky wrote: On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Btw, this is not a NACK. Well AFAIK the exit status on POSIX systems is encoded into a single byte so you cannot have the return value greater that 255. We would have to devise some mapping between our XMLRPC status codes and subprocess return codes. Some of our exceptions have defined return values outside plain '1', e.g. NotFound has return value of 2. It would be possible to extend this concept on other common errors. Even more importantly, the forward zone is completely unnecessary because DNS when DNS is set up properly. I would simply remove the dnsforwardzone-add. Grr, I meant this: Even more importantly, the forward zone is completely unnecessary when DNS is set up properly. I would simply remove the dnsforwardzone-add. +1, our tests should not fiddle with the provisioned environment as much as they sometimes do. Well, I have nothing against removing it completely, but left it there just because with previous AD machines with "wild" domains it was necessary. Looking at the code, your suggestion would probably mean to entirely remove method configure_dns_for_trust from ipatests/test_integration/tasks.py, right? I'll have to verify this won't break anything else. Lenka -- 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
Re: [Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
On 06/27/2016 10:04 AM, Petr Vobornik wrote: On 06/27/2016 09:42 AM, Lenka Doudova wrote: Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka Current approach expects that every error of ipa dnsforward-add here will mean that the zone exists. So it might hide other issues - not very good. If I understand your comment correctly, you think that the patch would pass ANY dnsforwardzone-add error and being OK and continue, right? That's not intended behaviour - I have an assertion there that checks that it's really the 'correct' error: assert "already exists in DNS" in result.stderr_text So any other error should still prevent continuing in tests. On the other hand it is not very robust to parse error message. Question for general audience: What do you think if IPA client's exit status would be the IPA error code instead of "1" for every error. E.g. in DuplicateEntry case it's 4002. Personally I think it would be nice to have DuplicateEntry error rather the just "1" in this case. Even for testing purposes I believe it would be better than bunch of asserts. Btw, this is not a NACK. -- 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
[Freeipa-devel] [PATCH 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone
Hi! With newly created AD machines in Brno lab, existing trust tests fail on 'ipa dnsforwardzone-add' command claiming the zone is already present, as new AD domain is dom-221.idm.lab.eng.brq.redhat.com. To prevent these failures I prepared attached patch, that will still attempt to add the forward zone, but in case of non-zero return code will check the message if it says that the forward zone is already configured, and lets the tests continue, if it is so. Lenka From 33761de592e867d63665ebe974dbec7c29294367 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 27 Jun 2016 09:29:31 +0200 Subject: [PATCH] Tests: Prevent trust test failures caused by adding duplicate DNS forward zone When a DNS forward zone is already configured, the 'ipa dnsforwardzone-add' command fails and prevents running trust test. New check is added that will allow tests to continue even if the command raises error caused by attempt to add already existing zone. --- ipatests/test_integration/tasks.py | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 38218fa709c2c220d5fea98a092b55e995d48d77..662fa3b73b9d1bea597199cad4d438e1ab339b01 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -473,11 +473,13 @@ def configure_dns_for_trust(master, ad): master.run_command(['ipa', 'dnszone-mod', master.domain.name, '--allow-transfer', ad.ip]) else: -master.run_command(['ipa', 'dnsforwardzone-add', ad.domain.name, -'--forwarder', ad.ip, -'--forward-policy', 'only', -]) - +result = master.run_command(['ipa', 'dnsforwardzone-add', + ad.domain.name, + '--forwarder', ad.ip, + '--forward-policy', 'only' + ], raiseonerr=False) +if result.returncode != 0: +assert "already exists in DNS" in result.stderr_text def establish_trust_with_ad(master, ad, extra_args=()): """ -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0021][Tests] Fix failing ipatests/test_ipaserver/test_rpcserver.py
Hi, attaching patch for one of the failing tests. Failure caused by an assertion that was no longer valid. Lenka From 9eb1ec2335145916ce0e83a6f2a9ca6bac056682 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 24 Jun 2016 10:24:40 +0200 Subject: [PATCH] Tests: Fix ipatests/test_ipaserver/test_rpcserver.py Removed no longer valid assert. --- ipatests/test_ipaserver/test_rpcserver.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ipatests/test_ipaserver/test_rpcserver.py b/ipatests/test_ipaserver/test_rpcserver.py index 94ebd062c91aece0558bac4c8bdf1fb01448f605..b739de0b94e0c850404347a0de227f19426a30c2 100644 --- a/ipatests/test_ipaserver/test_rpcserver.py +++ b/ipatests/test_ipaserver/test_rpcserver.py @@ -212,7 +212,6 @@ class test_jsonserver(PluginTester): # Test with invalid JSON-data: e = raises(errors.JSONError, o.unmarshal, 'this wont work') -assert isinstance(e.error, ValueError) if six.PY2: assert unicode(e.error) == 'No JSON object could be decoded' else: -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0020][Tests] Make ID views test reflect new krbcanonicalname attribute
Patch attached. Lenka From 3a3adaaf9c2f778ad82d0138f05455b488eb870a Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 23 Jun 2016 15:47:03 +0200 Subject: [PATCH] Tests: Make ID views tests reflect new krbcanonicalname attribute --- ipatests/test_xmlrpc/test_idviews_plugin.py | 7 +++ 1 file changed, 7 insertions(+) diff --git a/ipatests/test_xmlrpc/test_idviews_plugin.py b/ipatests/test_xmlrpc/test_idviews_plugin.py index 205b0fc0e930f5db1cd2dc464cb24b9a1e1b13a5..9cd44fe2f8263c14015001af6a79e10ff9801903 100644 --- a/ipatests/test_xmlrpc/test_idviews_plugin.py +++ b/ipatests/test_xmlrpc/test_idviews_plugin.py @@ -1016,6 +1016,7 @@ class test_idviews(Declarative): description=[u'Test host 3'], l=[u'Undisclosed location 3'], krbprincipalname=[get_host_principal(host3)], +krbcanonicalname=[get_host_principal(host3)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host3)], @@ -1043,6 +1044,7 @@ class test_idviews(Declarative): description=[u'Test host 2'], l=[u'Undisclosed location 2'], krbprincipalname=[get_host_principal(host2)], +krbcanonicalname=[get_host_principal(host2)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host2)], @@ -1094,6 +1096,7 @@ class test_idviews(Declarative): description=[u'Test host 2'], l=[u'Undisclosed location 2'], krbprincipalname=[get_host_principal(host2)], +krbcanonicalname=[get_host_principal(host2)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host2)], @@ -1123,6 +1126,7 @@ class test_idviews(Declarative): description=[u'Test host 1'], l=[u'Undisclosed location 1'], krbprincipalname=[get_host_principal(host1)], +krbcanonicalname=[get_host_principal(host1)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host1)], @@ -1195,6 +1199,7 @@ class test_idviews(Declarative): description=[u'Test host 1'], l=[u'Undisclosed location 1'], krbprincipalname=[get_host_principal(host1)], +krbcanonicalname=[get_host_principal(host1)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host1)], @@ -1222,6 +1227,7 @@ class test_idviews(Declarative): description=[u'Test host 3'], l=[u'Undisclosed location 3'], krbprincipalname=[get_host_principal(host3)], +krbcanonicalname=[get_host_principal(host3)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host3)], @@ -1473,6 +1479,7 @@ class test_idviews(Declarative): description=[u'Test host 4'], l=[u'Undisclosed location 4'], krbprincipalname=[get_host_principal(host4)], +krbcanonicalname=[get_host_principal(host4)], has_keytab=False, has_password=False, managedby_host=[get_fqdn(host4)], -- 2.7.4 -- 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
Re: [Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests
On 06/23/2016 10:30 AM, Martin Basti wrote: On 23.06.2016 06:55, Lenka Doudova wrote: On 06/22/2016 05:11 PM, Lenka Doudova wrote: On 06/22/2016 04:37 PM, Lenka Doudova wrote: On 06/22/2016 08:33 AM, Martin Basti wrote: On 22.06.2016 07:37, Lenka Doudova wrote: On 06/21/2016 06:57 PM, Martin Basti wrote: On 21.06.2016 15:39, Lenka Doudova wrote: Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka Hello, 1) + expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, This depends on services installed on server, so server without DNS will cause test failures. We probably should skip test id DNS isn't installed. Without DNS installed you get much more different warnings 2) +def update(self, updates, expected_updates=None, messages=None): +self.messages = messages Why is this needed? I'm puzzled by this It is defined outside __init__ what is wrong and it is never used. Hi, thanks for review. ad 1: will fix ad 2: the 'messages' key is indeed used, because this key is returned every time a server is attached to/removed from a location. If the 'messages' is not supplied to the result comparison, the test fails (see https://paste.fedoraproject.org/382936/65732411/ for result of test without applied patch). Lenka Please point me to the line where ServerTracker.messages is used, I still don't see it. In your fpaste, removed self.messages in that context is TestLocationsServer.messages not ServerTracker.messages Martin^2 Ah, right you are, my good sir. I fixed both issues, hopefully will be fine now. Fixed patch attached. Lenka Self NACK, will provide some more changes tomorrow. Lenka And here it goes... Lenka This is really weird fuzzy_servrole = Fuzzy( type=list, test=lambda other: False not in set( item in (u'CA server', u'DNS server', u'NTP server') for item in other) and u'DNS server' in other) 1) Why is there special case 'DNS server' in other, IMO general fuzzy object should not enforce DNS server role 2) Several servroles related to AD trust are missing there, so if you are doing general fuzzy object for server roles it should contains all roles 3) I suggest something like lambda other: all(item in {u'CA server', u'DNS server', u'NTP server', ...} for item in other) 4) during yesterday's interactive review, I suggested to use just lambda other: True, as serveroles are tested in different test suite, but If you fix issues above, it can stay as it is Martin^2 Fixed according to suggestion from interactiove review. Attached. Lenka From e5e795c1fd1c589c505c3312788ef5663c67af51 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 21 Jun 2016 15:29:46 +0200 Subject: [PATCH] Tests: Fix for failing location tests --- ipatests/test_xmlrpc/test_location_plugin.py| 54 - ipatests/test_xmlrpc/tracker/location_plugin.py | 10 - ipatests/test_xmlrpc/tracker/server_plugin.py | 49 +++--- 3 files changed, 87 insertions(+), 26 deletions(-) diff --git a/ipatests/test_xmlrpc/test_location_plugin.py b/ipatests/test_xmlrpc/test_location_plugin.py index 3f0edfbcf791feea1a9e1b584386215d84b44606..a7d8098c9bcccd13c99e549591563c86b5c59848 100644 --- a/ipatests/test_xmlrpc/test_location_plugin.py +++ b/ipatests/test_xmlrpc/test_location_plugin.py @@ -10,7 +10,7 @@ from ipatests.test_xmlrpc.tracker.location_plugin import LocationTracker from ipatests.test_xmlrpc.tracker.server_plugin import ServerTracker from ipatests.test_xmlrpc.xmlrpc_test import ( XMLRPC_test, -raises_exact, +raises_exact ) from ipapython.dnsutil import DNSName @@ -30,13 +30,13 @@ def location_invalid(request): @pytest.fixture(scope='class') def location_absolute(request): tracker = LocationTracker(u'invalid.absolute.') -return tracker.make_fixture(request) +return tracker @pytest.fixture(scope='class') def server(request): tracker = ServerTracker(api.env.host) -return tracker +return tracker.make_fixture_clean_location(request) @pytest.mark.tier1 @@ -122,7 +122,18 @@ class TestCRUD(XMLRPC_test): @pytest.mark.tier1 +@pytest.mark.skipif( +not api.Command.dns_is_enabled()['result'], reason='DNS not configured') class TestLocationsServer(XMLRPC_test): +messages = [{ +u'data': {u'service': u'named-pkcs11.service', + u'server': u'%s' % api.env.host}, +u'message': (u'Service named-pkcs11.service requires restar
Re: [Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests
On 06/22/2016 05:11 PM, Lenka Doudova wrote: On 06/22/2016 04:37 PM, Lenka Doudova wrote: On 06/22/2016 08:33 AM, Martin Basti wrote: On 22.06.2016 07:37, Lenka Doudova wrote: On 06/21/2016 06:57 PM, Martin Basti wrote: On 21.06.2016 15:39, Lenka Doudova wrote: Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka Hello, 1) +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, This depends on services installed on server, so server without DNS will cause test failures. We probably should skip test id DNS isn't installed. Without DNS installed you get much more different warnings 2) +def update(self, updates, expected_updates=None, messages=None): +self.messages = messages Why is this needed? I'm puzzled by this It is defined outside __init__ what is wrong and it is never used. Hi, thanks for review. ad 1: will fix ad 2: the 'messages' key is indeed used, because this key is returned every time a server is attached to/removed from a location. If the 'messages' is not supplied to the result comparison, the test fails (see https://paste.fedoraproject.org/382936/65732411/ for result of test without applied patch). Lenka Please point me to the line where ServerTracker.messages is used, I still don't see it. In your fpaste, removed self.messages in that context is TestLocationsServer.messages not ServerTracker.messages Martin^2 Ah, right you are, my good sir. I fixed both issues, hopefully will be fine now. Fixed patch attached. Lenka Self NACK, will provide some more changes tomorrow. Lenka And here it goes... Lenka From 39a60e343ac762a3f4b5d8ea88b6cac174cfc54f Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 21 Jun 2016 15:29:46 +0200 Subject: [PATCH] Tests: Fix for failing location tests --- ipatests/test_xmlrpc/test_location_plugin.py| 52 - ipatests/test_xmlrpc/tracker/location_plugin.py | 11 +- ipatests/test_xmlrpc/tracker/server_plugin.py | 49 --- ipatests/test_xmlrpc/xmlrpc_test.py | 6 +++ 4 files changed, 93 insertions(+), 25 deletions(-) diff --git a/ipatests/test_xmlrpc/test_location_plugin.py b/ipatests/test_xmlrpc/test_location_plugin.py index 3f0edfbcf791feea1a9e1b584386215d84b44606..e5b6bd9e9c6921ad3b3d3e6ddde9940cedc5c107 100644 --- a/ipatests/test_xmlrpc/test_location_plugin.py +++ b/ipatests/test_xmlrpc/test_location_plugin.py @@ -11,6 +11,7 @@ from ipatests.test_xmlrpc.tracker.server_plugin import ServerTracker from ipatests.test_xmlrpc.xmlrpc_test import ( XMLRPC_test, raises_exact, +fuzzy_servrole ) from ipapython.dnsutil import DNSName @@ -30,13 +31,13 @@ def location_invalid(request): @pytest.fixture(scope='class') def location_absolute(request): tracker = LocationTracker(u'invalid.absolute.') -return tracker.make_fixture(request) +return tracker @pytest.fixture(scope='class') def server(request): tracker = ServerTracker(api.env.host) -return tracker +return tracker.make_fixture_clean_location(request) @pytest.mark.tier1 @@ -122,7 +123,17 @@ class TestCRUD(XMLRPC_test): @pytest.mark.tier1 +@pytest.mark.skipif(not api.Command.dns_is_enabled()['result'], reason='DNS not configured') class TestLocationsServer(XMLRPC_test): +messages = [{ +u'data': {u'service': u'named-pkcs11.service', + u'server': u'%s' % api.env.host}, +u'message': (u'Service named-pkcs11.service requires restart ' + u'on IPA server %s to apply configuration ' + u'changes.' % api.env.host), +u'code': 13025, +u'type': u'warning', +u'name': u'ServiceRestartRequired'}] def test_add_nonexistent_location_to_server(self, server): nonexistent_loc = DNSName(u'nonexistent-location') @@ -140,13 +151,13 @@ class TestLocationsServer(XMLRPC_test): def test_add_location_to_server(self, location, server): location.ensure_exists() server.update( -dict(ipalocation_location=location.idnsname_obj), -expected_updates=dict( -ipalocation_location=[location.idnsname_obj], -) -) +updates={u'ipalocation_location': location.idnsname_obj}, +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': fuzzy_ser
Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
Bump for review. Thanks. On 06/16/2016 03:23 PM, Lenka Doudova wrote: Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka -- 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
Re: [Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests
On 06/22/2016 04:37 PM, Lenka Doudova wrote: On 06/22/2016 08:33 AM, Martin Basti wrote: On 22.06.2016 07:37, Lenka Doudova wrote: On 06/21/2016 06:57 PM, Martin Basti wrote: On 21.06.2016 15:39, Lenka Doudova wrote: Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka Hello, 1) +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, This depends on services installed on server, so server without DNS will cause test failures. We probably should skip test id DNS isn't installed. Without DNS installed you get much more different warnings 2) +def update(self, updates, expected_updates=None, messages=None): +self.messages = messages Why is this needed? I'm puzzled by this It is defined outside __init__ what is wrong and it is never used. Hi, thanks for review. ad 1: will fix ad 2: the 'messages' key is indeed used, because this key is returned every time a server is attached to/removed from a location. If the 'messages' is not supplied to the result comparison, the test fails (see https://paste.fedoraproject.org/382936/65732411/ for result of test without applied patch). Lenka Please point me to the line where ServerTracker.messages is used, I still don't see it. In your fpaste, removed self.messages in that context is TestLocationsServer.messages not ServerTracker.messages Martin^2 Ah, right you are, my good sir. I fixed both issues, hopefully will be fine now. Fixed patch attached. Lenka Self NACK, will provide some more changes tomorrow. Lenka -- 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
Re: [Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests
On 06/22/2016 08:33 AM, Martin Basti wrote: On 22.06.2016 07:37, Lenka Doudova wrote: On 06/21/2016 06:57 PM, Martin Basti wrote: On 21.06.2016 15:39, Lenka Doudova wrote: Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka Hello, 1) +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, This depends on services installed on server, so server without DNS will cause test failures. We probably should skip test id DNS isn't installed. Without DNS installed you get much more different warnings 2) +def update(self, updates, expected_updates=None, messages=None): +self.messages = messages Why is this needed? I'm puzzled by this It is defined outside __init__ what is wrong and it is never used. Hi, thanks for review. ad 1: will fix ad 2: the 'messages' key is indeed used, because this key is returned every time a server is attached to/removed from a location. If the 'messages' is not supplied to the result comparison, the test fails (see https://paste.fedoraproject.org/382936/65732411/ for result of test without applied patch). Lenka Please point me to the line where ServerTracker.messages is used, I still don't see it. In your fpaste, removed self.messages in that context is TestLocationsServer.messages not ServerTracker.messages Martin^2 Ah, right you are, my good sir. I fixed both issues, hopefully will be fine now. Fixed patch attached. Lenka From c3b34ceaf8f3839c616172817651d9461b870812 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 21 Jun 2016 15:29:46 +0200 Subject: [PATCH] Tests: Fix for failing location tests --- ipatests/test_xmlrpc/test_location_plugin.py| 78 +++-- ipatests/test_xmlrpc/tracker/location_plugin.py | 12 +++- ipatests/test_xmlrpc/tracker/server_plugin.py | 58 +++--- 3 files changed, 120 insertions(+), 28 deletions(-) diff --git a/ipatests/test_xmlrpc/test_location_plugin.py b/ipatests/test_xmlrpc/test_location_plugin.py index 3f0edfbcf791feea1a9e1b584386215d84b44606..a17e243450a54c0c96198cb1f626e9abe49b 100644 --- a/ipatests/test_xmlrpc/test_location_plugin.py +++ b/ipatests/test_xmlrpc/test_location_plugin.py @@ -6,6 +6,7 @@ from __future__ import absolute_import import pytest from ipalib import errors, api +from ipatests.util import Fuzzy from ipatests.test_xmlrpc.tracker.location_plugin import LocationTracker from ipatests.test_xmlrpc.tracker.server_plugin import ServerTracker from ipatests.test_xmlrpc.xmlrpc_test import ( @@ -30,13 +31,13 @@ def location_invalid(request): @pytest.fixture(scope='class') def location_absolute(request): tracker = LocationTracker(u'invalid.absolute.') -return tracker.make_fixture(request) +return tracker @pytest.fixture(scope='class') def server(request): tracker = ServerTracker(api.env.host) -return tracker +return tracker.make_fixture_clean_location(request) @pytest.mark.tier1 @@ -123,8 +124,34 @@ class TestCRUD(XMLRPC_test): @pytest.mark.tier1 class TestLocationsServer(XMLRPC_test): +messages = [{ +u'data': {u'service': u'named-pkcs11.service', + u'server': u'%s' % api.env.host}, +u'message': (u'Service named-pkcs11.service requires restart ' + u'on IPA server %s to apply configuration ' + u'changes.' % api.env.host), +u'code': 13025, +u'type': u'warning', +u'name': u'ServiceRestartRequired'}] + +def run_command(self, command_name, **kw): +if not api.Backend.rpcclient.isconnected(): +api.Backend.rpcclient.connect() +try: +api.Command[command_name](**kw) +except errors.NetworkError: +raise pytest.skip('%r: Server not available: %r' % + (self.__module__, api.env.xmlrpc_uri)) + +def check_dns_configured(self): +try: +self.run_command('dnszone_add', idnsname=u'testzone.{0}'.format(api.env)) +self.run_command('dnszone_del', idnsname=u'testzone.{0}'.format(api.env)) +except errors.NotFound: +raise pytest.skip('DNS not configured') def test_add_nonexistent_location_to_server(self, server): +self.check_dns_configured() nonexistent_loc = DNSName(u'nonexistent-location') command = server.make_update_command( updates=dict( @@ -138,52 +165,59
Re: [Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests
On 06/21/2016 06:57 PM, Martin Basti wrote: On 21.06.2016 15:39, Lenka Doudova wrote: Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka Hello, 1) +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, This depends on services installed on server, so server without DNS will cause test failures. We probably should skip test id DNS isn't installed. Without DNS installed you get much more different warnings 2) +def update(self, updates, expected_updates=None, messages=None): +self.messages = messages Why is this needed? I'm puzzled by this It is defined outside __init__ what is wrong and it is never used. Hi, thanks for review. ad 1: will fix ad 2: the 'messages' key is indeed used, because this key is returned every time a server is attached to/removed from a location. If the 'messages' is not supplied to the result comparison, the test fails (see https://paste.fedoraproject.org/382936/65732411/ for result of test without applied patch). Lenka -- 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
[Freeipa-devel] [PATCH 0019][Tests] Fix for failing location tests
Hi, attaching patch for failing location tests (ipatests/test_xmlrpc/test_location_plugin.py). Lenka From 6fc64ea5574e730c5c4c733e4e1eeb60163f6163 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 21 Jun 2016 15:29:46 +0200 Subject: [PATCH] Tests: Fix for failing location tests --- ipatests/test_xmlrpc/test_location_plugin.py| 54 ++ ipatests/test_xmlrpc/tracker/location_plugin.py | 13 -- ipatests/test_xmlrpc/tracker/server_plugin.py | 59 ++--- 3 files changed, 98 insertions(+), 28 deletions(-) diff --git a/ipatests/test_xmlrpc/test_location_plugin.py b/ipatests/test_xmlrpc/test_location_plugin.py index 3f0edfbcf791feea1a9e1b584386215d84b44606..665911fad99673aef36a6ac3a833058c9428fc0e 100644 --- a/ipatests/test_xmlrpc/test_location_plugin.py +++ b/ipatests/test_xmlrpc/test_location_plugin.py @@ -30,13 +30,13 @@ def location_invalid(request): @pytest.fixture(scope='class') def location_absolute(request): tracker = LocationTracker(u'invalid.absolute.') -return tracker.make_fixture(request) +return tracker @pytest.fixture(scope='class') def server(request): tracker = ServerTracker(api.env.host) -return tracker +return tracker.make_fixture_clean_location(request) @pytest.mark.tier1 @@ -123,6 +123,15 @@ class TestCRUD(XMLRPC_test): @pytest.mark.tier1 class TestLocationsServer(XMLRPC_test): +messages = [{ +u'data': {u'service': u'named-pkcs11.service', + u'server': u'%s' % api.env.host}, +u'message': (u'Service named-pkcs11.service requires restart ' + u'on IPA server %s to apply configuration ' + u'changes.' % api.env.host), +u'code': 13025, +u'type': u'warning', +u'name': u'ServiceRestartRequired'}] def test_add_nonexistent_location_to_server(self, server): nonexistent_loc = DNSName(u'nonexistent-location') @@ -140,13 +149,14 @@ class TestLocationsServer(XMLRPC_test): def test_add_location_to_server(self, location, server): location.ensure_exists() server.update( -dict(ipalocation_location=location.idnsname_obj), -expected_updates=dict( -ipalocation_location=[location.idnsname_obj], -) -) +updates={u'ipalocation_location': location.idnsname_obj}, +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, +messages=self.messages) location.add_server_to_location(server.server_name) location.retrieve() +location.remove_server_from_location(server.server_name) def test_retrieve(self, server): server.retrieve() @@ -174,16 +184,16 @@ class TestLocationsServer(XMLRPC_test): def test_add_location_to_server_custom_weight(self, location, server): location.ensure_exists() + server.update( -dict( -ipalocation_location=location.idnsname_obj, -ipaserviceweight=200, -), -expected_updates=dict( -ipalocation_location=[location.idnsname_obj], -ipaserviceweight=[u'200'], -) -) +updates={u'ipalocation_location': location.idnsname_obj, + u'ipaserviceweight': 200}, +expected_updates={u'ipalocation_location': [location.idnsname_obj], + u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server'), + u'ipaserviceweight': [u'200']}, +messages=self.messages) + # remove invalid data from the previous test location.remove_server_from_location(server.server_name) @@ -191,10 +201,18 @@ class TestLocationsServer(XMLRPC_test): location.retrieve() def test_remove_location_from_server(self, location, server): -server.update(dict(ipalocation_location=None)) +server.update( +updates={u'ipalocation_location': None}, +expected_updates={u'enabled_role_servrole': ( + u'CA server', u'DNS server', u'NTP server')}, +messages=self.messages) location.remove_server_from_location(server.server_name) location.retrieve() def test_remove_service_weight_from_server(self, location, server): -server.updat
[Freeipa-devel] [PATCH 0018][Tests] Fix some of the failing tests in test_ipalib/test_frontend.py
Hi, attaching patch with fix for a few failing tests in ipatests/test_ipalib/test_frontend.py. Lenka From 31c7c0f792820aedb4429f8a9a4766653f7fa52c Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Tue, 21 Jun 2016 08:17:17 +0200 Subject: [PATCH] Tests: Fix failing tests in ipatests/test_ipalib/test_frontend.py Test fails were caused mainly by assertion between unicode and nonunicode string, or due to changes in code related to thin client. Fixes: test_Command::test_default_from_chaining test_Command::test_args_options_2_params test_Command::test_params_2_args_options test_Command::test_validate_output_per_type --- ipatests/test_ipalib/test_frontend.py | 20 +--- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py index 93ab547b0ec66c22f11f8e17d51b2930ac489236..0a5951159978706a2265bb688f08deca85de4c0d 100644 --- a/ipatests/test_ipalib/test_frontend.py +++ b/ipatests/test_ipalib/test_frontend.py @@ -24,7 +24,6 @@ Test the `ipalib.frontend` module. # FIXME: Pylint errors # pylint: disable=no-member import pytest - import six from ipatests.util import raises, read_only @@ -462,11 +461,10 @@ class test_Command(ClassChecker): api.finalize() o = my_cmd(api) o.finalize() -e = o(**kw) # pylint: disable=not-callable +e = o.get_default(**kw) # pylint: disable=not-callable assert type(e) is dict -assert 'result' in e -assert 'option2' in e['result'] -assert e['result']['option2'] == u'some value' +assert 'option2' in e +assert e['option2'] == u'some value' def test_validate(self): """ @@ -494,7 +492,7 @@ class test_Command(ClassChecker): fail = dict(okay) fail['option0'] = u'whatever' e = raises(errors.ValidationError, sub.validate, **fail) -assert_equal(e.name, 'option0') +assert_equal(e.name, u'option0') assert_equal(e.value, u'whatever') assert_equal(e.error, u"must equal 'option0'") assert e.rule.__class__.__name__ == 'Rule' @@ -548,7 +546,7 @@ class test_Command(ClassChecker): o = self.get_instance(args=('one', 'two'), options=('three', 'four')) e = raises(errors.OverlapError, o.args_options_2_params, 1, 2, three=3, two=2, four=4, one=1) -assert e.names == ['one', 'two'] +assert e.names == "['one', 'two']" # Test the permutations: o = self.get_instance(args=('one', 'two*'), options=('three', 'four')) @@ -605,9 +603,9 @@ class test_Command(ClassChecker): Test the `ipalib.frontend.Command.params_2_args_options` method. """ o = self.get_instance(args='one', options='two') -assert o.params_2_args_options() == ((None,), {}) +assert o.params_2_args_options() == ((), {}) assert o.params_2_args_options(one=1) == ((1,), {}) -assert o.params_2_args_options(two=2) == ((None,), dict(two=2)) +assert o.params_2_args_options(two=2) == ((), dict(two=2)) assert o.params_2_args_options(two=2, one=1) == ((1,), dict(two=2)) def test_run(self): @@ -751,13 +749,13 @@ class test_Command(ClassChecker): wrong = dict(foo=17.9, bar=[18]) e = raises(TypeError, inst.validate_output, wrong) -assert str(e) == '%s:\n output[%r]: need %r; got %r: %r' % ( +assert str(e) == '%s:\n output[%r]: need (%r,); got %r: %r' % ( 'Complex.validate_output()', 'foo', int, float, 17.9 ) wrong = dict(foo=18, bar=17) e = raises(TypeError, inst.validate_output, wrong) -assert str(e) == '%s:\n output[%r]: need %r; got %r: %r' % ( +assert str(e) == '%s:\n output[%r]: need (%r,); got %r: %r' % ( 'Complex.validate_output()', 'bar', list, int, 17 ) -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0017][Tests] Fix failing ipatests/test_ipalib/test_errors.py
Hi, attaching patch to fix failing test in ipatests/test_ipalib/test_errors.py. Failures were caused by comparing unicode and non-unicode strings, hence I modified responsible non-unicode strings to unicode. Lenka From 28f4860fc22307e9e16937b0e748aa16c2e85937 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 20 Jun 2016 10:29:26 +0200 Subject: [PATCH] Tests: Fix failing ipatests/test_ipalib/test_errors.py Some strings in the testsuite are unicode which wasn't reflected in the tests. This patch fixes the problem by changing concerned strings to unicode. --- ipatests/test_ipalib/test_errors.py | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ipatests/test_ipalib/test_errors.py b/ipatests/test_ipalib/test_errors.py index 7ad07b0418c8f1b1bb89397aa7bf6c8c2f87ab6d..8bf2cba450f26ba12d116725cca4f377029f05db 100644 --- a/ipatests/test_ipalib/test_errors.py +++ b/ipatests/test_ipalib/test_errors.py @@ -244,8 +244,8 @@ class test_PublicError(PublicExceptionTester): message = u'The translated, interpolated message' format = 'key=%(key1)r and key2=%(key2)r' uformat = u'Translated key=%(key1)r and key2=%(key2)r' -val1 = 'Value 1' -val2 = 'Value 2' +val1 = u'Value 1' +val2 = u'Value 2' kw = dict(key1=val1, key2=val2) # Test with format=str, message=None @@ -304,7 +304,7 @@ class test_PublicError(PublicExceptionTester): format = '%(true)r %(text)r %(number)r' uformat = u'Translated %(true)r %(text)r %(number)r' -kw = dict(true=True, text='Hello!', number=18) +kw = dict(true=True, text=u'Hello!', number=18) # Test with format=str, message=None e = raises(ValueError, subclass, format, **kw) @@ -342,7 +342,7 @@ class test_PublicError(PublicExceptionTester): '$') inst = subclass(instructions=instructions, **kw) assert inst.format is subclass.format -assert_equal(inst.instructions, instructions) +assert_equal(inst.instructions, unicode(instructions)) inst_match = regexp.match(inst.strerror).groups() assert_equal(list(inst_match),list(instructions)) -- 2.7.4 -- 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
[Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators
Hi, attached are tests for authentication indicators. Please note: 1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored. 2. patch 0015 depends on 0014, so it should not be pushed without it. Lenka From 2dfcc81b979a345dbed2449a5c30ba19fd2025ee Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 15 Jun 2016 13:40:00 +0200 Subject: [PATCH] Tests: Tracker class for services Provides basic service tracker, so far for purposes of [1]. Tracker is not complete, some methods will need to be added in case of service test refactoring. [1] https://fedorahosted.org/freeipa/ticket/433 --- ipatests/test_xmlrpc/tracker/service_plugin.py | 173 + 1 file changed, 173 insertions(+) create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py new file mode 100644 index ..3fdf17ddf9cd7fee7b5af71347b32c41f61cd355 --- /dev/null +++ b/ipatests/test_xmlrpc/tracker/service_plugin.py @@ -0,0 +1,173 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import six + +from ipalib import api +from ipatests.test_xmlrpc.tracker.base import Tracker +from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid +from ipatests.test_xmlrpc import objectclasses +from ipatests.util import assert_deepequal +from ipapython.dn import DN + +if six.PY3: +unicode = str + + +class ServiceTracker(Tracker): +""" +Tracker class for service plugin + +So far does not include methods for these commands: +service-add-host +service-remove-host +service-allow-retrieve-keytab +service-disallow-retrieve-keytab +service-allow-create-keytab +service-disallow-create-keytab +service-disable +service-add-cert +service-remove-cert +""" + +retrieve_keys = { +u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab', +u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject', +u'managedby', u'serial_number', u'serial_number_hex', u'issuer', +u'valid_not_before', u'valid_not_after', u'md5_fingerprint', +u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host'} +retrieve_all_keys = retrieve_keys | { +u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData', +u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof', +u'objectClass', u'ipakrbrequirespreauth', +u'ipakrbokasdelegate'} + +create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - { +u'usercertificate', u'has_keytab'} +update_keys = retrieve_keys - {u'dn'} + +def __init__(self, name, host_fqdn, options): +super(ServiceTracker, self).__init__(default_version=None) +self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm) +self.dn = DN( +('krbprincipalname', self.name), api.env.container_service, +api.env.basedn) +self.host_fqdn = host_fqdn +self.options = options + +@property +def name(self): +return self._name + +def make_create_command(self, force=True): +""" Make function that creates a service """ +return self.make_command('service_add', self.name, + force=force, **self.options) + +def make_delete_command(self): +""" Make function that deletes a service """ +return self.make_command('service_del', self.name) + +def make_retrieve_command(self, all=False, raw=False): +""" Make function that retrieves a service """ +return self.make_command('service_show', self.name, all=all) + +def make_find_command(self, *args, **kwargs): +""" Make function that searches for a service""" +return self.make_command('service_find', *args, **kwargs) + +def make_update_command(self, updates): +""" Make function that updates a service """ + +return self.make_command('service_mod', self.name, **updates) + +def track_create(self, **options): +""" Update expected state for service creation """ +self.attrs = { +u'dn': self.
[Freeipa-devel] [Testplan] External trust to AD
Hi all, here's [1] a draft of test plan for V4 RFE External trust to Active Directory. Please review this and let me know if there's something missing or wrong. Thanks, Lenka [1] http://www.freeipa.org/page/V4/External_trust_to_AD/Test_Plan -- 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
Re: [Freeipa-devel] [Testplan] Thin client
Hi, thanks for reviewing, I changed the test plan to reflect your comments. Lenka On 05/31/2016 06:07 PM, Petr Vobornik wrote: On 05/31/2016 03:30 PM, Lenka Doudova wrote: Hi all, here's [1] a draft of test plan for V4 RFE Thin client. Please review this and let me know if there's something missing or wrong. Thanks, Lenka [1] http://www.freeipa.org/page/V4/Thin_Client/Test_Plan Hi Lenka, It is implied, but somewhere should be mentioned that "the command" is a command of IPA CLI - `ipa`. E.g. once in overview. Missing: Install 4.4 client against newer server, e.g. non-existant 4.5. Could be simulated. I'm not sure if API version file is still relevant here. Maybe it could be simulated by adding custom plugin which would automatically change version hash (assuming it was implemented that way, if not, please correct me). -- 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
[Freeipa-devel] [Testplan] Authentication indicators
Hi all, here's [1] a draft of test plan for V4 RFE Authentication Indicators. Please review this and let me know if there's something missing or wrong. Thanks, Lenka [1] http://www.freeipa.org/page/V4/Authentication_Indicators/Test_Plan -- 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
[Freeipa-devel] [Testplan] Thin client
Hi all, here's [1] a draft of test plan for V4 RFE Thin client. Please review this and let me know if there's something missing or wrong. Thanks, Lenka [1] http://www.freeipa.org/page/V4/Thin_Client/Test_Plan -- 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
[Freeipa-devel] [Testplan] Support of UPN for trusted domains
Hi all, here [1] is a draft of test plan for V4 RFE Support of UPN for trusted domains. Please review this and let me know if there's something missing or wrong. Thanks, Lenka [1] http://www.freeipa.org/page/V4/Support_of_UPN_for_trusted_domains/Test_Plan -- 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
Re: [Freeipa-devel] [TESTS]{PATCH 0013] Maximum username length higher than 255 cannot be set
On 05/18/2016 01:51 PM, Ganna Kaihorodova wrote: - Original Message - From: "Lenka Doudova" To: "Ganna Kaihorodova" Sent: Wednesday, May 18, 2016 10:37:49 AM Subject: Fwd: [Freeipa-devel] [TESTS]{PATCH 0013] Maximum username length higher than 255 cannot be set Forwarded Message Subject:[Freeipa-devel] [TESTS]{PATCH 0013] Maximum username length higher than 255 cannot be set Date: Fri, 13 May 2016 13:08:29 +0200 From: Lenka Doudova To: freeipa-devel Patch attached. Lenka Hello, sorry, nack, because: 1. Tests doesn't clean up after performing. Max user name isn't returned original value. 2. pep8 errors: ./ipatests/test_xmlrpc/test_config_plugin.py:167:21: E126 continuation line over-indented for hanging indent ./ipatests/test_xmlrpc/test_config_plugin.py:170:17: E121 continuation line under-indented for hanging indent Best regards, Ganna Kaihorodova Associate Software Quality Engineer Hi, thanks for review, fixed patch attached. Lenka From 13a103bf40197d15058a32d61dc2ac3f38fec779 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 13 May 2016 12:56:03 +0200 Subject: [PATCH] Test: Maximum username length higher than 255 cannot be set https://fedorahosted.org/freeipa/ticket/5774 --- ipatests/test_xmlrpc/test_config_plugin.py | 31 +- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/test_config_plugin.py b/ipatests/test_xmlrpc/test_config_plugin.py index 2a9086f258149198a2df91562d1a972b848d92fa..2f57fd236c521a33f08a0a7fc5f955f85efa0dbb 100644 --- a/ipatests/test_xmlrpc/test_config_plugin.py +++ b/ipatests/test_xmlrpc/test_config_plugin.py @@ -1,7 +1,8 @@ # Authors: # Petr Viktorin +# Lenka Doudova # -# Copyright (C) 2010 Red Hat +# Copyright (C) 2010, 2016 Red Hat # see file 'COPYING' for use and warranty information # # This program is free software; you can redistribute it and/or modify @@ -151,4 +152,32 @@ class test_config(Declarative): ), ), +dict( +desc='Set maximum username length higher than limit of 255', +command=('config_mod', [], dict(ipamaxusernamelength=256)), +expected=errors.ValidationError( +name='maxusername', +error='can be at most 255'), +), + +dict( +desc='Set maximum username length equal to limit 255', +command=('config_mod', [], dict(ipamaxusernamelength=255)), +expected=dict( +result=lambda d: d['ipamaxusernamelength'] == (u'255',), +value=None, +summary=None, +), +), + +# Cleanup after previous test - returns max username length to 32 +dict( +desc='Return maximum username length to default value', +command=('config_mod', [], dict(ipamaxusernamelength=32)), +expected=dict( +result=lambda d: d['ipamaxusernamelength'] == (u'32',), +value=None, +summary=None, +), +), ] -- 2.5.5 -- 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
Re: [Freeipa-devel] [TESTS]{PATCH 0013] Maximum username length higher than 255 cannot be set
Bump for review (Ganna) Thanks, Lenka On 05/13/2016 01:08 PM, Lenka Doudova wrote: Patch attached. Lenka -- 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
[Freeipa-devel] [TESTS]{PATCH 0013] Maximum username length higher than 255 cannot be set
Patch attached. Lenka From 2b6d0b4fe1a3f468e5125d521194fb10d4e654c1 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 13 May 2016 12:56:03 +0200 Subject: [PATCH] Test: Maximum username length higher than 255 cannot be set https://fedorahosted.org/freeipa/ticket/5774 --- ipatests/test_xmlrpc/test_config_plugin.py | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/test_config_plugin.py b/ipatests/test_xmlrpc/test_config_plugin.py index 2a9086f258149198a2df91562d1a972b848d92fa..b9992ded895df5aecfa488f9099ededb11291656 100644 --- a/ipatests/test_xmlrpc/test_config_plugin.py +++ b/ipatests/test_xmlrpc/test_config_plugin.py @@ -1,7 +1,8 @@ # Authors: # Petr Viktorin +# Lenka Doudova # -# Copyright (C) 2010 Red Hat +# Copyright (C) 2010, 2016 Red Hat # see file 'COPYING' for use and warranty information # # This program is free software; you can redistribute it and/or modify @@ -151,4 +152,21 @@ class test_config(Declarative): ), ), +dict( +desc='Set maximum username length higher than limit of 255', +command=('config_mod', [], dict(ipamaxusernamelength=256)), +expected=errors.ValidationError( +name='maxusername', +error='can be at most 255'), +), + +dict( +desc='Set maximum username length equal to limit 255', +command=('config_mod', [], dict(ipamaxusernamelength=255)), +expected=dict( +result=lambda d: d['ipamaxusernamelength'] == (u'255',), +value=None, +summary=None, +), +), ] -- 2.5.5 -- 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
[Freeipa-devel] [DESIGN REVIEW] V4/Support of UPN for trusted domains
Hi, design look good, no remarks. Lenka -- 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
[Freeipa-devel] [DESIGN REVIEW] V4/Thin client
Hi, looks fine, but it would be nice to update the document so that it would reflect changes mentioned on previous email thread. Lenka -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0012] Provide cleanup for host certificate
On 05/03/2016 02:08 PM, Lenka Doudova wrote: On 05/03/2016 12:15 PM, Martin Basti wrote: On 03.05.2016 11:18, Lenka Doudova wrote: On 05/03/2016 10:33 AM, Martin Basti wrote: Hello I'm quite confused what is happening in that code, can you explain it more to me? I see duplicated code there. Sorry, that was just an unnecessary leftover. Fixed patch attached. The code is expected to remove any certificates that were added to the local host but not to try to remove the host itself. Lenka Martin^2 Looks better, please follow proper naming of patches according how to format patch guide. I propose following (Patch attached) changes. It looks weird to me to return self object Martin Ok, fixed patch attached. Thanks, Lenka And one more small change. Lenka From 74f4d4782e2c4d468d2ac563c8cbd9e8e2b8ef91 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 2 May 2016 14:04:24 +0200 Subject: [PATCH] Test fix: Cleanup for host certificate This fix provides means to remove certificates from host that were added during tests, but not removed. Ticket: https://fedorahosted.org/freeipa/ticket/5839 --- ipatests/test_xmlrpc/test_host_plugin.py| 4 +++- ipatests/test_xmlrpc/tracker/host_plugin.py | 14 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py index ea8f49656b47b91950130f78375f1c4447157ecb..4d54dac4fec43e342ee3a8a732ad7527d84acc5a 100644 --- a/ipatests/test_xmlrpc/test_host_plugin.py +++ b/ipatests/test_xmlrpc/test_host_plugin.py @@ -130,8 +130,10 @@ def this_host(request): """Fixture for the current master""" tracker = HostTracker(name=api.env.host.partition('.')[0], fqdn=api.env.host) -# This host is not created/deleted, so don't call make_fixture tracker.exists = True +# Finalizer ensures that any certificates added to this_host are removed +tracker.add_finalizer_certcleanup(request) +# This host is not created/deleted, so don't call make_fixture return tracker diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py index 67faa1acf9eeb6174e8d09ca012ae20636fb0f51..d8b59b98907dff955f415b6edb0ce34d74e54f19 100644 --- a/ipatests/test_xmlrpc/tracker/host_plugin.py +++ b/ipatests/test_xmlrpc/tracker/host_plugin.py @@ -10,6 +10,7 @@ from ipatests.test_xmlrpc.tracker.base import Tracker from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid from ipatests.test_xmlrpc import objectclasses from ipatests.util import assert_deepequal +from ipalib import errors class HostTracker(Tracker): @@ -155,3 +156,16 @@ class HostTracker(Tracker): summary=u'Modified host "%s"' % self.fqdn, result=self.filter_attrs(self.update_keys | set(extra_keys)) ), result) + +def add_finalizer_certcleanup(self, request): +""" Fixture to cleanup certificate from local host """ +cleanup_command = self.make_update_command( +updates={'usercertificate':''}) + +def cleanup(): +try: +cleanup_command() +except errors.EmptyModlist: +pass + +request.addfinalizer(cleanup) -- 2.5.5 -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0012] Provide cleanup for host certificate
On 05/03/2016 12:15 PM, Martin Basti wrote: On 03.05.2016 11:18, Lenka Doudova wrote: On 05/03/2016 10:33 AM, Martin Basti wrote: Hello I'm quite confused what is happening in that code, can you explain it more to me? I see duplicated code there. Sorry, that was just an unnecessary leftover. Fixed patch attached. The code is expected to remove any certificates that were added to the local host but not to try to remove the host itself. Lenka Martin^2 Looks better, please follow proper naming of patches according how to format patch guide. I propose following (Patch attached) changes. It looks weird to me to return self object Martin Ok, fixed patch attached. Thanks, Lenka From 46a8fdc91918bfbb2bc9f231581fcb767bc635f5 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 2 May 2016 14:04:24 +0200 Subject: [PATCH] Test fix: Cleanup for host certificate This fix provides means to remove certificates from host that were added during tests, but not removed. Ticket: https://fedorahosted.org/freeipa/ticket/5839 --- ipatests/test_xmlrpc/test_host_plugin.py| 3 ++- ipatests/test_xmlrpc/tracker/host_plugin.py | 14 ++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py index 47f05a403ddb519f201b11251c2acb71faa9133b..85fec423ed530727a20b52534f7568f24e918efa 100644 --- a/ipatests/test_xmlrpc/test_host_plugin.py +++ b/ipatests/test_xmlrpc/test_host_plugin.py @@ -130,8 +130,9 @@ def this_host(request): """Fixture for the current master""" tracker = HostTracker(name=api.env.host.partition('.')[0], fqdn=api.env.host) -# This host is not created/deleted, so don't call make_fixture tracker.exists = True +# Finalizer ensures that any certificates added to this_host are removed +tracker.add_finalizer_certcleanup(request) return tracker diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py index bf199f4f50820fe27384eea4897b73bd02391c56..5e7bd55302ccac942fe740a5e3e15ad828dcb9d1 100644 --- a/ipatests/test_xmlrpc/tracker/host_plugin.py +++ b/ipatests/test_xmlrpc/tracker/host_plugin.py @@ -10,6 +10,7 @@ from ipatests.test_xmlrpc.tracker.base import Tracker from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid from ipatests.test_xmlrpc import objectclasses from ipatests.util import assert_deepequal +from ipalib import errors class HostTracker(Tracker): @@ -152,3 +153,16 @@ class HostTracker(Tracker): summary=u'Modified host "%s"' % self.fqdn, result=self.filter_attrs(self.update_keys | set(extra_keys)) ), result) + +def add_finalizer_certcleanup(self, request): +""" Fixture to cleanup certificate from local host """ +cleanup_command = self.make_update_command( +updates={'usercertificate':''}) + +def cleanup(): +try: +cleanup_command() +except errors.EmptyModlist: +pass + +request.addfinalizer(cleanup) -- 2.5.5 -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0012] Provide cleanup for host certificate
On 05/03/2016 10:33 AM, Martin Basti wrote: Hello I'm quite confused what is happening in that code, can you explain it more to me? I see duplicated code there. Sorry, that was just an unnecessary leftover. Fixed patch attached. The code is expected to remove any certificates that were added to the local host but not to try to remove the host itself. Lenka Martin^2 From 6d61205372653353629308df56f950223ac6671f Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 2 May 2016 14:04:24 +0200 Subject: [PATCH] Test fix: Cleanup for host certificate This fix provides means to remove certificates from host that were added during tests, but not removed. Ticket: https://fedorahosted.org/freeipa/ticket/5839 --- ipatests/test_xmlrpc/test_host_plugin.py| 4 ++-- ipatests/test_xmlrpc/tracker/host_plugin.py | 16 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py index 47f05a403ddb519f201b11251c2acb71faa9133b..b62ce32a2206b489bdc9cdf32851f6101b004218 100644 --- a/ipatests/test_xmlrpc/test_host_plugin.py +++ b/ipatests/test_xmlrpc/test_host_plugin.py @@ -130,9 +130,9 @@ def this_host(request): """Fixture for the current master""" tracker = HostTracker(name=api.env.host.partition('.')[0], fqdn=api.env.host) -# This host is not created/deleted, so don't call make_fixture tracker.exists = True -return tracker +# Fixture ensures that any certificates added to this_host are removed +return tracker.make_fixture_certcleanup(request) @pytest.fixture(scope='class') diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py index bf199f4f50820fe27384eea4897b73bd02391c56..9561f57b3e7d45781f3e36230ef53b21a96e30f3 100644 --- a/ipatests/test_xmlrpc/tracker/host_plugin.py +++ b/ipatests/test_xmlrpc/tracker/host_plugin.py @@ -10,6 +10,7 @@ from ipatests.test_xmlrpc.tracker.base import Tracker from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid from ipatests.test_xmlrpc import objectclasses from ipatests.util import assert_deepequal +from ipalib import errors class HostTracker(Tracker): @@ -152,3 +153,18 @@ class HostTracker(Tracker): summary=u'Modified host "%s"' % self.fqdn, result=self.filter_attrs(self.update_keys | set(extra_keys)) ), result) + +def make_fixture_certcleanup(self, request): +""" Fixture to cleanup certificate from local host """ +cleanup_command = self.make_update_command( +updates={'usercertificate':''}) + +def cleanup(): +try: +cleanup_command() +except errors.EmptyModlist: +pass + +request.addfinalizer(cleanup) + +return self -- 2.5.5 -- 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
[Freeipa-devel] [TESTS][PATCH 0012] Provide cleanup for host certificate
Hi, attached patch provides solution for https://fedorahosted.org/freeipa/ticket/5839 by removing all certificates added to local host during tests. Lenka From 031adf1f50308b70e87c93a7a853f04eae593bf0 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 2 May 2016 14:04:24 +0200 Subject: [PATCH] Test fix: Cleanup for host certificate This fix provides means to remove certificates from host that were added during tests, but not removed. Ticket: https://fedorahosted.org/freeipa/ticket/5839 --- ipatests/test_xmlrpc/test_host_plugin.py| 4 ++-- ipatests/test_xmlrpc/tracker/host_plugin.py | 20 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py index 47f05a403ddb519f201b11251c2acb71faa9133b..b62ce32a2206b489bdc9cdf32851f6101b004218 100644 --- a/ipatests/test_xmlrpc/test_host_plugin.py +++ b/ipatests/test_xmlrpc/test_host_plugin.py @@ -130,9 +130,9 @@ def this_host(request): """Fixture for the current master""" tracker = HostTracker(name=api.env.host.partition('.')[0], fqdn=api.env.host) -# This host is not created/deleted, so don't call make_fixture tracker.exists = True -return tracker +# Fixture ensures that any certificates added to this_host are removed +return tracker.make_fixture_certcleanup(request) @pytest.fixture(scope='class') diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py index bf199f4f50820fe27384eea4897b73bd02391c56..a7f0445fe4119c525d481eb8eab7e5cea36706c9 100644 --- a/ipatests/test_xmlrpc/tracker/host_plugin.py +++ b/ipatests/test_xmlrpc/tracker/host_plugin.py @@ -10,6 +10,7 @@ from ipatests.test_xmlrpc.tracker.base import Tracker from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid from ipatests.test_xmlrpc import objectclasses from ipatests.util import assert_deepequal +from ipalib import errors class HostTracker(Tracker): @@ -152,3 +153,22 @@ class HostTracker(Tracker): summary=u'Modified host "%s"' % self.fqdn, result=self.filter_attrs(self.update_keys | set(extra_keys)) ), result) + +def make_fixture_certcleanup(self, request): +""" Fixture to cleanup certificate from local host """ +cleanup_command = self.make_update_command( +updates={'usercertificate':''}) +try: +cleanup_command() +except errors.EmptyModlist: +pass + +def cleanup(): +try: +cleanup_command() +except errors.EmptyModlist: +pass + +request.addfinalizer(cleanup) + +return self -- 2.5.5 -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group
On 04/04/2016 06:54 PM, Martin Basti wrote: On 01.04.2016 14:34, Pavel Vomacka wrote: On 03/31/2016 04:16 PM, Lenka Doudova wrote: On 03/31/2016 12:42 PM, Pavel Vomacka wrote: On 03/18/2016 11:24 AM, Lenka Doudova wrote: On 03/10/2016 06:58 PM, Petr Vobornik wrote: On 03/08/2016 01:17 PM, Lenka Doudova wrote: On 03/08/2016 12:59 PM, Petr Vobornik wrote: On 03/07/2016 04:29 PM, Pavel Vomacka wrote: On 02/25/2016 03:08 PM, Lenka Doudova wrote: Hi, here's a patch for webUI tests that provides test for creating user without private group. Related to ticket https://fedorahosted.org/freeipa/ticket/4986 Since the option to specify GID when creating a user is not available https://fedorahosted.org/freeipa/ticket/5505 the test creates a new posix group, makes it a default user group instead of 'ipausers' and then attemps to create the user without private group. Returning default user group value to 'ipausers' is provided even for cases when the test fails so it would not block other tests from performing properly. Lenka Hi, ACK, works well. Pavel^3 Vomacka NACK, don't use naked except, specify at least 'Exception' +except: Thanks, patch fixed according to Petr's review attached. Lenka Ticket 5505 was pushed. So the workaround can be removed. Do you prefer to do it in this patch? Also, maybe it would be good to test both cases and check if the error is actually the right one. Hi, attaching patch fixed according to recently pushed changes. Lenka Hi, NACK, 1) The data definition for user3 (user.DATA3) is not used anywhere. And the definition is actually the same as definition of user4. So, I think that it could be removed. 2) This is just a detail, but I would rather use 'combobox_input' or 'combobox_textbox' as parameter name because the parameter actually doesn't represent the value of combobox. Otherwise it works as expected. -- Pavel^3 Vomacka Hi, thanks for comments, updated patch attached. Lenka Thank you, ACK. -- Pavel^3 Vomacka Ticket is in closed milestone, this patch cannot be attached to closed milestone, please create a new ticket/or post the proper ticket. Hi, new ticket has been created for this issue. Patch with modified commit message to refer to the new ticket attached. Lenka From 42be921e6d9651e38cbd4e2a536972f205219c0e Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 25 Feb 2016 15:00:49 +0100 Subject: [PATCH] WebUI: Test creating user without private group Test for option to create a user without private group in web UI. Covers ticket https://fedorahosted.org/freeipa/ticket/5804 --- ipatests/test_webui/data_group.py | 10 ++ ipatests/test_webui/data_user.py | 24 +++- ipatests/test_webui/test_user.py | 38 ++ ipatests/test_webui/ui_driver.py | 31 +++ 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/ipatests/test_webui/data_group.py b/ipatests/test_webui/data_group.py index 2b32b2f32983c78a002a06888b8f82b38b608d1a..9d79d18a1348e6b90bb0867261bf30706134db22 100644 --- a/ipatests/test_webui/data_group.py +++ b/ipatests/test_webui/data_group.py @@ -68,3 +68,13 @@ DATA5 = { ('textarea', 'description', 'test-group5 desc'), ] } + +PKEY6 = 'itest-group6' +DATA6 = { +'pkey': PKEY6, +'add': [ +('textbox', 'cn', PKEY6), +('textarea', 'description', 'test-group6 desc'), +('textbox', 'gidnumber', '7'), +] +} diff --git a/ipatests/test_webui/data_user.py b/ipatests/test_webui/data_user.py index 79a53898035a2723c1e79c459ba4aef3ffa67723..c5ed796c7bfe9f56d6ab1e179b2f7343bd05d73b 100644 --- a/ipatests/test_webui/data_user.py +++ b/ipatests/test_webui/data_user.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. - ENTITY = 'user' PKEY = 'itest-user' @@ -63,3 +62,26 @@ DATA2 = { ('textbox', 'sn', 'OtherSurname2'), ], } + +PKEY3 = 'itest-user3' +DATA3 = { +'pkey': PKEY3, +'add': [ +('textbox', 'uid', PKEY3), +('textbox', 'givenname', 'Name3'), +('textbox', 'sn', 'Surname3'), +('checkbox', 'noprivate', None), +] +} + +PKEY4 = 'itest-user4' +DATA4 = { +'pkey': PKEY4, +'add': [ +('textbox', 'uid', PKEY4), +('textbox', 'givenname', 'Name4'), +('textbox', 'sn'
Re: [Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group
On 03/31/2016 12:42 PM, Pavel Vomacka wrote: On 03/18/2016 11:24 AM, Lenka Doudova wrote: On 03/10/2016 06:58 PM, Petr Vobornik wrote: On 03/08/2016 01:17 PM, Lenka Doudova wrote: On 03/08/2016 12:59 PM, Petr Vobornik wrote: On 03/07/2016 04:29 PM, Pavel Vomacka wrote: On 02/25/2016 03:08 PM, Lenka Doudova wrote: Hi, here's a patch for webUI tests that provides test for creating user without private group. Related to ticket https://fedorahosted.org/freeipa/ticket/4986 Since the option to specify GID when creating a user is not available https://fedorahosted.org/freeipa/ticket/5505 the test creates a new posix group, makes it a default user group instead of 'ipausers' and then attemps to create the user without private group. Returning default user group value to 'ipausers' is provided even for cases when the test fails so it would not block other tests from performing properly. Lenka Hi, ACK, works well. Pavel^3 Vomacka NACK, don't use naked except, specify at least 'Exception' +except: Thanks, patch fixed according to Petr's review attached. Lenka Ticket 5505 was pushed. So the workaround can be removed. Do you prefer to do it in this patch? Also, maybe it would be good to test both cases and check if the error is actually the right one. Hi, attaching patch fixed according to recently pushed changes. Lenka Hi, NACK, 1) The data definition for user3 (user.DATA3) is not used anywhere. And the definition is actually the same as definition of user4. So, I think that it could be removed. 2) This is just a detail, but I would rather use 'combobox_input' or 'combobox_textbox' as parameter name because the parameter actually doesn't represent the value of combobox. Otherwise it works as expected. -- Pavel^3 Vomacka Hi, thanks for comments, updated patch attached. Lenka From a1d462cdb692b6330fa8f948829bb42d5d9fd7b6 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 25 Feb 2016 15:00:49 +0100 Subject: [PATCH] WebUI: Test creating user without private group Test for option to create a user without private group in web UI. Covers ticket https://fedorahosted.org/freeipa/ticket/4986 --- ipatests/test_webui/data_group.py | 10 ++ ipatests/test_webui/data_user.py | 24 +++- ipatests/test_webui/test_user.py | 38 ++ ipatests/test_webui/ui_driver.py | 31 +++ 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/ipatests/test_webui/data_group.py b/ipatests/test_webui/data_group.py index 2b32b2f32983c78a002a06888b8f82b38b608d1a..9d79d18a1348e6b90bb0867261bf30706134db22 100644 --- a/ipatests/test_webui/data_group.py +++ b/ipatests/test_webui/data_group.py @@ -68,3 +68,13 @@ DATA5 = { ('textarea', 'description', 'test-group5 desc'), ] } + +PKEY6 = 'itest-group6' +DATA6 = { +'pkey': PKEY6, +'add': [ +('textbox', 'cn', PKEY6), +('textarea', 'description', 'test-group6 desc'), +('textbox', 'gidnumber', '7'), +] +} diff --git a/ipatests/test_webui/data_user.py b/ipatests/test_webui/data_user.py index 79a53898035a2723c1e79c459ba4aef3ffa67723..c5ed796c7bfe9f56d6ab1e179b2f7343bd05d73b 100644 --- a/ipatests/test_webui/data_user.py +++ b/ipatests/test_webui/data_user.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. - ENTITY = 'user' PKEY = 'itest-user' @@ -63,3 +62,26 @@ DATA2 = { ('textbox', 'sn', 'OtherSurname2'), ], } + +PKEY3 = 'itest-user3' +DATA3 = { +'pkey': PKEY3, +'add': [ +('textbox', 'uid', PKEY3), +('textbox', 'givenname', 'Name3'), +('textbox', 'sn', 'Surname3'), +('checkbox', 'noprivate', None), +] +} + +PKEY4 = 'itest-user4' +DATA4 = { +'pkey': PKEY4, +'add': [ +('textbox', 'uid', PKEY4), +('textbox', 'givenname', 'Name4'), +('textbox', 'sn', 'Surname4'), +('checkbox', 'noprivate', None), +('combobox', 'gidnumber', '7'), +] +} diff --git a/ipatests/test_webui/test_user.py b/ipatests/test_webui/test_user.py index b216125b226230ab268de3f86ec7ac6f785c6e16..5b509d18ca9a0738af1595678fd56ba57d405079 100644 --- a/ipatests/test_webui/test_user.py +++ b/ipatests/test_webui/test_user.py @@ -261,3 +261,41 @@ cl
Re: [Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group
On 03/10/2016 06:58 PM, Petr Vobornik wrote: On 03/08/2016 01:17 PM, Lenka Doudova wrote: On 03/08/2016 12:59 PM, Petr Vobornik wrote: On 03/07/2016 04:29 PM, Pavel Vomacka wrote: On 02/25/2016 03:08 PM, Lenka Doudova wrote: Hi, here's a patch for webUI tests that provides test for creating user without private group. Related to ticket https://fedorahosted.org/freeipa/ticket/4986 Since the option to specify GID when creating a user is not available https://fedorahosted.org/freeipa/ticket/5505 the test creates a new posix group, makes it a default user group instead of 'ipausers' and then attemps to create the user without private group. Returning default user group value to 'ipausers' is provided even for cases when the test fails so it would not block other tests from performing properly. Lenka Hi, ACK, works well. Pavel^3 Vomacka NACK, don't use naked except, specify at least 'Exception' +except: Thanks, patch fixed according to Petr's review attached. Lenka Ticket 5505 was pushed. So the workaround can be removed. Do you prefer to do it in this patch? Also, maybe it would be good to test both cases and check if the error is actually the right one. Hi, attaching patch fixed according to recently pushed changes. Lenka From 3f8f4f3d1485b27adfcdcb72fc382db72a1a44a1 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 25 Feb 2016 15:00:49 +0100 Subject: [PATCH] WebUI: Test creating user without private group Test for option to create a user without private group in web UI. Covers ticket https://fedorahosted.org/freeipa/ticket/4986 --- ipatests/test_webui/data_group.py | 10 ++ ipatests/test_webui/data_user.py | 35 ++- ipatests/test_webui/test_user.py | 38 ++ ipatests/test_webui/ui_driver.py | 31 +++ 4 files changed, 101 insertions(+), 13 deletions(-) diff --git a/ipatests/test_webui/data_group.py b/ipatests/test_webui/data_group.py index 2b32b2f32983c78a002a06888b8f82b38b608d1a..9d79d18a1348e6b90bb0867261bf30706134db22 100644 --- a/ipatests/test_webui/data_group.py +++ b/ipatests/test_webui/data_group.py @@ -68,3 +68,13 @@ DATA5 = { ('textarea', 'description', 'test-group5 desc'), ] } + +PKEY6 = 'itest-group6' +DATA6 = { +'pkey': PKEY6, +'add': [ +('textbox', 'cn', PKEY6), +('textarea', 'description', 'test-group6 desc'), +('textbox', 'gidnumber', '7'), +] +} diff --git a/ipatests/test_webui/data_user.py b/ipatests/test_webui/data_user.py index 79a53898035a2723c1e79c459ba4aef3ffa67723..e0d7f7b209291eca1adccf5771c63a7d96a20d44 100644 --- a/ipatests/test_webui/data_user.py +++ b/ipatests/test_webui/data_user.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. - ENTITY = 'user' PKEY = 'itest-user' @@ -63,3 +62,37 @@ DATA2 = { ('textbox', 'sn', 'OtherSurname2'), ], } + +PKEY3 = 'itest-user3' +DATA3 = { +'pkey': PKEY3, +'add': [ +('textbox', 'uid', PKEY3), +('textbox', 'givenname', 'Name3'), +('textbox', 'sn', 'Surname3'), +('checkbox', 'noprivate', None), +] +} + +PKEY4 = 'itest-user4' +DATA4 = { +'pkey': PKEY4, +'add': [ +('textbox', 'uid', PKEY4), +('textbox', 'givenname', 'Name4'), +('textbox', 'sn', 'Surname4'), +('checkbox', 'noprivate', None), +] +} + +PKEY5 = 'itest-user5' +DATA5 = { +'pkey': PKEY5, +'add': [ +('textbox', 'uid', PKEY5), +('textbox', 'givenname', 'Name5'), +('textbox', 'sn', 'Surname5'), +('checkbox', 'noprivate', None), +('combobox', 'gidnumber', '7'), +] +} diff --git a/ipatests/test_webui/test_user.py b/ipatests/test_webui/test_user.py index b216125b226230ab268de3f86ec7ac6f785c6e16..1598d6e4de18ed9261fee1b7fa533a7701216fa8 100644 --- a/ipatests/test_webui/test_user.py +++ b/ipatests/test_webui/test_user.py @@ -261,3 +261,41 @@ class test_user(UI_driver): self.dialog_button_click('confirm') self.wait_for_request(n=3) self.assert_no_error_dialog() + + +@pytest.mark.tier1 +class test_user_no_private_
Re: [Freeipa-devel] [TESTS][PATCH 0010] WebUI tests - ID views
On 03/08/2016 12:43 PM, Pavel Vomacka wrote: On 02/23/2016 03:20 PM, Lenka Doudova wrote: Hi, attached is patch providing missing test coverage for ID views in webUI. Lenka Hi, Thank you for your patch. I have small one comment: The delete_associtaion() function duplicates code, it would be good to create auxiliary function and move that code there. NACK. -- Pavel^3 Vomacka Hi, thanks for review, fixed patch attached. Lenka From 1b705000574722e57850b51792254f8e1f96b88f Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 19 Feb 2016 14:59:19 +0100 Subject: [PATCH] WebUI test: ID views Provides missing test coverage for ID views web UI. --- ipatests/test_webui/data_idviews.py | 20 ++ ipatests/test_webui/test_idviews.py | 127 ipatests/test_webui/ui_driver.py| 66 ++- 3 files changed, 195 insertions(+), 18 deletions(-) create mode 100644 ipatests/test_webui/data_idviews.py create mode 100644 ipatests/test_webui/test_idviews.py diff --git a/ipatests/test_webui/data_idviews.py b/ipatests/test_webui/data_idviews.py new file mode 100644 index ..9d62f33fe47dcf2cbde525d687ddd80c50b367e0 --- /dev/null +++ b/ipatests/test_webui/data_idviews.py @@ -0,0 +1,20 @@ +# +# Copyright (C) 2015 FreeIPA Contributors see COPYING for license +# + +ENTITY = 'idview' +USER_FACET = 'idoverrideuser' +GROUP_FACET = 'idoverridegroup' +HOST_FACET = 'appliedtohosts' + +PKEY = 'itest-view' +DATA = { +'pkey': PKEY, +'add': [ +('textbox', 'cn', PKEY), +('textarea', 'description', 'Description of ID view'), +], +'mod': [ +('textarea', 'description', 'Different description'), +], +} diff --git a/ipatests/test_webui/test_idviews.py b/ipatests/test_webui/test_idviews.py new file mode 100644 index ..b7f5c312eb2a5a7680b55126ae91446b5010a5b8 --- /dev/null +++ b/ipatests/test_webui/test_idviews.py @@ -0,0 +1,127 @@ +# +# Copyright (C) 2015 FreeIPA Contributors see COPYING for license +# + +from ipatests.test_webui.ui_driver import UI_driver +from ipatests.test_webui.ui_driver import screenshot +import ipatests.test_webui.data_idviews as idview +import ipatests.test_webui.data_user as user +import ipatests.test_webui.data_group as group +import ipatests.test_webui.data_hostgroup as hostgroup +from ipatests.test_webui.test_host import host_tasks, ENTITY as HOST_ENTITY +import pytest + +DATA_USER = { +'pkey': user.PKEY, +'add': [ +('combobox', 'ipaanchoruuid', user.PKEY), +('textbox', 'uid', 'iduser'), +('textbox', 'gecos', 'id user'), +('textbox', 'uidnumber', 1), +('textbox', 'gidnumber', 1), +('textbox', 'loginshell', 'shell'), +('textbox', 'homedirectory', 'home'), +('textarea', 'description', 'desc'), +], +'mod': [ +('textbox', 'uid', 'moduser'), +('textbox', 'uidnumber', 3), +], +} + +DATA_GROUP = { +'pkey': group.PKEY, +'add': [ +('combobox', 'ipaanchoruuid', group.PKEY), +('textbox', 'cn', 'idgroup'), +('textbox', 'gidnumber', 2), +('textarea', 'description', 'desc'), +], +'mod': [ +('textbox', 'cn', 'modgroup'), +('textbox', 'gidnumber', 3), +], +} + + +@pytest.mark.tier1 +class test_idviews(UI_driver): + +@screenshot +def test_crud(self): +""" +Basic CRUD: ID view +""" +self.init_app() +self.basic_crud( +idview.ENTITY, idview.DATA, default_facet=idview.USER_FACET) + +@screenshot +def test_overrides(self): +""" +User and group overrides +""" +self.init_app() + +self.add_record(user.ENTITY, user.DATA, navigate=False) +self.add_record(group.ENTITY, group.DATA) +self.add_record(idview.ENTITY, idview.DATA) + +self.navigate_to_record(idview.PKEY) +parent_entity = 'idview' + +# user override +self.add_record(parent_entity, DATA_USER, facet=idview.USER_FACET) +self.navigate_to_record(user.PKEY) +self.mod_record(idview.USER_FACET, DATA_USER) +self.delete_action(idview.ENTITY, user.PKEY) + +#
Re: [Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group
On 03/08/2016 12:59 PM, Petr Vobornik wrote: On 03/07/2016 04:29 PM, Pavel Vomacka wrote: On 02/25/2016 03:08 PM, Lenka Doudova wrote: Hi, here's a patch for webUI tests that provides test for creating user without private group. Related to ticket https://fedorahosted.org/freeipa/ticket/4986 Since the option to specify GID when creating a user is not available https://fedorahosted.org/freeipa/ticket/5505 the test creates a new posix group, makes it a default user group instead of 'ipausers' and then attemps to create the user without private group. Returning default user group value to 'ipausers' is provided even for cases when the test fails so it would not block other tests from performing properly. Lenka Hi, ACK, works well. Pavel^3 Vomacka NACK, don't use naked except, specify at least 'Exception' +except: Thanks, patch fixed according to Petr's review attached. Lenka From 9d7ffe59b752675b666c7c16158c7caa0b7ef09f Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 25 Feb 2016 15:00:49 +0100 Subject: [PATCH] WebUI: Test creating user without private group Test for option to create a user without private group in web UI. Covers ticket https://fedorahosted.org/freeipa/ticket/4986 --- ipatests/test_webui/data_user.py | 11 +++ ipatests/test_webui/test_user.py | 31 +++ 2 files changed, 42 insertions(+) diff --git a/ipatests/test_webui/data_user.py b/ipatests/test_webui/data_user.py index 79a53898035a2723c1e79c459ba4aef3ffa67723..50945c1e95634654ca3a8f0dabd09e6d7e2d0452 100644 --- a/ipatests/test_webui/data_user.py +++ b/ipatests/test_webui/data_user.py @@ -63,3 +63,14 @@ DATA2 = { ('textbox', 'sn', 'OtherSurname2'), ], } + +PKEY3 = 'itest-user3' +DATA3 = { +'pkey': PKEY3, +'add': [ +('textbox', 'uid', PKEY3), +('textbox', 'givenname', 'Name3'), +('textbox', 'sn', 'Surname3'), +('checkbox', 'noprivate', None), +] +} diff --git a/ipatests/test_webui/test_user.py b/ipatests/test_webui/test_user.py index b216125b226230ab268de3f86ec7ac6f785c6e16..5a3c1e46d3b9239b19ee7822faa1976a8fbedfe0 100644 --- a/ipatests/test_webui/test_user.py +++ b/ipatests/test_webui/test_user.py @@ -222,6 +222,37 @@ class test_user(UI_driver): self.delete(user.ENTITY, [user.DATA]) self.delete(group.ENTITY, [group.DATA]) +@screenshot +def test_noprivate(self): +""" +User without private group +""" +self.init_app() +self.add_record(group.ENTITY, group.DATA2) + +try: +self.set_ipadefaultprimarygroup(group.PKEY2) +self.add_record(user.ENTITY, user.DATA3) +self.delete(user.ENTITY, [user.DATA3]) +except Exception: +self.dialog_button_click('cancel') # exit error dialog +self.dialog_button_click('cancel') # exit add user dialog +raise +finally: +self.set_ipadefaultprimarygroup('ipausers') +self.delete(group.ENTITY, [group.DATA2]) + +def set_ipadefaultprimarygroup(self, group): +""" +Set ipa config "Default users group" field +""" +self.navigate_to_entity('config') +self.mod_record('config', { +'mod': [ +('combobox', 'ipadefaultprimarygroup', group), +] +}) + def set_ipapwdexpadvnotify(self, days): """ Set ipa config "Password Expiration Notification (days)" field -- 2.5.0 -- 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
[Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group
Hi, here's a patch for webUI tests that provides test for creating user without private group. Related to ticket https://fedorahosted.org/freeipa/ticket/4986 Since the option to specify GID when creating a user is not available https://fedorahosted.org/freeipa/ticket/5505 the test creates a new posix group, makes it a default user group instead of 'ipausers' and then attemps to create the user without private group. Returning default user group value to 'ipausers' is provided even for cases when the test fails so it would not block other tests from performing properly. Lenka From b9d0d7d5887e339de24d9878b733b45a0618bb9b Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Thu, 25 Feb 2016 15:00:49 +0100 Subject: [PATCH] WebUI: Test creating user without private group Test for option to create a user without private group in web UI. Covers ticket https://fedorahosted.org/freeipa/ticket/4986 --- ipatests/test_webui/data_user.py | 11 +++ ipatests/test_webui/test_user.py | 31 +++ 2 files changed, 42 insertions(+) diff --git a/ipatests/test_webui/data_user.py b/ipatests/test_webui/data_user.py index 79a53898035a2723c1e79c459ba4aef3ffa67723..50945c1e95634654ca3a8f0dabd09e6d7e2d0452 100644 --- a/ipatests/test_webui/data_user.py +++ b/ipatests/test_webui/data_user.py @@ -63,3 +63,14 @@ DATA2 = { ('textbox', 'sn', 'OtherSurname2'), ], } + +PKEY3 = 'itest-user3' +DATA3 = { +'pkey': PKEY3, +'add': [ +('textbox', 'uid', PKEY3), +('textbox', 'givenname', 'Name3'), +('textbox', 'sn', 'Surname3'), +('checkbox', 'noprivate', None), +] +} diff --git a/ipatests/test_webui/test_user.py b/ipatests/test_webui/test_user.py index b216125b226230ab268de3f86ec7ac6f785c6e16..713df5dc929c0b4e1761cab5c1a1a1675cc00669 100644 --- a/ipatests/test_webui/test_user.py +++ b/ipatests/test_webui/test_user.py @@ -222,6 +222,37 @@ class test_user(UI_driver): self.delete(user.ENTITY, [user.DATA]) self.delete(group.ENTITY, [group.DATA]) +@screenshot +def test_noprivate(self): +""" +User without private group +""" +self.init_app() +self.add_record(group.ENTITY, group.DATA2) + +try: +self.set_ipadefaultprimarygroup(group.PKEY2) +self.add_record(user.ENTITY, user.DATA3) +self.delete(user.ENTITY, [user.DATA3]) +except: +self.dialog_button_click('cancel') # exit error dialog +self.dialog_button_click('cancel') # exit add user dialog +raise +finally: +self.set_ipadefaultprimarygroup('ipausers') +self.delete(group.ENTITY, [group.DATA2]) + +def set_ipadefaultprimarygroup(self, group): +""" +Set ipa config "Default users group" field +""" +self.navigate_to_entity('config') +self.mod_record('config', { +'mod': [ +('combobox', 'ipadefaultprimarygroup', group), +] +}) + def set_ipapwdexpadvnotify(self, days): """ Set ipa config "Password Expiration Notification (days)" field -- 2.5.0 -- 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
[Freeipa-devel] [TESTS][PATCH 0010] WebUI tests - ID views
Hi, attached is patch providing missing test coverage for ID views in webUI. Lenka From 5940a3e7f63b7b6360a28fd52ba6c7df65e4ea98 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Fri, 19 Feb 2016 14:59:19 +0100 Subject: [PATCH] WebUI test: ID views Provides missing test coverage for ID views web UI. --- ipatests/test_webui/data_idviews.py | 20 ++ ipatests/test_webui/test_idviews.py | 127 ipatests/test_webui/ui_driver.py| 48 -- 3 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 ipatests/test_webui/data_idviews.py create mode 100644 ipatests/test_webui/test_idviews.py diff --git a/ipatests/test_webui/data_idviews.py b/ipatests/test_webui/data_idviews.py new file mode 100644 index ..9d62f33fe47dcf2cbde525d687ddd80c50b367e0 --- /dev/null +++ b/ipatests/test_webui/data_idviews.py @@ -0,0 +1,20 @@ +# +# Copyright (C) 2015 FreeIPA Contributors see COPYING for license +# + +ENTITY = 'idview' +USER_FACET = 'idoverrideuser' +GROUP_FACET = 'idoverridegroup' +HOST_FACET = 'appliedtohosts' + +PKEY = 'itest-view' +DATA = { +'pkey': PKEY, +'add': [ +('textbox', 'cn', PKEY), +('textarea', 'description', 'Description of ID view'), +], +'mod': [ +('textarea', 'description', 'Different description'), +], +} diff --git a/ipatests/test_webui/test_idviews.py b/ipatests/test_webui/test_idviews.py new file mode 100644 index ..b7f5c312eb2a5a7680b55126ae91446b5010a5b8 --- /dev/null +++ b/ipatests/test_webui/test_idviews.py @@ -0,0 +1,127 @@ +# +# Copyright (C) 2015 FreeIPA Contributors see COPYING for license +# + +from ipatests.test_webui.ui_driver import UI_driver +from ipatests.test_webui.ui_driver import screenshot +import ipatests.test_webui.data_idviews as idview +import ipatests.test_webui.data_user as user +import ipatests.test_webui.data_group as group +import ipatests.test_webui.data_hostgroup as hostgroup +from ipatests.test_webui.test_host import host_tasks, ENTITY as HOST_ENTITY +import pytest + +DATA_USER = { +'pkey': user.PKEY, +'add': [ +('combobox', 'ipaanchoruuid', user.PKEY), +('textbox', 'uid', 'iduser'), +('textbox', 'gecos', 'id user'), +('textbox', 'uidnumber', 1), +('textbox', 'gidnumber', 1), +('textbox', 'loginshell', 'shell'), +('textbox', 'homedirectory', 'home'), +('textarea', 'description', 'desc'), +], +'mod': [ +('textbox', 'uid', 'moduser'), +('textbox', 'uidnumber', 3), +], +} + +DATA_GROUP = { +'pkey': group.PKEY, +'add': [ +('combobox', 'ipaanchoruuid', group.PKEY), +('textbox', 'cn', 'idgroup'), +('textbox', 'gidnumber', 2), +('textarea', 'description', 'desc'), +], +'mod': [ +('textbox', 'cn', 'modgroup'), +('textbox', 'gidnumber', 3), +], +} + + +@pytest.mark.tier1 +class test_idviews(UI_driver): + +@screenshot +def test_crud(self): +""" +Basic CRUD: ID view +""" +self.init_app() +self.basic_crud( +idview.ENTITY, idview.DATA, default_facet=idview.USER_FACET) + +@screenshot +def test_overrides(self): +""" +User and group overrides +""" +self.init_app() + +self.add_record(user.ENTITY, user.DATA, navigate=False) +self.add_record(group.ENTITY, group.DATA) +self.add_record(idview.ENTITY, idview.DATA) + +self.navigate_to_record(idview.PKEY) +parent_entity = 'idview' + +# user override +self.add_record(parent_entity, DATA_USER, facet=idview.USER_FACET) +self.navigate_to_record(user.PKEY) +self.mod_record(idview.USER_FACET, DATA_USER) +self.delete_action(idview.ENTITY, user.PKEY) + +# group override +self.navigate_to_record(idview.PKEY) +self.switch_to_facet(idview.GROUP_FACET) +self.add_record(parent_entity, DATA_GROUP, facet=idview.GROUP_FACET) +self.navigate_to_record(group.PKEY) +self.mod_record(idview.GROUP_FACET, DATA_GROUP) +self.delete_action(idview.ENTITY, group.PKEY) + +#
Re: [Freeipa-devel] [TESTS][PATCH 0009] WebUI tests fix
On 02/19/2016 10:51 AM, Petr Vobornik wrote: On 02/16/2016 10:10 AM, Lenka Doudova wrote: On 02/11/2016 11:13 AM, Lenka Doudova wrote: Hi all, most of webUI tests fail with AssertionError: Can't click on checkbox label: table.table Message: Element is not clickable at point (37, 340.338964844). Other element would receive the click: The problem seems to be that the tests attempt to click on a checkbox label that is no longer there. Since the checkbox is clickable directly, I changed the code accordingly. Most of the tests should now proceed successfully. Lenka Attaching updated patch with minor fix. Lenka would ACK but the commit message is so generic that it doesn't say anything. Also the description in the mail should be in commit message to be usable in a future. Fix attached, hope this is better. Lenka From df577030af9e29f41a18838d2d8c83b5cca95154 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 10 Feb 2016 16:16:22 +0100 Subject: [PATCH] WebUI tests: fix failing of tests due to unclicable label Checkbox label is no longer clickable, most tests fail with error like this: AssertionError: Can't click on checkbox label: table.table Message: Element is not clickable at point (37, 340.338964844). Other element would receive the click: The checkbox is clickable directly without the label, this patch provides according fix. --- ipatests/test_webui/ui_driver.py | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py index fc22f8612d49e300437eb91cb58add1a0376eb2c..ad3a9100f65f8ce64aaaf707df04a2bd411c3299 100644 --- a/ipatests/test_webui/ui_driver.py +++ b/ipatests/test_webui/ui_driver.py @@ -933,12 +933,8 @@ class UI_driver(object): s = self.get_table_selector(table_name) input_s = s + " tbody td input[value='%s']" % pkey checkbox = self.find(input_s, By.CSS_SELECTOR, parent, strict=True) -checkbox_id = checkbox.get_attribute('id') -label_s = s + " tbody td label[for='%s']" % checkbox_id -print(label_s) -label = self.find(label_s, By.CSS_SELECTOR, parent, strict=True) try: -ActionChains(self.driver).move_to_element(label).click().perform() +ActionChains(self.driver).move_to_element(checkbox).click().perform() except WebDriverException as e: assert False, 'Can\'t click on checkbox label: %s \n%s' % (s, e) self.wait() -- 2.5.0 -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0009] WebUI tests fix
On 02/11/2016 11:13 AM, Lenka Doudova wrote: Hi all, most of webUI tests fail with AssertionError: Can't click on checkbox label: table.table Message: Element is not clickable at point (37, 340.338964844). Other element would receive the click: type="checkbox"> The problem seems to be that the tests attempt to click on a checkbox label that is no longer there. Since the checkbox is clickable directly, I changed the code accordingly. Most of the tests should now proceed successfully. Lenka Attaching updated patch with minor fix. Lenka From dac7d7bdbf1567c25f3fe7f44541f421d3405d59 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 10 Feb 2016 16:16:22 +0100 Subject: [PATCH] WebUI tests: fix failing tests --- ipatests/test_webui/ui_driver.py | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py index fc22f8612d49e300437eb91cb58add1a0376eb2c..ad3a9100f65f8ce64aaaf707df04a2bd411c3299 100644 --- a/ipatests/test_webui/ui_driver.py +++ b/ipatests/test_webui/ui_driver.py @@ -933,12 +933,8 @@ class UI_driver(object): s = self.get_table_selector(table_name) input_s = s + " tbody td input[value='%s']" % pkey checkbox = self.find(input_s, By.CSS_SELECTOR, parent, strict=True) -checkbox_id = checkbox.get_attribute('id') -label_s = s + " tbody td label[for='%s']" % checkbox_id -print(label_s) -label = self.find(label_s, By.CSS_SELECTOR, parent, strict=True) try: -ActionChains(self.driver).move_to_element(label).click().perform() +ActionChains(self.driver).move_to_element(checkbox).click().perform() except WebDriverException as e: assert False, 'Can\'t click on checkbox label: %s \n%s' % (s, e) self.wait() -- 2.5.0 -- 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
[Freeipa-devel] [TESTS][PATCH 0009] WebUI tests fix
Hi all, most of webUI tests fail with AssertionError: Can't click on checkbox label: table.table Message: Element is not clickable at point (37, 340.338964844). Other element would receive the click: The problem seems to be that the tests attempt to click on a checkbox label that is no longer there. Since the checkbox is clickable directly, I changed the code accordingly. Most of the tests should now proceed successfully. Lenka From c628f695cc1441b0bde7ec41bee811fbad5fd92e Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 10 Feb 2016 16:16:22 +0100 Subject: [PATCH] WebUI tests: fix failing tests --- ipatests/test_webui/ui_driver.py | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py index fc22f8612d49e300437eb91cb58add1a0376eb2c..1c6a852b823165a0bcdf33bb16b1939bf650958d 100644 --- a/ipatests/test_webui/ui_driver.py +++ b/ipatests/test_webui/ui_driver.py @@ -933,12 +933,9 @@ class UI_driver(object): s = self.get_table_selector(table_name) input_s = s + " tbody td input[value='%s']" % pkey checkbox = self.find(input_s, By.CSS_SELECTOR, parent, strict=True) -checkbox_id = checkbox.get_attribute('id') -label_s = s + " tbody td label[for='%s']" % checkbox_id -print(label_s) -label = self.find(label_s, By.CSS_SELECTOR, parent, strict=True) try: -ActionChains(self.driver).move_to_element(label).click().perform() +ActionChains(self.driver).move_to_element(checkbox)\ +.click().perform() except WebDriverException as e: assert False, 'Can\'t click on checkbox label: %s \n%s' % (s, e) self.wait() -- 2.5.0 -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0007] Multiple managers per user
Hi, everyone having time to take a look at this? It's been hanging for a few weeks. Thanks, Lenka On 12/15/2015 12:42 PM, Lenka Doudova wrote: Hi, I updated the (stage)user tests to reflect the multiple managers per user feature. Corresponding ticket: https://fedorahosted.org/freeipa/ticket/5344 Lenka -- 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
[Freeipa-devel] [TESTS][PATCH 0008] Fix tests for (stage)user plugin
Hi, this patch fixes few Tracker methods for staged and 'normal' user, which were mistakenly modified by my patch 0006.3. Applies for ipa-4-2 branch only. Lenka From f632687a338bd17f1b143fa72e040f4111510998 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 16 Dec 2015 13:44:28 +0100 Subject: [PATCH] Tests: Fix tests for (stage)user plugin Fixes tests for (stage)user plugin that were mistakenly modified in commit 75675f (patch 0006.3 by ldoudova). --- ipatests/test_xmlrpc/test_stageuser_plugin.py | 2 +- ipatests/test_xmlrpc/test_user_plugin.py | 4 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index ed5fc8b72a944e89a35e1960fee54852955d1a58..efbbab7e68f2ab1c6bd7a3182898aa13c46fad37 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -186,7 +186,7 @@ class StageUserTracker(Tracker): (self.kwargs[key].split('@'))[0].lower(), (self.kwargs[key].split('@'))[1])] elif key == u'manager': -self.attrs[key] = [self.kwargs[key]] +self.attrs[key] = [get_user_dn(self.kwargs[key])] elif key == u'ipasshpubkey': self.attrs[u'sshpubkeyfp'] = [sshpubkeyfp] self.attrs[key] = [self.kwargs[key]] diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index cce769f8f31fc05df218fd4f5afa8978bce091be..81185e449acaa127aa9429fff9587d39a2be81e6 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -1903,10 +1903,6 @@ class UserTracker(Tracker): summary=u'Stage user %s activated' % self.uid, result=self.filter_attrs(self.activate_keys)) -if 'manager' in expected['result']: -expected['result']['manager'] = [ -unicode(get_user_dn(expected['result']['manager'][0]))] - # work around to eliminate inconsistency in returned objectclass # (case sensitive assertion) expected['result']['objectclass'] = [item.lower() for item in -- 2.4.3 -- 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
[Freeipa-devel] [TESTS][PATCH 0007] Multiple managers per user
Hi, I updated the (stage)user tests to reflect the multiple managers per user feature. Corresponding ticket: https://fedorahosted.org/freeipa/ticket/5344 Lenka From 03b4673debf019562d00d0c0f4cfcf3f295fa612 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 14 Dec 2015 06:51:47 +0100 Subject: [PATCH] Tests: Multiple managers per user Adding Tracker methods and new tests to test multiple managers per (stage)user feature. https://fedorahosted.org/freeipa/ticket/5344 --- ipatests/test_xmlrpc/test_stageuser_plugin.py| 127 +++ ipatests/test_xmlrpc/test_user_plugin.py | 96 + ipatests/test_xmlrpc/tracker/stageuser_plugin.py | 97 + ipatests/test_xmlrpc/tracker/user_plugin.py | 117 - 4 files changed, 434 insertions(+), 3 deletions(-) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index 42ecf046884369bd74b03194937c425215b99f6c..e597acf0d32a2cf707d69eced655d5e34cb3a3be 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -673,3 +673,130 @@ class TestGroups(XMLRPC_test): command = group.make_add_member_command(options={u'user': user.uid}) result = command() group.check_add_member_negative(result) + + +@pytest.fixture(scope='class') +def stageduser_with_manager(request): +tracker = StageUserTracker(u'tuser1', u'test', u'user') +return tracker.make_fixture_activate(request) + + +@pytest.fixture(scope='class') +def manager1(request): +tracker = UserTracker(u'manager1', u'test', u'manager') +return tracker.make_fixture(request) + + +@pytest.fixture(scope='class') +def manager2(request): +tracker = UserTracker(u'manager2', u'test', u'manager') +return tracker.make_fixture(request) + + +@pytest.fixture(scope='class') +def user_activated_with_manager(request): +tracker = UserTracker(u'tuser1', u'test', u'user') +return tracker.make_fixture(request) + + +@pytest.mark.tier1 +class TestMultipleManagers(XMLRPC_test): +def test_add_manager( +self, stageduser_with_manager, manager1, manager2, +user_activated_with_manager): +stageduser_with_manager.ensure_exists() +manager1.ensure_exists() +manager2.ensure_exists() +stageduser_with_manager.add_manager( +managers=[manager1.uid, manager2.uid]) + +# verify managers are preserved after activation +user_activated_with_manager.activate(stageduser_with_manager) +user_activated_with_manager.delete() + +def test_add_duplicate_manager(self, stageduser, manager1): +stageduser.ensure_exists() +manager1.ensure_exists() +stageduser.add_manager( +managers=[manager1.uid, manager1.uid], +expected_fail={manager1.uid: u'already_manager'}) +stageduser.delete() + +def test_add_nonexistent_manager(self, stageduser, manager1): +stageduser.ensure_exists() +manager1.ensure_missing() +stageduser.add_manager( +managers=[manager1.uid], +expected_fail={manager1.uid: 'nonexistent'}) +stageduser.delete() + +def test_remove_manager(self, stageduser, manager1, manager2): +stageduser.ensure_exists() +manager1.ensure_exists() +manager2.ensure_exists() +stageduser.add_manager(managers=[manager1.uid, manager2.uid]) +stageduser.remove_manager(managers=[manager1.uid]) +stageduser.delete() + +def test_remove_nonmanager(self, stageduser, manager1): +stageduser.ensure_exists() +manager1.ensure_exists() +stageduser.remove_manager( +managers=[manager1.uid], +expected_fail={manager1.uid: u'not_manager'}) +stageduser.delete() + +def test_remove_nonexistent(self, stageduser, manager1): +stageduser.ensure_exists() +manager1.ensure_missing() +stageduser.remove_manager( +managers=[manager1.uid], +expected_fail=[manager1.uid]) +stageduser.delete() + +def test_show_multiple_managers(self, stageduser, manager1, manager2): +""" Verifies that multiple managers are listed properly +on 'stageuser-show' command """ +stageduser.ensure_exists() +manager1.ensure_exists() +manager2.ensure_exists() +stageduser.add_manager(managers=[manager1.uid, manager2.uid]) + +command = stageduser.make_retrieve_command() +result = command() +stageduser.check_retrieve(result) +stageduser.delete() + +def test_delete_all_managers(self, stageduser, manager1, manag
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 12/09/2015 10:13 AM, Martin Basti wrote: On 09.12.2015 09:41, Lenka Doudova wrote: Hi, attaching fixed patches for master and ipa-4-2 branch. Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387. Lenka On 11/20/2015 12:13 PM, Martin Babinsky wrote: On 11/19/2015 10:34 AM, Petr Viktorin wrote: On 11/19/2015 09:30 AM, Lenka Doudova wrote: On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture This looks like a better approach IMHO, you can then see which attribute/value was being checked. I would very much favor more descriptive test/fixture names in the beginning. Hello, we use usually bottom posting on freeipa-devel please try to keep reply in this way. Patch: I do not like the idea of separated lists, IMO it is hard to manage and is easy to create mistakes. How about this (untested, just from top of my head): http://fpaste.org/298994/65184014/ Martin Great idea, thanks. Fixed patches attached. Lenka From 646db80b7cc0912c310615d804fcb1561e19961f Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 23 Nov 2015 10:27:07 +0100 Subject: [PATCH] Adding descriptive IDs to stageuser tests Adding descriptive IDs to parametrized stageuser test for better identification of test cases. --- ipatests/test_xmlrpc/test_stageuser_plugin.py| 84 ++-- ipatests/test_xmlrpc/tracker/stageuser_plugin.py | 2 +- ipatests/test_xmlrpc/tracker/user_plugin.py | 9 ++- 3 files changed, 56 insertions(+), 39 deletions(-) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..42ecf046884369bd74b03194937c425215b99f6c 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -15,6 +15,7 @@ import pytest import six +from collections import OrderedDict from ipalib import api, errors from ipatests.test_xmlrpc import objectclasses @@ -53,35 +54,40 @@ sshpubkey = (u'ssh-rsa B3NzaC1yc2EDAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6X' sshpubkeyfp = (u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B ' 'public key test (ssh-rsa)') -options_ok = [ -{u'cn': u'name'}, -{u'initials': u'in'}, -{u'displayname': u'display'}, -{u'homedirectory': u'/home/homedi
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
Hi, attaching fixed patches for master and ipa-4-2 branch. Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387. Lenka On 11/20/2015 12:13 PM, Martin Babinsky wrote: On 11/19/2015 10:34 AM, Petr Viktorin wrote: On 11/19/2015 09:30 AM, Lenka Doudova wrote: On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture This looks like a better approach IMHO, you can then see which attribute/value was being checked. I would very much favor more descriptive test/fixture names in the beginning. From 163bbd04cf02d278b3d4ef8d0ee91878270e4377 Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Mon, 23 Nov 2015 10:27:07 +0100 Subject: [PATCH] Adding descriptive IDs to stageuser tests Adding descriptive IDs to parametrized stageuser test for better identification of test cases. --- ipatests/test_xmlrpc/test_stageuser_plugin.py| 30 ++-- ipatests/test_xmlrpc/tracker/stageuser_plugin.py | 2 +- ipatests/test_xmlrpc/tracker/user_plugin.py | 9 +-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..bda738b786f99296deaeb192e84965bb3e97cfea 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -83,6 +83,16 @@ options_ok = [ {u'random': True}, ] +options_ids = [ +'full name', 'initials', 'display name', 'home directory', 'GECOS', +'shell', 'email address', 'job title', 'kerberos principal', +'uppercase kerberos principal', 'street address', 'city', 'state', +'zip code', 'telephone number', 'fax number', 'mobile tel. number', +'pager number', 'organizational unit', 'car license', 'SSH key', +'manager', 'user ID number', 'group ID number', 'UID and GID numbers', +'password', 'random password' +] + @pytest.fixture(scope='class') def stageduser(request): @@ -90,13 +100,19 @@ def stageduser(request): return tracker.make_fixture(request) -@pytest.fixture(scope='class', params=options_ok) +@pytest.fixture(sco
Re: [Freeipa-devel] [patch 0025] Separated Tracker implementations into standalone package
NACK - there's a "typo" in /tracker/user_plugin.py, line 17-18: def get_user_dn(cn): return DN(('cn', cn), api.env.container_user, api.env.basedn) should be def get_user_dn(uid): return DN(('uid', uid), api.env.container_user, api.env.basedn) Some tests may fail because of that. Lenka On 11/20/2015 08:54 PM, Aleš Mareček wrote: Looks good. ACK. - Original Message - From: "Milan Kubík" To: "freeipa-devel" Cc: "Filip Skola" , "Ales Marecek" Sent: Friday, November 20, 2015 3:44:30 PM Subject: [patch 0025] Separated Tracker implementations into standalone package Fixes https://fedorahosted.org/freeipa/ticket/5467 Patches attached. -- 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
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Lenka -- 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
[Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka From a1ec552f27eeb05f5f9c41d59e076aa6ef7601db Mon Sep 17 00:00:00 2001 From: Lenka Doudova Date: Wed, 18 Nov 2015 09:11:29 +0100 Subject: [PATCH] Adding comments in stageuser plugin tests. Adding comments that should allow better orientation in the test_create_attr method. Original description was insufficient for identification of problem in case of failed tests. Adding comments in stageuser plugin tests Adding comments that should allow better orientation in the test_create_attr method. Original description was insufficient for identification of problem in case of failed tests. --- ipatests/test_xmlrpc/test_stageuser_plugin.py | 62 +++ 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index 43c59b7c7ec2902064fc363c66e98cc98e8b9e17..78244f30b57986a9d45a272f15458d438e5cb6b0 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -54,33 +54,33 @@ sshpubkeyfp = (u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B ' 'public key test (ssh-rsa)') options_ok = [ -{u'cn': u'name'}, -{u'initials': u'in'}, -{u'displayname': u'display'}, -{u'homedirectory': u'/home/homedir'}, -{u'gecos': u'gecos'}, -{u'loginshell': u'/bin/shell'}, -{u'mail': u'email@email.email'}, -{u'title': u'newbie'}, -{u'krbprincipalname': u'kerberos@%s' % api.env.realm}, -{u'krbprincipalname': u'KERBEROS@%s' % api.env.realm}, -{u'street': u'first street'}, -{u'l': u'prague'}, -{u'st': u'czech'}, -{u'postalcode': u'12345'}, -{u'telephonenumber': u'123456789'}, -{u'facsimiletelephonenumber': u'123456789'}, -{u'mobile': u'123456789'}, -{u'pager': u'123456789'}, -{u'ou': u'engineering'}, -{u'carlicense': u'abc1234'}, -{u'ipasshpubkey': sshpubkey}, -{u'manager': u'auser1'}, -{u'uidnumber': uid}, -{u'gidnumber': gid}, -{u'uidnumber': uid, u'gidnumber': gid}, -{u'userpassword': u'Secret123'}, -{u'random': True}, +{u'cn': u'name'}, # 1 +{u'initials': u'in'}, # 2 +{u'displayname': u'display'}, # 3 +{u'homedirectory': u'/home/homedir'}, # 4 +{u'gecos': u'gecos'}, # 5 +{u'loginshell': u'/bin/shell'}, # 6 +{u'mail': u'email@email.email'}, # 7 +{u'title': u'newbie'}, # 8 +{u'krbprincipalname': u'kerberos@%s' % api.env.realm}, # 9 +{u'krbprincipalname': u'KERBEROS@%s' % api.env.realm}, # 10 +{u'street': u'first street'}, # 11 +{u'l': u'prague'}, # 12 +{u'st': u'czech'}, # 13 +{u'postalcode': u'12345'}, # 14 +{u'telephonenumber': u'123456789'}, # 15 +{u'facsimiletelephonenumber': u'123456789'}, # 16 +{u'mobile': u'123456789'}, # 17 +{u'pager': u'123456789'}, # 18 +{u'ou': u'engineering'}, # 19 +{u'carlicense': u'abc1234'}, # 20 +{u'ipasshpubkey': sshpubkey}, # 21 +{u'manager': u'auser1'}, # 22 +{u'uidnumber': uid}, # 23 +{u'gidnumber': gid}, # 24 +{u'uidnumber': uid, u'gidnumber': gid}, # 25 +{u'userpassword': u'Secret123'}, # 26 +{u'random': True}, # 27 ] @@ -461,7 +461,13 @@ class TestStagedUser(XMLRPC_test): def test_create_attr(self, stageduser2, user, user6): """ Tests creating a user with various valid attributes listed -in 'options_ok' list""" +in 'options_ok' list +Should this test fail, a message like the following appears: +...::test_create_attr[stageduser2XX] FAILED +where the 'XX' specifies option with which the 'ipa stageuser-add' +command was performed. The options are listed and numbered +accordingly in 'options_ok' list. +""" # create staged user with specified parameters user.ensure_exists() # necessary for manager test stageduser2.ensure_missing() -- 2.4.3 -- 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