Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
On 08.03.2016 10:52, Milan Kubík wrote: On 02/22/2016 11:09 AM, Filip Skola wrote: - Original Message - On 02/10/2016 09:17 AM, Milan Kubík wrote: On 02/09/2016 04:19 PM, Milan Kubík wrote: On 01/28/2016 10:42 AM, Filip Skola wrote: - Original Message - On 01/25/2016 11:11 AM, Filip Skola wrote: - Original Message - On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- Milan Kubik Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). Filip Linter: * Module ipatests.test_xmlrpc.tracker.group_plugin W:100,26: Calling a dict.iter*() method (dict-iter-method) please use dict.items -- Milan Kubik Hi, sorry. This has been fixed in this patch. Filip ACK, thanks for the patience. :) Sorry, there are some other things I need clarified. NACK. Mail will follow later. What is the purpose of `make_fixture_detach` in your patches? They are not used anywhere and the finalizer does nothing. -- Milan Kubik Hi, none, I guess, probably a leftover copied from the tracker in the early days. Deleting the function. Filip Ack. Pushed to master: de63e16922c4f9926752016a2105bee4b974ba32 -- 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 0002] Refactor test_group_plugin
On 02/22/2016 11:09 AM, Filip Skola wrote: - Original Message - On 02/10/2016 09:17 AM, Milan Kubík wrote: On 02/09/2016 04:19 PM, Milan Kubík wrote: On 01/28/2016 10:42 AM, Filip Skola wrote: - Original Message - On 01/25/2016 11:11 AM, Filip Skola wrote: - Original Message - On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- Milan Kubik Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). Filip Linter: * Module ipatests.test_xmlrpc.tracker.group_plugin W:100,26: Calling a dict.iter*() method (dict-iter-method) please use dict.items -- Milan Kubik Hi, sorry. This has been fixed in this patch. Filip ACK, thanks for the patience. :) Sorry, there are some other things I need clarified. NACK. Mail will follow later. What is the purpose of `make_fixture_detach` in your patches? They are not used anywhere and the finalizer does nothing. -- Milan Kubik Hi, none, I guess, probably a leftover copied from the tracker in the early days. Deleting the function. Filip Ack. -- 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] [PATCH 0002] Refactor test_group_plugin
- Original Message - > On 02/10/2016 09:17 AM, Milan Kubík wrote: > > On 02/09/2016 04:19 PM, Milan Kubík wrote: > >> On 01/28/2016 10:42 AM, Filip Skola wrote: > >>> > >>> - Original Message - > >>>> On 01/25/2016 11:11 AM, Filip Skola wrote: > >>>>> - Original Message - > >>>>>> On 01/15/2016 03:38 PM, Filip Skola wrote: > >>>>>>> Hi, > >>>>>>> > >>>>>>> sending rebased patch. > >>>>>>> > >>>>>>> F. > >>>>>>> > >>>>>>> - Original Message - > >>>>>>>> Hello, > >>>>>>>> > >>>>>>>> sorry for delays. The patch no longer applies to master. Rebase > >>>>>>>> it, > >>>>>>>> please. > >>>>>>>> > >>>>>>>> Milan > >>>>>>>> > >>>>>>>> - Original Message - > >>>>>>>> From: "Filip Škola" > >>>>>>>> To: "Milan Kubík" > >>>>>>>> Cc: freeipa-devel@redhat.com > >>>>>>>> Sent: Wednesday, 9 December, 2015 7:01:02 PM > >>>>>>>> Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor > >>>>>>>> test_group_plugin > >>>>>>>> > >>>>>>>> On Mon, 7 Dec 2015 17:49:18 +0100 > >>>>>>>> Milan Kubík wrote: > >>>>>>>> > >>>>>>>>> On 12/03/2015 08:15 PM, Filip Škola wrote: > >>>>>>>>>> On Mon, 30 Nov 2015 17:18:30 +0100 > >>>>>>>>>> Milan Kubík wrote: > >>>>>>>>>> > >>>>>>>>>>> On 11/23/2015 04:42 PM, Filip Škola wrote: > >>>>>>>>>>>> Sending updated patch. > >>>>>>>>>>>> > >>>>>>>>>>>> F. > >>>>>>>>>>>> > >>>>>>>>>>>> On Mon, 23 Nov 2015 14:59:34 +0100 > >>>>>>>>>>>> Filip Škola wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> Found couple of issues (broke some dependencies). > >>>>>>>>>>>>> > >>>>>>>>>>>>> NACK > >>>>>>>>>>>>> > >>>>>>>>>>>>> F. > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Fri, 20 Nov 2015 13:56:36 +0100 > >>>>>>>>>>>>> Filip Škola wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Another one. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> F. > >>>>>>>>>>> Hi, the tests look good. Few remarks, though. > >>>>>>>>>>> > >>>>>>>>>>> 1. Please, use the shortes copyright notice in new modules. > >>>>>>>>>>> > >>>>>>>>>>> # > >>>>>>>>>>> # Copyright (C) 2015 FreeIPA Contributors see > >>>>>>>>>>> COPYING for > >>>>>>>>>>> license # > >>>>>>>>>>> > >>>>>>>>>>> 2. The tests `test_group_remove_group_from_protected_group` and > >>>>>>>>>>> `test_group_full_set_of_objectclass_not_available_post_detach` > >>>>>>>>>>> were not ported. Please, include them in the patch. > >>>>>>>>>>> > >>>>>>>>>>> Also, for less hassle, please rebase your patches on top of > >>>>>>>>>>> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > >>>>>>>>>>> > >>>>>>>>>>> Which changes the location of tracker implementations and > >>>>>>>>>>> preven
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
On 02/10/2016 09:17 AM, Milan Kubík wrote: On 02/09/2016 04:19 PM, Milan Kubík wrote: On 01/28/2016 10:42 AM, Filip Skola wrote: - Original Message - On 01/25/2016 11:11 AM, Filip Skola wrote: - Original Message - On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- Milan Kubik Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). Filip Linter: * Module ipatests.test_xmlrpc.tracker.group_plugin W:100,26: Calling a dict.iter*() method (dict-iter-method) please use dict.items -- Milan Kubik Hi, sorry. This has been fixed in this patch. Filip ACK, thanks for the patience. :) Sorry, there are some other things I need clarified. NACK. Mail will follow later. What is the purpose of `make_fixture_detach` in your patches? They are not used anywhere and the finalizer does nothing. -- 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] [PATCH 0002] Refactor test_group_plugin
On 02/09/2016 04:19 PM, Milan Kubík wrote: On 01/28/2016 10:42 AM, Filip Skola wrote: - Original Message - On 01/25/2016 11:11 AM, Filip Skola wrote: - Original Message - On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- Milan Kubik Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). Filip Linter: * Module ipatests.test_xmlrpc.tracker.group_plugin W:100,26: Calling a dict.iter*() method (dict-iter-method) please use dict.items -- Milan Kubik Hi, sorry. This has been fixed in this patch. Filip ACK, thanks for the patience. :) Sorry, there are some other things I need clarified. NACK. Mail will follow later. -- 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] [PATCH 0002] Refactor test_group_plugin
On 01/28/2016 10:42 AM, Filip Skola wrote: - Original Message - On 01/25/2016 11:11 AM, Filip Skola wrote: - Original Message - On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- Milan Kubik Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). Filip Linter: * Module ipatests.test_xmlrpc.tracker.group_plugin W:100,26: Calling a dict.iter*() method (dict-iter-method) please use dict.items -- Milan Kubik Hi, sorry. This has been fixed in this patch. Filip ACK, thanks for the patience. :) -- 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] [PATCH 0002] Refactor test_group_plugin
- Original Message - > On 01/25/2016 11:11 AM, Filip Skola wrote: > > > > - Original Message - > >> On 01/15/2016 03:38 PM, Filip Skola wrote: > >>> Hi, > >>> > >>> sending rebased patch. > >>> > >>> F. > >>> > >>> - Original Message - > >>>> Hello, > >>>> > >>>> sorry for delays. The patch no longer applies to master. Rebase it, > >>>> please. > >>>> > >>>> Milan > >>>> > >>>> - Original Message - > >>>> From: "Filip Škola" > >>>> To: "Milan Kubík" > >>>> Cc: freeipa-devel@redhat.com > >>>> Sent: Wednesday, 9 December, 2015 7:01:02 PM > >>>> Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin > >>>> > >>>> On Mon, 7 Dec 2015 17:49:18 +0100 > >>>> Milan Kubík wrote: > >>>> > >>>>> On 12/03/2015 08:15 PM, Filip Škola wrote: > >>>>>> On Mon, 30 Nov 2015 17:18:30 +0100 > >>>>>> Milan Kubík wrote: > >>>>>> > >>>>>>> On 11/23/2015 04:42 PM, Filip Škola wrote: > >>>>>>>> Sending updated patch. > >>>>>>>> > >>>>>>>> F. > >>>>>>>> > >>>>>>>> On Mon, 23 Nov 2015 14:59:34 +0100 > >>>>>>>> Filip Škola wrote: > >>>>>>>> > >>>>>>>>> Found couple of issues (broke some dependencies). > >>>>>>>>> > >>>>>>>>> NACK > >>>>>>>>> > >>>>>>>>> F. > >>>>>>>>> > >>>>>>>>> On Fri, 20 Nov 2015 13:56:36 +0100 > >>>>>>>>> Filip Škola wrote: > >>>>>>>>> > >>>>>>>>>> Another one. > >>>>>>>>>> > >>>>>>>>>> F. > >>>>>>> Hi, the tests look good. Few remarks, though. > >>>>>>> > >>>>>>> 1. Please, use the shortes copyright notice in new modules. > >>>>>>> > >>>>>>> # > >>>>>>> # Copyright (C) 2015 FreeIPA Contributors see COPYING for > >>>>>>> license # > >>>>>>> > >>>>>>> 2. The tests `test_group_remove_group_from_protected_group` and > >>>>>>> `test_group_full_set_of_objectclass_not_available_post_detach` > >>>>>>> were not ported. Please, include them in the patch. > >>>>>>> > >>>>>>> Also, for less hassle, please rebase your patches on top of > >>>>>>> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > >>>>>>> Which changes the location of tracker implementations and prevents > >>>>>>> circular imports. > >>>>>>> > >>>>>>> Thanks. > >>>>>>> > >>>>>> Hi, > >>>>>> > >>>>>> these cases are there, in corresponding classes. They are marked > >>>>>> with the original comments. (However I can move them to separate > >>>>>> class if desirable.) > >>>>>> > >>>>>> The copyright notice is changed. Also included a few changes in the > >>>>>> test with user without private group. > >>>>>> > >>>>>> Filip > >>>>> NACK > >>>>> > >>>>> linter: > >>>>> * Module tracker.group_plugin > >>>>> ipatests/test_xmlrpc/tracker/group_plugin.py:257: > >>>>> [E0102(function-redefined), GroupTracker.check_remove_member] method > >>>>> already defined line 253) > >>>>> > >>>>> Probably a leftover after the rebase made on top of my patch. Please > >>>>> fix it. You can check youch changes by make-lint script before > >>>>> sending them. > >>>>> > >>>>> Thanks > >>>>> > >>>> Hi, > >>>> > >&
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
On 01/25/2016 11:11 AM, Filip Skola wrote: - Original Message - On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- Milan Kubik Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). Filip Linter: * Module ipatests.test_xmlrpc.tracker.group_plugin W:100,26: Calling a dict.iter*() method (dict-iter-method) please use dict.items -- 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] [PATCH 0002] Refactor test_group_plugin
- Original Message - > On 01/15/2016 03:38 PM, Filip Skola wrote: > > Hi, > > > > sending rebased patch. > > > > F. > > > > - Original Message - > >> Hello, > >> > >> sorry for delays. The patch no longer applies to master. Rebase it, > >> please. > >> > >> Milan > >> > >> - Original Message - > >> From: "Filip Škola" > >> To: "Milan Kubík" > >> Cc: freeipa-devel@redhat.com > >> Sent: Wednesday, 9 December, 2015 7:01:02 PM > >> Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin > >> > >> On Mon, 7 Dec 2015 17:49:18 +0100 > >> Milan Kubík wrote: > >> > >>> On 12/03/2015 08:15 PM, Filip Škola wrote: > >>>> On Mon, 30 Nov 2015 17:18:30 +0100 > >>>> Milan Kubík wrote: > >>>> > >>>>> On 11/23/2015 04:42 PM, Filip Škola wrote: > >>>>>> Sending updated patch. > >>>>>> > >>>>>> F. > >>>>>> > >>>>>> On Mon, 23 Nov 2015 14:59:34 +0100 > >>>>>> Filip Škola wrote: > >>>>>> > >>>>>>> Found couple of issues (broke some dependencies). > >>>>>>> > >>>>>>> NACK > >>>>>>> > >>>>>>> F. > >>>>>>> > >>>>>>> On Fri, 20 Nov 2015 13:56:36 +0100 > >>>>>>> Filip Škola wrote: > >>>>>>> > >>>>>>>> Another one. > >>>>>>>> > >>>>>>>> F. > >>>>> Hi, the tests look good. Few remarks, though. > >>>>> > >>>>> 1. Please, use the shortes copyright notice in new modules. > >>>>> > >>>>># > >>>>># Copyright (C) 2015 FreeIPA Contributors see COPYING for > >>>>> license # > >>>>> > >>>>> 2. The tests `test_group_remove_group_from_protected_group` and > >>>>> `test_group_full_set_of_objectclass_not_available_post_detach` > >>>>> were not ported. Please, include them in the patch. > >>>>> > >>>>> Also, for less hassle, please rebase your patches on top of > >>>>> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > >>>>> Which changes the location of tracker implementations and prevents > >>>>> circular imports. > >>>>> > >>>>> Thanks. > >>>>> > >>>> > >>>> Hi, > >>>> > >>>> these cases are there, in corresponding classes. They are marked > >>>> with the original comments. (However I can move them to separate > >>>> class if desirable.) > >>>> > >>>> The copyright notice is changed. Also included a few changes in the > >>>> test with user without private group. > >>>> > >>>> Filip > >>> NACK > >>> > >>> linter: > >>> * Module tracker.group_plugin > >>> ipatests/test_xmlrpc/tracker/group_plugin.py:257: > >>> [E0102(function-redefined), GroupTracker.check_remove_member] method > >>> already defined line 253) > >>> > >>> Probably a leftover after the rebase made on top of my patch. Please > >>> fix it. You can check youch changes by make-lint script before > >>> sending them. > >>> > >>> Thanks > >>> > >> > >> Hi, > >> > >> I learned to use make-lint! > >> > >> Thanks, > >> F. > >> > Hello, > > NACK, pylint doesn't seem to like the way the fixtures are imported > (pytest does a lot of runtime magic) [1]. > One possible solution would be [2]. Though, I don't think this would be > a good idea in our environment. I suggest to create the fixtures on per > module basis. > > > [1]: http://fpaste.org/311949/53118942/ > [2]: > https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects > > -- > Milan Kubik > > Hi, the fixtures were copied into corresponding module. Please note that this patch has a dependence on my patch 0001 (user plugin). FilipFrom d0f1815a2df4a98354cdd73360fe8e861368c0f3 Mon
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
On 01/15/2016 03:38 PM, Filip Skola wrote: Hi, sending rebased patch. F. - Original Message - Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks Hi, I learned to use make-lint! Thanks, F. Hello, NACK, pylint doesn't seem to like the way the fixtures are imported (pytest does a lot of runtime magic) [1]. One possible solution would be [2]. Though, I don't think this would be a good idea in our environment. I suggest to create the fixtures on per module basis. [1]: http://fpaste.org/311949/53118942/ [2]: https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects -- 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] [PATCH 0002] Refactor test_group_plugin
Hi, sending rebased patch. F. - Original Message - > Hello, > > sorry for delays. The patch no longer applies to master. Rebase it, please. > > Milan > > - Original Message - > From: "Filip Škola" > To: "Milan Kubík" > Cc: freeipa-devel@redhat.com > Sent: Wednesday, 9 December, 2015 7:01:02 PM > Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin > > On Mon, 7 Dec 2015 17:49:18 +0100 > Milan Kubík wrote: > > > On 12/03/2015 08:15 PM, Filip Škola wrote: > > > On Mon, 30 Nov 2015 17:18:30 +0100 > > > Milan Kubík wrote: > > > > > >> On 11/23/2015 04:42 PM, Filip Škola wrote: > > >>> Sending updated patch. > > >>> > > >>> F. > > >>> > > >>> On Mon, 23 Nov 2015 14:59:34 +0100 > > >>> Filip Škola wrote: > > >>> > > >>>> Found couple of issues (broke some dependencies). > > >>>> > > >>>> NACK > > >>>> > > >>>> F. > > >>>> > > >>>> On Fri, 20 Nov 2015 13:56:36 +0100 > > >>>> Filip Škola wrote: > > >>>> > > >>>>> Another one. > > >>>>> > > >>>>> F. > > >>> > > >> Hi, the tests look good. Few remarks, though. > > >> > > >> 1. Please, use the shortes copyright notice in new modules. > > >> > > >> # > > >> # Copyright (C) 2015 FreeIPA Contributors see COPYING for > > >> license # > > >> > > >> 2. The tests `test_group_remove_group_from_protected_group` and > > >> `test_group_full_set_of_objectclass_not_available_post_detach` > > >> were not ported. Please, include them in the patch. > > >> > > >> Also, for less hassle, please rebase your patches on top of > > >> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > > >> Which changes the location of tracker implementations and prevents > > >> circular imports. > > >> > > >> Thanks. > > >> > > > > > > > > > Hi, > > > > > > these cases are there, in corresponding classes. They are marked > > > with the original comments. (However I can move them to separate > > > class if desirable.) > > > > > > The copyright notice is changed. Also included a few changes in the > > > test with user without private group. > > > > > > Filip > > NACK > > > > linter: > > * Module tracker.group_plugin > > ipatests/test_xmlrpc/tracker/group_plugin.py:257: > > [E0102(function-redefined), GroupTracker.check_remove_member] method > > already defined line 253) > > > > Probably a leftover after the rebase made on top of my patch. Please > > fix it. You can check youch changes by make-lint script before > > sending them. > > > > Thanks > > > > > Hi, > > I learned to use make-lint! > > Thanks, > F. > From 0f4585c1595cb0130c771d61f883c80a4349ff98 Mon Sep 17 00:00:00 2001 From: Filip Skola Date: Mon, 9 Nov 2015 16:48:55 +0100 Subject: [PATCH] Refactor test_group_plugin, use GroupTracker for tests --- ipatests/test_xmlrpc/test_group_plugin.py | 1738 + ipatests/test_xmlrpc/test_stageuser_plugin.py |4 +- ipatests/test_xmlrpc/tracker/group_plugin.py | 146 ++- 3 files changed, 735 insertions(+), 1153 deletions(-) diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index 6eb57c12f18d125de04beefa056f53b4caff1d64..ee672859376fcd2823907ed9d3ffc77943f1061a 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -1,6 +1,7 @@ # Authors: # Rob Crittenden # Pavel Zuna +# Filip Skola # # Copyright (C) 2008 Red Hat # see file 'COPYING' for use and warranty information @@ -23,1141 +24,646 @@ Test the `ipalib/plugins/group.py` module. import pytest -from ipalib import api, errors +from ipalib import errors from ipatests.test_xmlrpc import objectclasses from ipatests.test_xmlrpc.xmlrpc_test import ( -Declarative, -fuzzy_digits, -fuzzy_uuid, -fuzzy_set_ci, -add_sid, -add_oc) -from ipapython.dn import DN -from ipatests.test_xmlrpc.test_user_plugin import get_user_result +fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, add_oc, +XMLRPC_test, raises_exact +) +from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker +fro
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
Hello, sorry for delays. The patch no longer applies to master. Rebase it, please. Milan - Original Message - From: "Filip Škola" To: "Milan Kubík" Cc: freeipa-devel@redhat.com Sent: Wednesday, 9 December, 2015 7:01:02 PM Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: > On 12/03/2015 08:15 PM, Filip Škola wrote: > > On Mon, 30 Nov 2015 17:18:30 +0100 > > Milan Kubík wrote: > > > >> On 11/23/2015 04:42 PM, Filip Škola wrote: > >>> Sending updated patch. > >>> > >>> F. > >>> > >>> On Mon, 23 Nov 2015 14:59:34 +0100 > >>> Filip Škola wrote: > >>> > >>>> Found couple of issues (broke some dependencies). > >>>> > >>>> NACK > >>>> > >>>> F. > >>>> > >>>> On Fri, 20 Nov 2015 13:56:36 +0100 > >>>> Filip Škola wrote: > >>>> > >>>>> Another one. > >>>>> > >>>>> F. > >>> > >> Hi, the tests look good. Few remarks, though. > >> > >> 1. Please, use the shortes copyright notice in new modules. > >> > >> # > >> # Copyright (C) 2015 FreeIPA Contributors see COPYING for > >> license # > >> > >> 2. The tests `test_group_remove_group_from_protected_group` and > >> `test_group_full_set_of_objectclass_not_available_post_detach` > >> were not ported. Please, include them in the patch. > >> > >> Also, for less hassle, please rebase your patches on top of > >> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > >> Which changes the location of tracker implementations and prevents > >> circular imports. > >> > >> Thanks. > >> > > > > > > Hi, > > > > these cases are there, in corresponding classes. They are marked > > with the original comments. (However I can move them to separate > > class if desirable.) > > > > The copyright notice is changed. Also included a few changes in the > > test with user without private group. > > > > Filip > NACK > > linter: > * Module tracker.group_plugin > ipatests/test_xmlrpc/tracker/group_plugin.py:257: > [E0102(function-redefined), GroupTracker.check_remove_member] method > already defined line 253) > > Probably a leftover after the rebase made on top of my patch. Please > fix it. You can check youch changes by make-lint script before > sending them. > > Thanks > Hi, I learned to use make-lint! Thanks, F. -- 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 0002] Refactor test_group_plugin
On Mon, 7 Dec 2015 17:49:18 +0100 Milan Kubík wrote: > On 12/03/2015 08:15 PM, Filip Škola wrote: > > On Mon, 30 Nov 2015 17:18:30 +0100 > > Milan Kubík wrote: > > > >> On 11/23/2015 04:42 PM, Filip Škola wrote: > >>> Sending updated patch. > >>> > >>> F. > >>> > >>> On Mon, 23 Nov 2015 14:59:34 +0100 > >>> Filip Škola wrote: > >>> > Found couple of issues (broke some dependencies). > > NACK > > F. > > On Fri, 20 Nov 2015 13:56:36 +0100 > Filip Škola wrote: > > > Another one. > > > > F. > >>> > >> Hi, the tests look good. Few remarks, though. > >> > >> 1. Please, use the shortes copyright notice in new modules. > >> > >> # > >> # Copyright (C) 2015 FreeIPA Contributors see COPYING for > >> license # > >> > >> 2. The tests `test_group_remove_group_from_protected_group` and > >> `test_group_full_set_of_objectclass_not_available_post_detach` > >> were not ported. Please, include them in the patch. > >> > >> Also, for less hassle, please rebase your patches on top of > >> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > >> Which changes the location of tracker implementations and prevents > >> circular imports. > >> > >> Thanks. > >> > > > > > > Hi, > > > > these cases are there, in corresponding classes. They are marked > > with the original comments. (However I can move them to separate > > class if desirable.) > > > > The copyright notice is changed. Also included a few changes in the > > test with user without private group. > > > > Filip > NACK > > linter: > * Module tracker.group_plugin > ipatests/test_xmlrpc/tracker/group_plugin.py:257: > [E0102(function-redefined), GroupTracker.check_remove_member] method > already defined line 253) > > Probably a leftover after the rebase made on top of my patch. Please > fix it. You can check youch changes by make-lint script before > sending them. > > Thanks > Hi, I learned to use make-lint! Thanks, F. >From 2e231e285215818bbe1e06aeba573d43c86fab8b Mon Sep 17 00:00:00 2001 From: Filip Skola Date: Mon, 9 Nov 2015 16:48:55 +0100 Subject: [PATCH] Refactor test_group_plugin, use GroupTracker for tests --- ipatests/test_xmlrpc/test_group_plugin.py | 1728 + ipatests/test_xmlrpc/test_stageuser_plugin.py |4 +- ipatests/test_xmlrpc/tracker/group_plugin.py | 149 ++- 3 files changed, 736 insertions(+), 1145 deletions(-) diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index f2bd0f4b9c8d517500b63cf49a8a7bc7c29aab6e..1ac278c7e224b8dd8793dc56c299dcae88aa78a3 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -1,6 +1,7 @@ # Authors: # Rob Crittenden # Pavel Zuna +# Filip Skola # # Copyright (C) 2008 Red Hat # see file 'COPYING' for use and warranty information @@ -26,1137 +27,648 @@ import pytest from ipalib import api, errors from ipatests.test_xmlrpc import objectclasses -from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, - add_sid, add_oc, XMLRPC_test, raises_exact) +from ipatests.test_xmlrpc.xmlrpc_test import ( +fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, add_sid, add_oc, +XMLRPC_test, raises_exact +) from ipapython.dn import DN -from ipatests.test_xmlrpc.test_user_plugin import get_user_result +from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker -from ipatests.util import assert_deepequal +from ipatests.test_xmlrpc.test_user_plugin import user, user_npg2 +from ipatests.util import assert_deepequal, get_group_dn -group1 = u'testgroup1' -group2 = u'testgroup2' -group3 = u'testgroup3' -renamedgroup1 = u'testgroup' -user1 = u'tuser1' +notagroup = u'notagroup' +renamedgroup1 = u'renamedgroup' +invalidgroup1 = u'+tgroup1' +external_sid1 = u'S-1-1-123456-789-1' -invalidgroup1=u'+tgroup1' -# When adding external SID member to a group we can't test -# it fully due to possibly missing Samba 4 python bindings -# and/or not configured AD trusts. Thus, we'll use incorrect -# SID value to merely test that proper exceptions are raised -external_sid1=u'S-1-1-123456-789-1' +@pytest.fixture(scope='class') +def group(request): +tracker = GroupTracker(name=u'testgroup1', description=u'Test desc1') +return tracker.make_fixture(request) -def get_group_dn(cn): -return DN(('cn', cn), api.env.container_group, api.env.basedn) + +@pytest.fixture(scope='class') +def group2(request): +tracker = GroupTracker(name=u'testgroup2', description=u'Test desc2') +return tracker.make_fixture(request) + + +@pytest.fixture(scope='class') +def managed_group(request, user): +user.ensure_exists() +tracker = GroupTracker( +name=user.uid, description=u'User private group for %s' % user.uid +) +tracker.exists = True +
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
On 12/03/2015 08:15 PM, Filip Škola wrote: On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip NACK linter: * Module tracker.group_plugin ipatests/test_xmlrpc/tracker/group_plugin.py:257: [E0102(function-redefined), GroupTracker.check_remove_member] method already defined line 253) Probably a leftover after the rebase made on top of my patch. Please fix it. You can check youch changes by make-lint script before sending them. Thanks -- 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] [PATCH 0002] Refactor test_group_plugin
On Mon, 30 Nov 2015 17:18:30 +0100 Milan Kubík wrote: > On 11/23/2015 04:42 PM, Filip Škola wrote: > > Sending updated patch. > > > > F. > > > > On Mon, 23 Nov 2015 14:59:34 +0100 > > Filip Škola wrote: > > > >> Found couple of issues (broke some dependencies). > >> > >> NACK > >> > >> F. > >> > >> On Fri, 20 Nov 2015 13:56:36 +0100 > >> Filip Škola wrote: > >> > >>> Another one. > >>> > >>> F. > >> > > > > > > Hi, the tests look good. Few remarks, though. > > 1. Please, use the shortes copyright notice in new modules. > > # > # Copyright (C) 2015 FreeIPA Contributors see COPYING for > license # > > 2. The tests `test_group_remove_group_from_protected_group` and > `test_group_full_set_of_objectclass_not_available_post_detach` > were not ported. Please, include them in the patch. > > Also, for less hassle, please rebase your patches on top of > freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch > Which changes the location of tracker implementations and prevents > circular imports. > > Thanks. > Hi, these cases are there, in corresponding classes. They are marked with the original comments. (However I can move them to separate class if desirable.) The copyright notice is changed. Also included a few changes in the test with user without private group. Filip >From ff0b1fd07f15a076d5b370ff5299784b85e40af8 Mon Sep 17 00:00:00 2001 From: Filip Skola Date: Mon, 9 Nov 2015 16:48:55 +0100 Subject: [PATCH] Refactor test_group_plugin, use GroupTracker for tests --- ipatests/test_xmlrpc/test_group_plugin.py| 1728 +- ipatests/test_xmlrpc/tracker/group_plugin.py | 153 ++- 2 files changed, 739 insertions(+), 1142 deletions(-) diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index f2bd0f4b9c8d517500b63cf49a8a7bc7c29aab6e..1ac278c7e224b8dd8793dc56c299dcae88aa78a3 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -1,6 +1,7 @@ # Authors: # Rob Crittenden # Pavel Zuna +# Filip Skola # # Copyright (C) 2008 Red Hat # see file 'COPYING' for use and warranty information @@ -26,1137 +27,648 @@ import pytest from ipalib import api, errors from ipatests.test_xmlrpc import objectclasses -from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, - add_sid, add_oc, XMLRPC_test, raises_exact) +from ipatests.test_xmlrpc.xmlrpc_test import ( +fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, add_sid, add_oc, +XMLRPC_test, raises_exact +) from ipapython.dn import DN -from ipatests.test_xmlrpc.test_user_plugin import get_user_result +from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker -from ipatests.util import assert_deepequal +from ipatests.test_xmlrpc.test_user_plugin import user, user_npg2 +from ipatests.util import assert_deepequal, get_group_dn -group1 = u'testgroup1' -group2 = u'testgroup2' -group3 = u'testgroup3' -renamedgroup1 = u'testgroup' -user1 = u'tuser1' +notagroup = u'notagroup' +renamedgroup1 = u'renamedgroup' +invalidgroup1 = u'+tgroup1' +external_sid1 = u'S-1-1-123456-789-1' -invalidgroup1=u'+tgroup1' -# When adding external SID member to a group we can't test -# it fully due to possibly missing Samba 4 python bindings -# and/or not configured AD trusts. Thus, we'll use incorrect -# SID value to merely test that proper exceptions are raised -external_sid1=u'S-1-1-123456-789-1' +@pytest.fixture(scope='class') +def group(request): +tracker = GroupTracker(name=u'testgroup1', description=u'Test desc1') +return tracker.make_fixture(request) -def get_group_dn(cn): -return DN(('cn', cn), api.env.container_group, api.env.basedn) + +@pytest.fixture(scope='class') +def group2(request): +tracker = GroupTracker(name=u'testgroup2', description=u'Test desc2') +return tracker.make_fixture(request) + + +@pytest.fixture(scope='class') +def managed_group(request, user): +user.ensure_exists() +tracker = GroupTracker( +name=user.uid, description=u'User private group for %s' % user.uid +) +tracker.exists = True +# Managed group gets created when user is created +tracker.track_create() +return tracker + + +@pytest.fixture(scope='class') +def admins(request): +# Track the admins group +tracker = GroupTracker( +name=u'admins', description=u'Account administrators group' +) +tracker.exists = True +tracker.track_create() +tracker.attrs.update(member_user=[u'admin']) +return tracker + + +@pytest.fixture(scope='class') +def trustadmins(request): +# Track the 'trust admins' group +tracker = GroupTracker( +name=u'trust admins', description=u'Trusts administrators group' +) +tracker.exists = True +tracker.track_create() +tracker.attrs.update(member_user=[u'
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
On 11/23/2015 04:42 PM, Filip Škola wrote: Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: Another one. F. Hi, the tests look good. Few remarks, though. 1. Please, use the shortes copyright notice in new modules. # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 2. The tests `test_group_remove_group_from_protected_group` and `test_group_full_set_of_objectclass_not_available_post_detach` were not ported. Please, include them in the patch. Also, for less hassle, please rebase your patches on top of freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch Which changes the location of tracker implementations and prevents circular imports. Thanks. -- 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] [PATCH 0002] Refactor test_group_plugin
Sending updated patch. F. On Mon, 23 Nov 2015 14:59:34 +0100 Filip Škola wrote: > Found couple of issues (broke some dependencies). > > NACK > > F. > > On Fri, 20 Nov 2015 13:56:36 +0100 > Filip Škola wrote: > > > Another one. > > > > F. > > >From d6e30ee42ea427e9a2d5a85a787eddffd557eeba Mon Sep 17 00:00:00 2001 From: Filip Skola Date: Mon, 9 Nov 2015 16:48:55 +0100 Subject: [PATCH] Refactor test_group_plugin, use GroupTracker for tests --- ipatests/test_xmlrpc/test_group_plugin.py| 1881 +- ipatests/test_xmlrpc/tracker/__init__.py | 22 + ipatests/test_xmlrpc/tracker/group_plugin.py | 303 + 3 files changed, 927 insertions(+), 1279 deletions(-) create mode 100644 ipatests/test_xmlrpc/tracker/__init__.py create mode 100644 ipatests/test_xmlrpc/tracker/group_plugin.py diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index ed38c696e643a394510b6cbf7988f8c17520daf4..4350f128bad6306ac37492a8f5fdbb74b64478ab 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -1,6 +1,7 @@ # Authors: # Rob Crittenden # Pavel Zuna +# Filip Skola # # Copyright (C) 2008 Red Hat # see file 'COPYING' for use and warranty information @@ -26,1325 +27,647 @@ import pytest from ipalib import api, errors from ipatests.test_xmlrpc import objectclasses -from xmlrpc_test import (Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, - add_sid, add_oc, XMLRPC_test, raises_exact) +from ipatests.test_xmlrpc.xmlrpc_test import ( +fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, add_sid, add_oc, +XMLRPC_test, raises_exact +) from ipapython.dn import DN -from ipatests.test_xmlrpc.test_user_plugin import get_user_result -from ipatests.test_xmlrpc.ldaptracker import Tracker -from ipatests.test_xmlrpc.test_user_plugin import UserTracker +from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker +from ipatests.test_xmlrpc.test_user_plugin import UserTracker, user, user_npg from ipatests.util import assert_deepequal -group1 = u'testgroup1' -group2 = u'testgroup2' -group3 = u'testgroup3' -renamedgroup1 = u'testgroup' -user1 = u'tuser1' +notagroup = u'notagroup' +renamedgroup1 = u'renamedgroup' +invalidgroup1 = u'+tgroup1' +external_sid1 = u'S-1-1-123456-789-1' -invalidgroup1=u'+tgroup1' - -# When adding external SID member to a group we can't test -# it fully due to possibly missing Samba 4 python bindings -# and/or not configured AD trusts. Thus, we'll use incorrect -# SID value to merely test that proper exceptions are raised -external_sid1=u'S-1-1-123456-789-1' def get_group_dn(cn): return DN(('cn', cn), api.env.container_group, api.env.basedn) -@pytest.mark.tier1 -class test_group(Declarative): -cleanup_commands = [ -('group_del', [group1], {}), -('group_del', [group2], {}), -('group_del', [group3], {}), -('group_del', [renamedgroup1], {}), -('user_del', [user1], {}), -] - -tests = [ - - -# create group1: -dict( -desc='Try to retrieve non-existent %r' % group1, -command=('group_show', [group1], {}), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Try to update non-existent %r' % group1, -command=('group_mod', [group1], dict(description=u'Foo')), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Try to delete non-existent %r' % group1, -command=('group_del', [group1], {}), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Try to rename non-existent %r' % group1, -command=('group_mod', [group1], dict(setattr=u'cn=%s' % renamedgroup1)), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Create non-POSIX %r' % group1, -command=( -'group_add', [group1], dict(description=u'Test desc 1',nonposix=True) -), -expected=dict( -value=group1, -summary=u'Added group "testgroup1"', -result=dict( -cn=[group1], -description=[u'Test desc 1'], -objectclass=objectclasses.group, -ipauniqueid=[fuzzy_uuid], -dn=get_group_dn('testgroup1'), -), -), -), - - -dict( -desc='Try to create duplicate %r' % group1, -command=( -'group_add', [group1], dict(description=u'Test desc 1') -), -expected=errors.DuplicateEntry( -message=u'group with name "%s" already exists' % group1),
Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
Found couple of issues (broke some dependencies). NACK F. On Fri, 20 Nov 2015 13:56:36 +0100 Filip Škola wrote: > Another one. > > F. -- 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 0002] Refactor test_group_plugin
Another one. F. >From 0f0edcb1c7e32e24cf421e197657b8ac3a6a16a8 Mon Sep 17 00:00:00 2001 From: Filip Skola Date: Mon, 9 Nov 2015 16:48:55 +0100 Subject: [PATCH] Refactor test_group_plugin, use GroupTracker for tests --- ipatests/test_xmlrpc/test_group_plugin.py | 1896 +++-- 1 file changed, 742 insertions(+), 1154 deletions(-) diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index ed38c696e643a394510b6cbf7988f8c17520daf4..a37cabc9dc1e9a32c971e4dc973cc57dcedddb32 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -1,6 +1,7 @@ # Authors: # Rob Crittenden # Pavel Zuna +# Filip Skola # # Copyright (C) 2008 Red Hat # see file 'COPYING' for use and warranty information @@ -26,1147 +27,34 @@ import pytest from ipalib import api, errors from ipatests.test_xmlrpc import objectclasses -from xmlrpc_test import (Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, +from xmlrpc_test import (fuzzy_digits, fuzzy_uuid, fuzzy_set_ci, add_sid, add_oc, XMLRPC_test, raises_exact) from ipapython.dn import DN -from ipatests.test_xmlrpc.test_user_plugin import get_user_result from ipatests.test_xmlrpc.ldaptracker import Tracker -from ipatests.test_xmlrpc.test_user_plugin import UserTracker +from ipatests.test_xmlrpc.test_user_plugin import UserTracker, user, user_npg from ipatests.util import assert_deepequal -group1 = u'testgroup1' +notagroup = u'notagroup' group2 = u'testgroup2' group3 = u'testgroup3' -renamedgroup1 = u'testgroup' +renamedgroup1 = u'renamedgroup' user1 = u'tuser1' -invalidgroup1=u'+tgroup1' +invalidgroup1 = u'+tgroup1' + +external_sid1 = u'S-1-1-123456-789-1' -# When adding external SID member to a group we can't test -# it fully due to possibly missing Samba 4 python bindings -# and/or not configured AD trusts. Thus, we'll use incorrect -# SID value to merely test that proper exceptions are raised -external_sid1=u'S-1-1-123456-789-1' def get_group_dn(cn): return DN(('cn', cn), api.env.container_group, api.env.basedn) -@pytest.mark.tier1 -class test_group(Declarative): -cleanup_commands = [ -('group_del', [group1], {}), -('group_del', [group2], {}), -('group_del', [group3], {}), -('group_del', [renamedgroup1], {}), -('user_del', [user1], {}), -] - -tests = [ - - -# create group1: -dict( -desc='Try to retrieve non-existent %r' % group1, -command=('group_show', [group1], {}), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Try to update non-existent %r' % group1, -command=('group_mod', [group1], dict(description=u'Foo')), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Try to delete non-existent %r' % group1, -command=('group_del', [group1], {}), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Try to rename non-existent %r' % group1, -command=('group_mod', [group1], dict(setattr=u'cn=%s' % renamedgroup1)), -expected=errors.NotFound(reason=u'%s: group not found' % group1), -), - - -dict( -desc='Create non-POSIX %r' % group1, -command=( -'group_add', [group1], dict(description=u'Test desc 1',nonposix=True) -), -expected=dict( -value=group1, -summary=u'Added group "testgroup1"', -result=dict( -cn=[group1], -description=[u'Test desc 1'], -objectclass=objectclasses.group, -ipauniqueid=[fuzzy_uuid], -dn=get_group_dn('testgroup1'), -), -), -), - - -dict( -desc='Try to create duplicate %r' % group1, -command=( -'group_add', [group1], dict(description=u'Test desc 1') -), -expected=errors.DuplicateEntry( -message=u'group with name "%s" already exists' % group1), -), - - -dict( -desc='Retrieve non-POSIX %r' % group1, -command=('group_show', [group1], {}), -expected=dict( -value=group1, -summary=None, -result=dict( -cn=[group1], -description=[u'Test desc 1'], -dn=get_group_dn('testgroup1'), -), -), -), - - -dict( -desc='Updated non-POSIX %r' % group1, -command=( -'group_mod', [group1], dict(description=u'New desc 1') -), -