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')
-),
-
