Hello, ACK for code NACK for the placing "get_client_ip_with_hostmask" function to test_sudo.py (this function should be in some more general file)
----- Original Message ----- > From: "Oleg Fayans" <ofay...@redhat.com> > To: freeipa-devel@redhat.com > Sent: Thursday, December 3, 2015 3:32:46 PM > Subject: Re: [Freeipa-devel] [PATCH 0388] tests: Add hostmask detection for > sudo rules validating > > Hi Tomas, > > I would prefer get_client_ip_with_hostmask function to go to > ipatests/test_integration/tasks.py, and to be more generic: we probably > will need to run it against an arbitrary host. tasks.py is not the proper place as well > The rest looks fine > > On 12/03/2015 11:35 AM, Tomas Babej wrote: > > > > > > On 12/02/2015 05:25 PM, Lukas Slebodnik wrote: > >> On (02/12/15 15:41), Tomas Babej wrote: > >>> > >>> > >>> On 12/02/2015 09:24 AM, Tomas Babej wrote: > >>>> > >>>> > >>>> On 12/01/2015 06:27 PM, Tomas Babej wrote: > >>>>> > >>>>> > >>>>> On 11/30/2015 05:32 PM, Lukas Slebodnik wrote: > >>>>>> On (30/11/15 13:09), Tomas Babej wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> IPA sudo tests worked under the assumption that the clients that > >>>>>>> are executing the sudo commands have their IPs assigned within > >>>>>>> 255.255.255.0 hostmask. > >>>>>>> > >>>>>>> Removes this (invalid) assumption and adds a dynamic detection of > >>>>>>> the hostmask of the IPA client. > >>>>>>> > >>>>>>> https://fedorahosted.org/freeipa/ticket/5501 > >>>>>> > >>>>>> >From e6f1846f0d7d17303e5b30b1643651ba739b2b6c Mon Sep 17 00:00:00 > >>>>>> >2001 > >>>>>>> From: Tomas Babej <tba...@redhat.com> > >>>>>>> Date: Mon, 30 Nov 2015 12:53:39 +0100 > >>>>>>> Subject: [PATCH] tests: Add hostmask detection for sudo rules > >>>>>>> validating on > >>>>>>> hostmask > >>>>>>> > >>>>>>> IPA sudo tests worked under the assumption that the clients that > >>>>>>> are executing the sudo commands have their IPs assigned within > >>>>>>> 255.255.255.0 hostmask. > >>>>>>> > >>>>>>> Removes this (invalid) assumption and adds a dynamic detection of > >>>>>>> the hostmask of the IPA client. > >>>>>>> > >>>>> > >>>>> Thanks, updated patch attached. > >>>>> > >>>>> Tomas > >>>>> > >>>> > >>>> Actually, a small improvement is necessary. > >>>> > >>>> Updated patch attached. > >>>> > >>>> Tomas > >>>> > >>>> > >>>> > >>> > >>> Thanks to Lukas, we found another problem with the test. > >>> > >>> Updated patch attached. > >>> > >> Thank you for 4th revision of patch > >> but there is still one issue. > >> > >> =================================== FAILURES > >> =================================== > >> _________ TestSudo.test_sudo_rule_restricted_to_one_hostmask_negative > >> __________ > >> > >> self = <ipatests.test_integration.test_sudo.TestSudo object at > >> 0x7ff695f56890> > >> > >> def test_sudo_rule_restricted_to_one_hostmask_negative(self): > >> result1 = self.list_sudo_commands("testuser1") > >>> assert result1.returncode != 0 > >> E assert 0 != 0 > >> E + where 0 = <pytest_multihost.transport.SSHCommand object at > >> 0x7ff695f56bd0>.returncode > >> > >> test_integration/test_sudo.py:323: AssertionError > >> ==================== 1 failed, 74 passed in 807.35 seconds > >> ===================== > >> > >> LS > >> > > > > Here the test affected the negative test setup, which is fixed in the > > latest revision. > > > > Thanks, > > Tomas > > > > > > > > -- > Oleg Fayans > Quality Engineer > FreeIPA team > RedHat. > > -- > 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
