You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Noorul Islam K M <no...@collab.net> on 2011/02/21 09:14:35 UTC
[PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
This patch reduces checkout by around 23 times.
Log
[[[
Combine input validation tests to reduce the overhead of multiple checkouts.
* subversion/tests/cmdline/input_validation_tests.py:
(invalid_targets): Build sandbox once and make following methods local.
(invalid_wcpath_add, invalid_wcpath_changelist,
invalid_wcpath_cleanup, invalid_wcpath_commit, invalid_copy_sources,
invalid_copy_target, invalid_delete_targets, invalid_diff_targets,
invalid_export_targets, invalid_import_rags, invalid_log_targets,
invalid_merge_rags, invalid_wcpath_upgrade, invalid_resolve_targets,
invalid_resolved_targets, invalid_revert_targets,
invalid_lock_targets, invalid_unlock_targets,
invalid_status_targets, invalid_patch_targets,
invalid_switch_targets, invalid_relocate_targets,
invalid_mkdir_targets, invalid_update_targets)
(test_list): Add new combined test and remove individual tests.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Lieven Govaerts <sv...@mobsol.be>.
On Sun, Feb 27, 2011 at 3:38 PM, Noorul Islam K M <no...@collab.net> wrote:
> Lieven Govaerts <sv...@mobsol.be> writes:
>
> > On Sun, Feb 27, 2011 at 6:02 AM, Noorul Islam K M <no...@collab.net>
> wrote:
> >
> >> Noorul Islam K M <no...@collab.net> writes:
> >>
> >> Since svntest.main.run_tests already run "svnadmin create" and "svn
> >> import", in this particular case of input_validation_tests we don't need
> >> to create wc. So I passed create_wc = False to sbox.build() and
> >> everything works fine. So no overhead of repeated checkouts. Here is the
> >> patch.
> >
> >
> >
> >> Log
> >>
> >> [[[
> >>
> >> Pass "create_wc = False" to sbox.build() in order reduce the overhead of
> >> repeated checkouts.
> >>
> >> * subversion/tests/cmdline/input_validation_tests.py
> >> (invalid_wcpath_add, invalid_wcpath_changelist,
> >> invalid_wcpath_cleanup, invalid_wcpath_commit, invalid_copy_sources,
> >> invalid_copy_target, invalid_delete_targets, invalid_diff_targets,
> >> invalid_export_targets, invalid_import_rags, invalid_log_targets,
> >> invalid_merge_rags, invalid_wcpath_upgrade, invalid_resolve_targets,
> >> invalid_resolved_targets, invalid_revert_targets,
> >> invalid_lock_targets, invalid_unlock_targets, invalid_status_targets,
> >> invalid_patch_targets, invalid_switch_targets,
> >> invalid_relocate_targets, invalid_mkdir_targets,
> >> invalid_update_targets): Pass "create_wc = False" to sbox.build()
> >>
> >> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> >> ]]]
> >>
> >> Thanks and Regards
> >> Noorul
> >>
> >>
> >> Index: subversion/tests/cmdline/input_validation_tests.py
> >> ===================================================================
> >> --- subversion/tests/cmdline/input_validation_tests.py (revision
> 1074971)
> >> +++ subversion/tests/cmdline/input_validation_tests.py (working copy)
> >> @@ -66,13 +66,13 @@
> >>
> >> def invalid_wcpath_add(sbox):
> >> "non-working copy paths for 'add'"
> >> - sbox.build(read_only=True)
> >> + sbox.build(create_wc = False)
> >> for target in _invalid_wc_path_targets:
> >> run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'add',
> >> target)
> >>
> >>
> > I don't get this patch.
> >
> > The tests are supposed to be run from inside a working copy. After
> applying
> > this patch no wc isn't created, so they're not testing the same thing
> > anymore.
> >
> > If I apply this patch on a trunk wc, I get errors for all the tests:
> >
> > OSError: [Errno 2] No such file or directory:
> > 'svn-test-work/working_copies/input_validation_tests-17'
> > FAIL: input_validation_tests.py 17: wc paths and repo URL target mixture
> > for 'lock'
> >
> > [..]
> >
>
> I re-applied this patch and ran against trunk code. All tests pass. Can
> someone else try this patch please?
>
Did you clean up your svn-test-work folder before running the tests?
> You'll need at least one working copy, so either use the approach I've
> > outlined in a previous mail, or create one global sandbox and pass that
> in
> > the calls to run_and_verify_svn_in_wc.
> >
>
> I agree and I observed that svntest.main.run_tests runs "svnadmin
> create" and "svn import" command to create greek tree structure. Doesn't
> that create a working copy?
>
No. The test framework creates the greek tree on disc
in svn-test-work/local_tmp/greekfiles, then creates a new repository in
svn-test-work/local_tmp/repos (svnadmin create) and imports the greek tree
in that repository (svn import).
If you comment out the last lines of svntest/main.py, run the tests and then
open svn-test-work/local_tmp, you'll find the greek tree and repository, so
you can see for yourself.
Thanks and Regards
> Noorul
>
> > In fact, looking a bit more deeply in the test code, I think you should
> > leave the current one-wc-per-test behavior as is. Why? Because the tests
> > aren't guaranteed to be read only. Take for instance test
> > invalid_copy_target. If that copy operation would succeed - thus the test
> > fails - than the working copy gets modified. In other words, because of
> one
> > test is failing, other tests might fail too. That's not acceptable for a
> > test suite.
>
Lieven
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Lieven Govaerts <sv...@mobsol.be> writes:
> On Sun, Feb 27, 2011 at 6:02 AM, Noorul Islam K M <no...@collab.net> wrote:
>
>> Noorul Islam K M <no...@collab.net> writes:
>>
>> Since svntest.main.run_tests already run "svnadmin create" and "svn
>> import", in this particular case of input_validation_tests we don't need
>> to create wc. So I passed create_wc = False to sbox.build() and
>> everything works fine. So no overhead of repeated checkouts. Here is the
>> patch.
>
>
>
>> Log
>>
>> [[[
>>
>> Pass "create_wc = False" to sbox.build() in order reduce the overhead of
>> repeated checkouts.
>>
>> * subversion/tests/cmdline/input_validation_tests.py
>> (invalid_wcpath_add, invalid_wcpath_changelist,
>> invalid_wcpath_cleanup, invalid_wcpath_commit, invalid_copy_sources,
>> invalid_copy_target, invalid_delete_targets, invalid_diff_targets,
>> invalid_export_targets, invalid_import_rags, invalid_log_targets,
>> invalid_merge_rags, invalid_wcpath_upgrade, invalid_resolve_targets,
>> invalid_resolved_targets, invalid_revert_targets,
>> invalid_lock_targets, invalid_unlock_targets, invalid_status_targets,
>> invalid_patch_targets, invalid_switch_targets,
>> invalid_relocate_targets, invalid_mkdir_targets,
>> invalid_update_targets): Pass "create_wc = False" to sbox.build()
>>
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> ]]]
>>
>> Thanks and Regards
>> Noorul
>>
>>
>> Index: subversion/tests/cmdline/input_validation_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/input_validation_tests.py (revision 1074971)
>> +++ subversion/tests/cmdline/input_validation_tests.py (working copy)
>> @@ -66,13 +66,13 @@
>>
>> def invalid_wcpath_add(sbox):
>> "non-working copy paths for 'add'"
>> - sbox.build(read_only=True)
>> + sbox.build(create_wc = False)
>> for target in _invalid_wc_path_targets:
>> run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'add',
>> target)
>>
>>
> I don't get this patch.
>
> The tests are supposed to be run from inside a working copy. After applying
> this patch no wc isn't created, so they're not testing the same thing
> anymore.
>
> If I apply this patch on a trunk wc, I get errors for all the tests:
>
> OSError: [Errno 2] No such file or directory:
> 'svn-test-work/working_copies/input_validation_tests-17'
> FAIL: input_validation_tests.py 17: wc paths and repo URL target mixture
> for 'lock'
>
> [..]
>
I re-applied this patch and ran against trunk code. All tests pass. Can
someone else try this patch please?
> You'll need at least one working copy, so either use the approach I've
> outlined in a previous mail, or create one global sandbox and pass that in
> the calls to run_and_verify_svn_in_wc.
>
I agree and I observed that svntest.main.run_tests runs "svnadmin
create" and "svn import" command to create greek tree structure. Doesn't
that create a working copy?
Thanks and Regards
Noorul
> In fact, looking a bit more deeply in the test code, I think you should
> leave the current one-wc-per-test behavior as is. Why? Because the tests
> aren't guaranteed to be read only. Take for instance test
> invalid_copy_target. If that copy operation would succeed - thus the test
> fails - than the working copy gets modified. In other words, because of one
> test is failing, other tests might fail too. That's not acceptable for a
> test suite.
>
> Lieven
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Lieven Govaerts <sv...@mobsol.be>.
On Sun, Feb 27, 2011 at 6:02 AM, Noorul Islam K M <no...@collab.net> wrote:
> Noorul Islam K M <no...@collab.net> writes:
>
> Since svntest.main.run_tests already run "svnadmin create" and "svn
> import", in this particular case of input_validation_tests we don't need
> to create wc. So I passed create_wc = False to sbox.build() and
> everything works fine. So no overhead of repeated checkouts. Here is the
> patch.
> Log
>
> [[[
>
> Pass "create_wc = False" to sbox.build() in order reduce the overhead of
> repeated checkouts.
>
> * subversion/tests/cmdline/input_validation_tests.py
> (invalid_wcpath_add, invalid_wcpath_changelist,
> invalid_wcpath_cleanup, invalid_wcpath_commit, invalid_copy_sources,
> invalid_copy_target, invalid_delete_targets, invalid_diff_targets,
> invalid_export_targets, invalid_import_rags, invalid_log_targets,
> invalid_merge_rags, invalid_wcpath_upgrade, invalid_resolve_targets,
> invalid_resolved_targets, invalid_revert_targets,
> invalid_lock_targets, invalid_unlock_targets, invalid_status_targets,
> invalid_patch_targets, invalid_switch_targets,
> invalid_relocate_targets, invalid_mkdir_targets,
> invalid_update_targets): Pass "create_wc = False" to sbox.build()
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> ]]]
>
> Thanks and Regards
> Noorul
>
>
> Index: subversion/tests/cmdline/input_validation_tests.py
> ===================================================================
> --- subversion/tests/cmdline/input_validation_tests.py (revision 1074971)
> +++ subversion/tests/cmdline/input_validation_tests.py (working copy)
> @@ -66,13 +66,13 @@
>
> def invalid_wcpath_add(sbox):
> "non-working copy paths for 'add'"
> - sbox.build(read_only=True)
> + sbox.build(create_wc = False)
> for target in _invalid_wc_path_targets:
> run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'add',
> target)
>
>
I don't get this patch.
The tests are supposed to be run from inside a working copy. After applying
this patch no wc isn't created, so they're not testing the same thing
anymore.
If I apply this patch on a trunk wc, I get errors for all the tests:
OSError: [Errno 2] No such file or directory:
'svn-test-work/working_copies/input_validation_tests-17'
FAIL: input_validation_tests.py 17: wc paths and repo URL target mixture
for 'lock'
[..]
You'll need at least one working copy, so either use the approach I've
outlined in a previous mail, or create one global sandbox and pass that in
the calls to run_and_verify_svn_in_wc.
In fact, looking a bit more deeply in the test code, I think you should
leave the current one-wc-per-test behavior as is. Why? Because the tests
aren't guaranteed to be read only. Take for instance test
invalid_copy_target. If that copy operation would succeed - thus the test
fails - than the working copy gets modified. In other words, because of one
test is failing, other tests might fail too. That's not acceptable for a
test suite.
Lieven
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> Noorul Islam K M <no...@collab.net> writes:
>
>> Lieven Govaerts <sv...@mobsol.be> writes:
>>
>>> On Mon, Feb 21, 2011 at 11:32 AM, Noorul Islam K M <no...@collab.net>wrote:
>>>
>>>> Noorul Islam K M <no...@collab.net> writes:
>>>>
>>>> > Stefan Sperling <st...@elego.ed> writes:
>>>> >
>>>> >> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>>>> >>
>>>> >>>
>>>> >>> This patch reduces checkout by around 23 times.
>>>> >>
>>>> >> On my system the difference is 43 seconds vs. 30 seconds.
>>>> >>
>>>> >> We lose the ability to easily spot which of the subtest is failing
>>>> >> if we do this. I.e. instead of:
>>>> >>
>>>> >> ...
>>>> >> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
>>>> >> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
>>>> >> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
>>>> >> ...
>>>> >>
>>>> >> all we'd get is:
>>>> >>
>>>> >> FAIL: input_validation_tests.py 1: inavlid wc and url targets
>>>> >>
>>>> >> Is there a way of keeping these as individual tests but also
>>>> >> avoiding the overhead of creating a repository and a working copy?
>>>> >> If there isn't I would prefer to just leave this as it is now because
>>>> >> I prefer the current output.
>>>> >
>>>> > I think it will be possible by keeping sandbox global. I will modify and
>>>> > send an updated patch.
>>>> >
>>>>
>>>> I looked into it. I don't think it is straight forward. I will leave it
>>>> as such. As you said 13 seconds gain is no big deal.
>>>>
>>>
>>> So the tests need one read-only working copy for the whole suite, instead of
>>> one per test right?
>>>
>>
>> Yes you are right.
>>
>>> What about this:
>>> - add optional setup_suite_func and destroy_suite_func parameters to
>>> svntest.main.run_tests.
>>> - in run_tests, before running the tests, call setup_suite_func. After
>>> running the tests, call destroy_suite_func.
>>> -> this is a standard feature of any unit test framework.
>>>
>>> - in your setup_suite_func callback, checkout a working copy
>>> - in your destroy_suite_func callback, rmtree the working copy
>>>
>>> - in all tests, when creating the sandbox, pass the create_wc = false
>>> option.
>>> - in run_and_verify_svn_in_wc, use the new single working copy.
>>>
>>> This doesn't guarantee the working copy stays unmodified, but since the
>>> actual actions on the wc are centralized in one function
>>> (run_and_verify_svn_in_wc) it's easy to see this from the code. You could
>>> run "svn status" at the end of each test if you really want to be sure.
>>>
>>
>> I will explore this and let you know.
>>
>
> Before start working on this I would like to know whether everyone is
> fine with this approach.
>
> Thanks and Regards
> Noorul
Since svntest.main.run_tests already run "svnadmin create" and "svn
import", in this particular case of input_validation_tests we don't need
to create wc. So I passed create_wc = False to sbox.build() and
everything works fine. So no overhead of repeated checkouts. Here is the
patch.
Log
[[[
Pass "create_wc = False" to sbox.build() in order reduce the overhead of
repeated checkouts.
* subversion/tests/cmdline/input_validation_tests.py
(invalid_wcpath_add, invalid_wcpath_changelist,
invalid_wcpath_cleanup, invalid_wcpath_commit, invalid_copy_sources,
invalid_copy_target, invalid_delete_targets, invalid_diff_targets,
invalid_export_targets, invalid_import_rags, invalid_log_targets,
invalid_merge_rags, invalid_wcpath_upgrade, invalid_resolve_targets,
invalid_resolved_targets, invalid_revert_targets,
invalid_lock_targets, invalid_unlock_targets, invalid_status_targets,
invalid_patch_targets, invalid_switch_targets,
invalid_relocate_targets, invalid_mkdir_targets,
invalid_update_targets): Pass "create_wc = False" to sbox.build()
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Thanks and Regards
Noorul
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> Lieven Govaerts <sv...@mobsol.be> writes:
>
>> On Mon, Feb 21, 2011 at 11:32 AM, Noorul Islam K M <no...@collab.net>wrote:
>>
>>> Noorul Islam K M <no...@collab.net> writes:
>>>
>>> > Stefan Sperling <st...@elego.ed> writes:
>>> >
>>> >> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>>> >>
>>> >>>
>>> >>> This patch reduces checkout by around 23 times.
>>> >>
>>> >> On my system the difference is 43 seconds vs. 30 seconds.
>>> >>
>>> >> We lose the ability to easily spot which of the subtest is failing
>>> >> if we do this. I.e. instead of:
>>> >>
>>> >> ...
>>> >> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
>>> >> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
>>> >> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
>>> >> ...
>>> >>
>>> >> all we'd get is:
>>> >>
>>> >> FAIL: input_validation_tests.py 1: inavlid wc and url targets
>>> >>
>>> >> Is there a way of keeping these as individual tests but also
>>> >> avoiding the overhead of creating a repository and a working copy?
>>> >> If there isn't I would prefer to just leave this as it is now because
>>> >> I prefer the current output.
>>> >
>>> > I think it will be possible by keeping sandbox global. I will modify and
>>> > send an updated patch.
>>> >
>>>
>>> I looked into it. I don't think it is straight forward. I will leave it
>>> as such. As you said 13 seconds gain is no big deal.
>>>
>>
>> So the tests need one read-only working copy for the whole suite, instead of
>> one per test right?
>>
>
> Yes you are right.
>
>> What about this:
>> - add optional setup_suite_func and destroy_suite_func parameters to
>> svntest.main.run_tests.
>> - in run_tests, before running the tests, call setup_suite_func. After
>> running the tests, call destroy_suite_func.
>> -> this is a standard feature of any unit test framework.
>>
>> - in your setup_suite_func callback, checkout a working copy
>> - in your destroy_suite_func callback, rmtree the working copy
>>
>> - in all tests, when creating the sandbox, pass the create_wc = false
>> option.
>> - in run_and_verify_svn_in_wc, use the new single working copy.
>>
>> This doesn't guarantee the working copy stays unmodified, but since the
>> actual actions on the wc are centralized in one function
>> (run_and_verify_svn_in_wc) it's easy to see this from the code. You could
>> run "svn status" at the end of each test if you really want to be sure.
>>
>
> I will explore this and let you know.
>
Before start working on this I would like to know whether everyone is
fine with this approach.
Thanks and Regards
Noorul
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Lieven Govaerts <sv...@mobsol.be> writes:
> On Mon, Feb 21, 2011 at 11:32 AM, Noorul Islam K M <no...@collab.net>wrote:
>
>> Noorul Islam K M <no...@collab.net> writes:
>>
>> > Stefan Sperling <st...@elego.ed> writes:
>> >
>> >> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>> >>
>> >>>
>> >>> This patch reduces checkout by around 23 times.
>> >>
>> >> On my system the difference is 43 seconds vs. 30 seconds.
>> >>
>> >> We lose the ability to easily spot which of the subtest is failing
>> >> if we do this. I.e. instead of:
>> >>
>> >> ...
>> >> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
>> >> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
>> >> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
>> >> ...
>> >>
>> >> all we'd get is:
>> >>
>> >> FAIL: input_validation_tests.py 1: inavlid wc and url targets
>> >>
>> >> Is there a way of keeping these as individual tests but also
>> >> avoiding the overhead of creating a repository and a working copy?
>> >> If there isn't I would prefer to just leave this as it is now because
>> >> I prefer the current output.
>> >
>> > I think it will be possible by keeping sandbox global. I will modify and
>> > send an updated patch.
>> >
>>
>> I looked into it. I don't think it is straight forward. I will leave it
>> as such. As you said 13 seconds gain is no big deal.
>>
>
> So the tests need one read-only working copy for the whole suite, instead of
> one per test right?
>
Yes you are right.
> What about this:
> - add optional setup_suite_func and destroy_suite_func parameters to
> svntest.main.run_tests.
> - in run_tests, before running the tests, call setup_suite_func. After
> running the tests, call destroy_suite_func.
> -> this is a standard feature of any unit test framework.
>
> - in your setup_suite_func callback, checkout a working copy
> - in your destroy_suite_func callback, rmtree the working copy
>
> - in all tests, when creating the sandbox, pass the create_wc = false
> option.
> - in run_and_verify_svn_in_wc, use the new single working copy.
>
> This doesn't guarantee the working copy stays unmodified, but since the
> actual actions on the wc are centralized in one function
> (run_and_verify_svn_in_wc) it's easy to see this from the code. You could
> run "svn status" at the end of each test if you really want to be sure.
>
I will explore this and let you know.
Thanks and Regards
Noorul
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Lieven Govaerts <sv...@mobsol.be>.
On Mon, Feb 21, 2011 at 11:32 AM, Noorul Islam K M <no...@collab.net>wrote:
> Noorul Islam K M <no...@collab.net> writes:
>
> > Stefan Sperling <st...@elego.ed> writes:
> >
> >> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
> >>
> >>>
> >>> This patch reduces checkout by around 23 times.
> >>
> >> On my system the difference is 43 seconds vs. 30 seconds.
> >>
> >> We lose the ability to easily spot which of the subtest is failing
> >> if we do this. I.e. instead of:
> >>
> >> ...
> >> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
> >> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
> >> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
> >> ...
> >>
> >> all we'd get is:
> >>
> >> FAIL: input_validation_tests.py 1: inavlid wc and url targets
> >>
> >> Is there a way of keeping these as individual tests but also
> >> avoiding the overhead of creating a repository and a working copy?
> >> If there isn't I would prefer to just leave this as it is now because
> >> I prefer the current output.
> >
> > I think it will be possible by keeping sandbox global. I will modify and
> > send an updated patch.
> >
>
> I looked into it. I don't think it is straight forward. I will leave it
> as such. As you said 13 seconds gain is no big deal.
>
So the tests need one read-only working copy for the whole suite, instead of
one per test right?
What about this:
- add optional setup_suite_func and destroy_suite_func parameters to
svntest.main.run_tests.
- in run_tests, before running the tests, call setup_suite_func. After
running the tests, call destroy_suite_func.
-> this is a standard feature of any unit test framework.
- in your setup_suite_func callback, checkout a working copy
- in your destroy_suite_func callback, rmtree the working copy
- in all tests, when creating the sandbox, pass the create_wc = false
option.
- in run_and_verify_svn_in_wc, use the new single working copy.
This doesn't guarantee the working copy stays unmodified, but since the
actual actions on the wc are centralized in one function
(run_and_verify_svn_in_wc) it's easy to see this from the code. You could
run "svn status" at the end of each test if you really want to be sure.
regards,
L
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
checkouts.
Posted by Branko Čibej <br...@e-reka.si>.
On 21.02.2011 11:32, Noorul Islam K M wrote:
> Noorul Islam K M <no...@collab.net> writes:
>
>> Stefan Sperling <st...@elego.ed> writes:
>>
>>> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>>>
>>>> This patch reduces checkout by around 23 times.
>>> On my system the difference is 43 seconds vs. 30 seconds.
>>>
>>> We lose the ability to easily spot which of the subtest is failing
>>> if we do this. I.e. instead of:
>>>
>>> ...
>>> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
>>> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
>>> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
>>> ...
>>>
>>> all we'd get is:
>>>
>>> FAIL: input_validation_tests.py 1: inavlid wc and url targets
>>>
>>> Is there a way of keeping these as individual tests but also
>>> avoiding the overhead of creating a repository and a working copy?
>>> If there isn't I would prefer to just leave this as it is now because
>>> I prefer the current output.
>> I think it will be possible by keeping sandbox global. I will modify and
>> send an updated patch.
>>
> I looked into it. I don't think it is straight forward. I will leave it
> as such. As you said 13 seconds gain is no big deal.
Given that the one commandment for writing test cases is to make them
independent of each other, I can hardly see how you could keep a global
sandbox. The most important "optimization" for tests is to make them
more reliable, testcase performance comes third on the list at best
(after comprehensive).
-- Brane
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> Stefan Sperling <st...@elego.ed> writes:
>
>> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>>
>>>
>>> This patch reduces checkout by around 23 times.
>>
>> On my system the difference is 43 seconds vs. 30 seconds.
>>
>> We lose the ability to easily spot which of the subtest is failing
>> if we do this. I.e. instead of:
>>
>> ...
>> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
>> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
>> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
>> ...
>>
>> all we'd get is:
>>
>> FAIL: input_validation_tests.py 1: inavlid wc and url targets
>>
>> Is there a way of keeping these as individual tests but also
>> avoiding the overhead of creating a repository and a working copy?
>> If there isn't I would prefer to just leave this as it is now because
>> I prefer the current output.
>
> I think it will be possible by keeping sandbox global. I will modify and
> send an updated patch.
>
I looked into it. I don't think it is straight forward. I will leave it
as such. As you said 13 seconds gain is no big deal.
Thanks and Regards
Noorul
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Stefan Sperling <st...@elego.ed> writes:
> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>
>>
>> This patch reduces checkout by around 23 times.
>
> On my system the difference is 43 seconds vs. 30 seconds.
>
> We lose the ability to easily spot which of the subtest is failing
> if we do this. I.e. instead of:
>
> ...
> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
> ...
>
> all we'd get is:
>
> FAIL: input_validation_tests.py 1: inavlid wc and url targets
>
> Is there a way of keeping these as individual tests but also
> avoiding the overhead of creating a repository and a working copy?
> If there isn't I would prefer to just leave this as it is now because
> I prefer the current output.
I think it will be possible by keeping sandbox global. I will modify and
send an updated patch.
Thanks and Regards
Noorul
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
Johan Corveleyn <jc...@gmail.com> writes:
> On Mon, Feb 21, 2011 at 1:50 PM, Noorul Islam K M <no...@collab.net> wrote:
>
>> "Bert Huijben" <be...@qqmail.nl> writes:
>>
>>>> -----Original Message-----
>>>> From: Branko Čibej [mailto:brane@xbc.nu] On Behalf Of Branko Cibej
>>>> Sent: maandag 21 februari 2011 13:19
>>>> To: Philip Martin
>>>> Cc: dev@subversion.apache.org
>>>> Subject: Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
>>>> checkouts.
>>>
>>>> You could argue that tiny tests that do not modify a sandbox can be
>>>> merged together. Of course, if there's a bug in the implementation and
>>>> one of those subtests /does/ modify the sandbox, causing another down
>>>> the line to fail for no obvious reason, that hardly makes life easier
>>>> for the poor sod who has to analyse that mess.
>>>
>>> Wasn't there some option to ask for a read-only sandbox?
>>>
>>
>> On trunk the test already use read-only option. This patch reduces
>> number of checkouts. All these tests require only read-only working
>> copy.
>
> There's another way to make these tests go faster: we simply have to
> make checkout faster :-). As in: "blazingly fast" ...
>
> Anyone wants to take on this bite-sized task? ;-)
>
> Cheers,
I assume this patch has been rejected.
Thanks and Regards
Noorul
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, Feb 21, 2011 at 1:50 PM, Noorul Islam K M <no...@collab.net> wrote:
> "Bert Huijben" <be...@qqmail.nl> writes:
>
>>> -----Original Message-----
>>> From: Branko Čibej [mailto:brane@xbc.nu] On Behalf Of Branko Cibej
>>> Sent: maandag 21 februari 2011 13:19
>>> To: Philip Martin
>>> Cc: dev@subversion.apache.org
>>> Subject: Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
>>> checkouts.
>>
>>> You could argue that tiny tests that do not modify a sandbox can be
>>> merged together. Of course, if there's a bug in the implementation and
>>> one of those subtests /does/ modify the sandbox, causing another down
>>> the line to fail for no obvious reason, that hardly makes life easier
>>> for the poor sod who has to analyse that mess.
>>
>> Wasn't there some option to ask for a read-only sandbox?
>>
>
> On trunk the test already use read-only option. This patch reduces
> number of checkouts. All these tests require only read-only working
> copy.
There's another way to make these tests go faster: we simply have to
make checkout faster :-). As in: "blazingly fast" ...
Anyone wants to take on this bite-sized task? ;-)
Cheers,
--
Johan
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Noorul Islam K M <no...@collab.net>.
"Bert Huijben" <be...@qqmail.nl> writes:
>> -----Original Message-----
>> From: Branko Čibej [mailto:brane@xbc.nu] On Behalf Of Branko Cibej
>> Sent: maandag 21 februari 2011 13:19
>> To: Philip Martin
>> Cc: dev@subversion.apache.org
>> Subject: Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
>> checkouts.
>
>> You could argue that tiny tests that do not modify a sandbox can be
>> merged together. Of course, if there's a bug in the implementation and
>> one of those subtests /does/ modify the sandbox, causing another down
>> the line to fail for no obvious reason, that hardly makes life easier
>> for the poor sod who has to analyse that mess.
>
> Wasn't there some option to ask for a read-only sandbox?
>
On trunk the test already use read-only option. This patch reduces
number of checkouts. All these tests require only read-only working
copy.
Thanks and Regards
Noorul
RE: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@xbc.nu] On Behalf Of Branko Cibej
> Sent: maandag 21 februari 2011 13:19
> To: Philip Martin
> Cc: dev@subversion.apache.org
> Subject: Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
> checkouts.
> You could argue that tiny tests that do not modify a sandbox can be
> merged together. Of course, if there's a bug in the implementation and
> one of those subtests /does/ modify the sandbox, causing another down
> the line to fail for no obvious reason, that hardly makes life easier
> for the poor sod who has to analyse that mess.
Wasn't there some option to ask for a read-only sandbox?
Bert
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
checkouts.
Posted by Branko Čibej <br...@e-reka.si>.
On 21.02.2011 13:12, Philip Martin wrote:
> Branko Čibej <br...@e-reka.si> writes:
>
>> We should not be optimising tests for performance over clarity, ever. In
>> other words -- don't combine them. Anyone who has trouble with tests
>> taking too long can use a RAMdisk, --bdb-txn-nosync, and other tweaks to
>> make the tests run faster.
> I do that. The time to run the testsuite keeps growing.
>
>> I think you've all gone way off track here. Reality check, please? What
>> is the purpose of tests? To validate the behaviour of the code, or to
>> make developers' life easier?
> To make it easy for developers to validate the code :)
>
> Like all code it requires a judgement about how it should be organised.
> I agree that complicated tests should be separate, but I think checking
> that:
>
> "svn revert foo"
>
> and
>
> "svn resoved foo"
>
> both report "foo is not a local path" doesn't require two completely
> separate tests.
>
> How far do you think we should go? A dozen separate tests for revert
> alone:
>
> "svn revert versioned_and_modified"
> "svn revert versioned_and_unmodified"
> "svn revert url"
> "svn revert unversioned"
> "svn revert versioned_and_modified versioned_and_unmodified"
> "svn revert versioned_and_unmodified unversioned"
> etc
It's certainly easier to read the test output that way. :)
You could argue that tiny tests that do not modify a sandbox can be
merged together. Of course, if there's a bug in the implementation and
one of those subtests /does/ modify the sandbox, causing another down
the line to fail for no obvious reason, that hardly makes life easier
for the poor sod who has to analyse that mess.
There's a balancing act here. Talk about a "global sandbox" reused by
several test cases definitely makes my hair stand on end and I do
recommend against that. Combining several small tests into a single
testcase is a different and slightly more palatable approach, as long as
everyone agrees to be the poor sod that has to analyse the fallout. :)
-- Brane
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Philip Martin <ph...@wandisco.com>.
Branko Čibej <br...@e-reka.si> writes:
> We should not be optimising tests for performance over clarity, ever. In
> other words -- don't combine them. Anyone who has trouble with tests
> taking too long can use a RAMdisk, --bdb-txn-nosync, and other tweaks to
> make the tests run faster.
I do that. The time to run the testsuite keeps growing.
> I think you've all gone way off track here. Reality check, please? What
> is the purpose of tests? To validate the behaviour of the code, or to
> make developers' life easier?
To make it easy for developers to validate the code :)
Like all code it requires a judgement about how it should be organised.
I agree that complicated tests should be separate, but I think checking
that:
"svn revert foo"
and
"svn resoved foo"
both report "foo is not a local path" doesn't require two completely
separate tests.
How far do you think we should go? A dozen separate tests for revert
alone:
"svn revert versioned_and_modified"
"svn revert versioned_and_unmodified"
"svn revert url"
"svn revert unversioned"
"svn revert versioned_and_modified versioned_and_unmodified"
"svn revert versioned_and_unmodified unversioned"
etc
--
Philip
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
checkouts.
Posted by Branko Čibej <br...@e-reka.si>.
On 21.02.2011 12:04, Philip Martin wrote:
> Stefan Sperling <st...@elego.de> writes:
>
>> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>>> This patch reduces checkout by around 23 times.
>> On my system the difference is 43 seconds vs. 30 seconds.
> On my low-end Linux desktop it's 7.5 seconds and 3.5 seconds, run
> sequentially on a SATA disk.
>
>> We lose the ability to easily spot which of the subtest is failing
>> if we do this. I.e. instead of:
>>
>> ...
>> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
>> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
>> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
>> ...
>>
>> all we'd get is:
>>
>> FAIL: input_validation_tests.py 1: inavlid wc and url targets
> When a test fails the first thing I do is look in tests.log, that will
> still work just as well with the combined test. I suppose we do lose
> out if there are multiple independent bugs, as the test will stop at the
> first one and not report the others.
>
> I feel we should be optimising for the common case where the tests PASS,
> and that optimising for FAILs is the wrong approach.
>
> I agree that combining big, complex tests, like merge, is a bad idea.
> But for relatively trivial tests I think reduced runtime is more
> important.
We should not be optimising tests for performance over clarity, ever. In
other words -- don't combine them. Anyone who has trouble with tests
taking too long can use a RAMdisk, --bdb-txn-nosync, and other tweaks to
make the tests run faster.
I think you've all gone way off track here. Reality check, please? What
is the purpose of tests? To validate the behaviour of the code, or to
make developers' life easier?
-- Brane
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple checkouts.
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>>
>> This patch reduces checkout by around 23 times.
>
> On my system the difference is 43 seconds vs. 30 seconds.
On my low-end Linux desktop it's 7.5 seconds and 3.5 seconds, run
sequentially on a SATA disk.
> We lose the ability to easily spot which of the subtest is failing
> if we do this. I.e. instead of:
>
> ...
> PASS: input_validation_tests.py 19: non-working copy paths for 'status'
> FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
> PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
> ...
>
> all we'd get is:
>
> FAIL: input_validation_tests.py 1: inavlid wc and url targets
When a test fails the first thing I do is look in tests.log, that will
still work just as well with the combined test. I suppose we do lose
out if there are multiple independent bugs, as the test will stop at the
first one and not report the others.
I feel we should be optimising for the common case where the tests PASS,
and that optimising for FAILs is the wrong approach.
I agree that combining big, complex tests, like merge, is a bad idea.
But for relatively trivial tests I think reduced runtime is more
important.
--
Philip
Re: [PATCH] input_validation_tests.py: Reduce overhead of multiple
checkouts.
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Feb 21, 2011 at 01:44:35PM +0530, Noorul Islam K M wrote:
>
> This patch reduces checkout by around 23 times.
On my system the difference is 43 seconds vs. 30 seconds.
We lose the ability to easily spot which of the subtest is failing
if we do this. I.e. instead of:
...
PASS: input_validation_tests.py 19: non-working copy paths for 'status'
FAIL: input_validation_tests.py 20: non-working copy paths for 'patch'
PASS: input_validation_tests.py 21: non-working copy paths for 'switch'
...
all we'd get is:
FAIL: input_validation_tests.py 1: inavlid wc and url targets
Is there a way of keeping these as individual tests but also
avoiding the overhead of creating a repository and a working copy?
If there isn't I would prefer to just leave this as it is now because
I prefer the current output.