1:56 p.m.
Hello Thierry,
layout OK.
As for tests - instead of reinventing the wheel by defing class Test_standAlone
to set up instance, use py.test fixture.
Also, you should not force setup, test, teardown execution for each test by
specifying sub-methods for each test. Testing framework (py.test) should be
doing that. I think this will make your tests fail badly if some exception
occurs - if _test_ticket47560_setup raises exception, it will propagate back to
py.test and cleanup method will never be executed for that ticket.
Also, I believe each ticket should have its own file which contains one or more
testcases. I think that would reasonably group relevant things together.
On 10/30/2013 05:57 PM, thierry bordaz wrote:
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
--
389-devel mailing list
389-devel(a)lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
2:11 p.m.
On 10/30/2013 12:56 PM, Jan Rusnacko wrote:
Hello Thierry,
layout OK.
As for tests - instead of reinventing the wheel by defing class Test_standAlone
to set up instance, use py.test fixture.
+1
Also, you should not force setup, test, teardown execution for each test by
specifying sub-methods for each test. Testing framework (py.test) should be
doing that. I think this will make your tests fail badly if some exception
occurs - if _test_ticket47560_setup raises exception, it will propagate back to
py.test and cleanup method will never be executed for that ticket.
+1
Also, I believe each ticket should have its own file which contains one or more
testcases. I think that would reasonably group relevant things together.
+1
On 10/30/2013 05:57 PM, thierry bordaz wrote:
>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>
>
>
> --
> 389-devel mailing list
> 389-devel(a)lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
--
389-devel mailing list
389-devel(a)lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
3:12 p.m.
Ok I realise it is not a good idea. I will reimplement it and send an
other review :-)
Thanks for all your feedbacks.
thierry
On 10/30/2013 08:11 PM, Rich Megginson wrote:
On 10/30/2013 12:56 PM, Jan Rusnacko wrote:
> Hello Thierry,
>
> layout OK.
>
> As for tests - instead of reinventing the wheel by defing class
> Test_standAlone
> to set up instance, use py.test fixture.
+1
>
> Also, you should not force setup, test, teardown execution for each
> test by
> specifying sub-methods for each test. Testing framework (py.test)
> should be
> doing that. I think this will make your tests fail badly if some
> exception
> occurs - if _test_ticket47560_setup raises exception, it will
> propagate back to
> py.test and cleanup method will never be executed for that ticket.
+1
>
> Also, I believe each ticket should have its own file which contains
> one or more
> testcases. I think that would reasonably group relevant things together.
+1
>
> On 10/30/2013 05:57 PM, thierry bordaz wrote:
>>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>>
>>
>>
>>
>> --
>> 389-devel mailing list
>> 389-devel(a)lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>
> --
> 389-devel mailing list
> 389-devel(a)lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
3:09 p.m.
On 10/30/2013 07:56 PM, Jan Rusnacko wrote:
Hello Thierry,
layout OK.
As for tests - instead of reinventing the wheel by defing class Test_standAlone
to set up instance, use py.test fixture.
Also, you should not force setup, test, teardown execution for each test by
specifying sub-methods for each test. Testing framework (py.test) should be
doing that. I think this will make your tests fail badly if some exception
occurs - if _test_ticket47560_setup raises exception, it will propagate back to
py.test and cleanup method will never be executed for that ticket.
Hi Jan,
thanks for you comments.
py.test will call for for each test_ticketxxx
setup
test_ticketxxx
teardown
setup will create the instance if it does not already exist, else it
provide a dirsrv to it.
teardown will remove the instance if the test_ticketxx is not able to
properly clean it up (if test_ticketxx raise the clean_please flag)
test_ticketxxx will start it executions with clean_please=true (set by
setup), if it succeeds it set clean_please=false. Unless it fails to
properly clean up the instance. In that case it sets clean_please to true.
What would be the advantages to make the setup/teardown sub-methods of
the test_ticketxx ?
At least a "small" drawback is that the person that write test_ticketxxx
will have to write them, instead of using the "standard" one.
Also, I believe each ticket should have its own file which contains one or more
testcases. I think that would reasonably group relevant things together.
Right, it was my first intention. But then I had this idea to group the
tickets per deployment module.
I don't know if it is a good idea, but it seems to be confusing :-[
On 10/30/2013 05:57 PM, thierry bordaz wrote:
>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>
>
>
> --
> 389-devel mailing list
> 389-devel(a)lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
3:13 p.m.
On 10/30/2013 02:09 PM, thierry bordaz wrote:
On 10/30/2013 07:56 PM, Jan Rusnacko wrote:
> Hello Thierry,
>
> layout OK.
>
> As for tests - instead of reinventing the wheel by defing class Test_standAlone
> to set up instance, use py.test fixture.
>
> Also, you should not force setup, test, teardown execution for each test by
> specifying sub-methods for each test. Testing framework (py.test) should be
> doing that. I think this will make your tests fail badly if some exception
> occurs - if _test_ticket47560_setup raises exception, it will propagate back to
> py.test and cleanup method will never be executed for that ticket.
Hi Jan,
thanks for you comments.
py.test will call for for each test_ticketxxx
setup
test_ticketxxx
teardown
setup will create the instance if it does not already exist, else it
provide a dirsrv to it.
teardown will remove the instance if the test_ticketxx is not able to
properly clean it up (if test_ticketxx raise the clean_please flag)
test_ticketxxx will start it executions with clean_please=true (set by
setup), if it succeeds it set clean_please=false. Unless it fails to
properly clean up the instance. In that case it sets clean_please to true.
What would be the advantages to make the setup/teardown sub-methods of
the test_ticketxx ?
At least a "small" drawback is that the person that write
test_ticketxxx will have to write them, instead of using the
"standard" one.
Can we have the test framework call a default setup/teardown method if
not provided by the test?
> Also, I believe each ticket should have its own file which contains one or more
> testcases. I think that would reasonably group relevant things together.
Right, it was my first intention. But then I had this idea to group
the tickets per deployment module.
I don't know if it is a good idea, but it seems to be confusing :-[
> On 10/30/2013 05:57 PM, thierry bordaz wrote:
>>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>>
>>
>>
>> --
>> 389-devel mailing list
>> 389-devel(a)lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>
--
389-devel mailing list
389-devel(a)lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-devel
5:27 a.m.
On 10/30/2013 07:56 PM, Jan Rusnacko wrote:
Hello Thierry,
layout OK.
As for tests - instead of reinventing the wheel by defing class Test_standAlone
to set up instance, use py.test fixture.
Also, you should not force setup, test, teardown execution for each test by
specifying sub-methods for each test. Testing framework (py.test) should be
doing that. I think this will make your tests fail badly if some exception
occurs - if _test_ticket47560_setup raises exception, it will propagate back to
py.test and cleanup method will never be executed for that ticket.
Hi Jan,
rereading your comments I realize I misunderstood your point...
Inside ticket method (e.g. test_ticket47560), the sub-methods (e.g.
_test_ticket47560_setup or _test_ticket47560_teardown) are not called by
py.test but by the body of ticket_method. I added them to separate more
clearly the initialization/cleanup phases of the test case from the real
test case. So we are not force to define those methods.
The important part is to set the 'clean_please=true' at the beginning of
the test case, so that if any uncaught exception occurs (and so the test
case can not cleanup all the initialization it did) the 'teardown'
method will remove the instance.
'clean_please' should be set to 'false' only when the test case
completed successfully its clean-up.
regards
thierry
Also, I believe each ticket should have its own file which contains one or more
testcases. I think that would reasonably group relevant things together.
On 10/30/2013 05:57 PM, thierry bordaz wrote:
>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>
>
>
> --
> 389-devel mailing list
> 389-devel(a)lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>
5:36 a.m.
On 10/31/2013 11:27 AM, thierry bordaz wrote:
On 10/30/2013 07:56 PM, Jan Rusnacko wrote:
> Hello Thierry,
>
> layout OK.
>
> As for tests - instead of reinventing the wheel by defing class Test_standAlone
> to set up instance, use py.test fixture.
>
> Also, you should not force setup, test, teardown execution for each test by
> specifying sub-methods for each test. Testing framework (py.test) should be
> doing that. I think this will make your tests fail badly if some exception
> occurs - if _test_ticket47560_setup raises exception, it will propagate back to
> py.test and cleanup method will never be executed for that ticket.
Hi Jan,
rereading your comments I realize I misunderstood your point...
Inside ticket method (e.g. test_ticket47560), the sub-methods (e.g.
_test_ticket47560_setup or _test_ticket47560_teardown) are not called by py.test
but by the body of ticket_method. I added them to separate more clearly the
initialization/cleanup phases of the test case from the real test case. So we
are not force to define those methods.
Hello Thierry,
I understand this. So if exception occurs anywhere in the testcase (whether in
sub-setup, test of sub-teardown), exception will propagate back to py.test,
which invokes global teardown in the end ?
This works assuming you only want to clean up DS instance. If any ticket would
ever create some temp file and want to remove it after the testcase runs, you
can either
1) put it in the sub-teardown and if the testcase fails, temp file will not be
cleaned up
2) clutter up global teardown method for DS instance, which means global
teardown is now taking care of cleaning up after testcases
Neither is perfect and you might very well end up with testcases with side effects.
I think you are unnecessarily building potential problems doing this.
The important part is to set the 'clean_please=true' at the beginning of the
test case, so that if any uncaught exception occurs (and so the test case can
not cleanup all the initialization it did) the 'teardown' method will remove the
instance.
'clean_please' should be set to 'false' only when the test case
completed
successfully its clean-up.
regards
thierry
>
> Also, I believe each ticket should have its own file which contains one or more
> testcases. I think that would reasonably group relevant things together.
>
> On 10/30/2013 05:57 PM, thierry bordaz wrote:
>>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>>
>>
>>
>>
>> --
>> 389-devel mailing list
>> 389-devel(a)lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>
6 a.m.
On 10/31/2013 11:36 AM, Jan Rusnacko wrote:
On 10/31/2013 11:27 AM, thierry bordaz wrote:
> On 10/30/2013 07:56 PM, Jan Rusnacko wrote:
>> Hello Thierry,
>>
>> layout OK.
>>
>> As for tests - instead of reinventing the wheel by defing class Test_standAlone
>> to set up instance, use py.test fixture.
>>
>> Also, you should not force setup, test, teardown execution for each test by
>> specifying sub-methods for each test. Testing framework (py.test) should be
>> doing that. I think this will make your tests fail badly if some exception
>> occurs - if _test_ticket47560_setup raises exception, it will propagate back to
>> py.test and cleanup method will never be executed for that ticket.
> Hi Jan,
>
> rereading your comments I realize I misunderstood your point...
>
> Inside ticket method (e.g. test_ticket47560), the sub-methods (e.g.
> _test_ticket47560_setup or _test_ticket47560_teardown) are not called by py.test
> but by the body of ticket_method. I added them to separate more clearly the
> initialization/cleanup phases of the test case from the real test case. So we
> are not force to define those methods.
Hello Thierry,
I understand this. So if exception occurs anywhere in the testcase (whether in
sub-setup, test of sub-teardown), exception will propagate back to py.test,
which invokes global teardown in the end ?
Yes. py.test will record the test as
FAILED, call teardown and call the
next test case
This works assuming you only want to clean up DS instance. If any ticket would
ever create some temp file and want to remove it after the testcase runs, you
can either
1) put it in the sub-teardown and if the testcase fails, temp file will not be
cleaned up
2) clutter up global teardown method for DS instance, which means global
teardown is now taking care of cleaning up after testcases
Yes. If a test case does not clean all its temporary components (because
of uncaught exception or lazy sub-teardown), these components will
remain. The global teardown will only recreate a new topology. Global
teardown could be enhanced to do additional cleanup but it will never
know all temporary created by all test case.
Neither is perfect and you might very well end up with testcases with side effects.
I think you are unnecessarily building potential problems doing this.
Yes, a test case may create side effects for the following test cases.
But I think this risk also exists with one separated file per ticket.
I agree that I need to go with a different approach (one ticket / one file).
regards
thierry
> The important part is to set the 'clean_please=true' at
the beginning of the
> test case, so that if any uncaught exception occurs (and so the test case can
> not cleanup all the initialization it did) the 'teardown' method will remove
the
> instance.
> 'clean_please' should be set to 'false' only when the test case
completed
> successfully its clean-up.
>
> regards
> thierry
>> Also, I believe each ticket should have its own file which contains one or more
>> testcases. I think that would reasonably group relevant things together.
>>
>> On 10/30/2013 05:57 PM, thierry bordaz wrote:
>>>
https://fedorahosted.org/389/attachment/ticket/47578/0001-Ticket-47578-CI...
>>>
>>>
>>>
>>>
>>> --
>>> 389-devel mailing list
>>> 389-devel(a)lists.fedoraproject.org
>>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>>
3829
days inactive
3830
days old
389-devel@lists.fedoraproject.org
8 comments
3 participants
participants (3)
-
Jan Rusnacko -
Rich Megginson -
thierry bordaz