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/07/08 06:38:55 UTC

[PATCH] Re: svn commit: r1143731 - /subversion/trunk/subversion/tests/cmdline/info_tests.py

Daniel Shahaf <d....@daniel.shahaf.name> writes:

> rhuijben@apache.org wrote on Thu, Jul 07, 2011 at 09:44:12 -0000:
>
>> Author: rhuijben
>> Date: Thu Jul  7 09:44:12 2011
>> New Revision: 1143731
>> 
>> URL: http://svn.apache.org/viewvc?rev=1143731&view=rev
>> Log:
>> Add testcase for issue #3787.
>> 
>> * subversion/tests/cmdline/info_tests.py
>>   (info_show_exclude): New testcase.
>>   (test_list): Add info_show_exclude.
>> 
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> (extended by me)
>> 
>> Modified:
>>     subversion/trunk/subversion/tests/cmdline/info_tests.py
>> 
>> Modified: subversion/trunk/subversion/tests/cmdline/info_tests.py
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/info_tests.py?rev=1143731&r1=1143730&r2=1143731&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/info_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/info_tests.py Thu Jul  7 09:44:12 2011
>> @@ -412,6 +412,77 @@ def info_repos_root_url(sbox):
>>    svntest.actions.run_and_verify_info(expected_info, sbox.repo_url,
>>                                        '--depth', 'files')
>>  
>> +@Issue(3787)
>> +def info_show_exclude(sbox):
>> +  "tests 'info --depth' variants on excluded node"
>> +
>> +  sbox.build()
>> +  wc_dir = sbox.wc_dir
>> +
>> +  A_path = os.path.join(wc_dir, 'A')
>> +  iota = os.path.join(wc_dir, 'iota')
>> +  svntest.main.run_svn(None, 'up', '--set-depth', 'exclude', A_path)
>> +  wc_uuid = svntest.actions.get_wc_uuid(wc_dir)
>> +  
>> +  expected_info = []
>> +  expected_info = [{
>> +      'Path' : '.',
>
> You probably want re.escape('.') or re.escape(wc_dir) here.
>

I think this is not necessary since these paths do not have the
separator. Otherwise this should have failed on windows bot. But still
to be consistent I made the changes and here is the patch.

Log
[[[

* subversion/tests/cmdline/info_tests.py
  (info_url_special_characters, info_repos_root_url,
   info_show_exclude): Escape paths.

Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]

Thanks and Regards
Noorul


Re: [PATCH] Re: svn commit: r1143731 - /subversion/trunk/subversion/tests/cmdline/info_tests.py

Posted by Noorul Islam K M <no...@collab.net>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:

> Noorul Islam K M wrote on Fri, Jul 08, 2011 at 10:08:55 +0530:
>
>> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>> 
>> > rhuijben@apache.org wrote on Thu, Jul 07, 2011 at 09:44:12 -0000:
>> >
>> >> Author: rhuijben
>> >> Date: Thu Jul  7 09:44:12 2011
>> >> New Revision: 1143731
>> >> 
>> >> URL: http://svn.apache.org/viewvc?rev=1143731&view=rev
>> >> Log:
>> >> Add testcase for issue #3787.
>> >> 
>> >> * subversion/tests/cmdline/info_tests.py
>> >>   (info_show_exclude): New testcase.
>> >>   (test_list): Add info_show_exclude.
>> >> 
>> >> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> >> (extended by me)
>> >> 
>> >> Modified:
>> >>     subversion/trunk/subversion/tests/cmdline/info_tests.py
>> >> 
>> >> Modified: subversion/trunk/subversion/tests/cmdline/info_tests.py
>> >> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/info_tests.py?rev=1143731&r1=1143730&r2=1143731&view=diff
>> >> ==============================================================================
>> >> --- subversion/trunk/subversion/tests/cmdline/info_tests.py (original)
>> >> +++ subversion/trunk/subversion/tests/cmdline/info_tests.py Thu Jul  7 09:44:12 2011
>> >> @@ -412,6 +412,77 @@ def info_repos_root_url(sbox):
>> >>    svntest.actions.run_and_verify_info(expected_info, sbox.repo_url,
>> >>                                        '--depth', 'files')
>> >>  
>> >> +@Issue(3787)
>> >> +def info_show_exclude(sbox):
>> >> +  "tests 'info --depth' variants on excluded node"
>> >> +
>> >> +  sbox.build()
>> >> +  wc_dir = sbox.wc_dir
>> >> +
>> >> +  A_path = os.path.join(wc_dir, 'A')
>> >> +  iota = os.path.join(wc_dir, 'iota')
>> >> +  svntest.main.run_svn(None, 'up', '--set-depth', 'exclude', A_path)
>> >> +  wc_uuid = svntest.actions.get_wc_uuid(wc_dir)
>> >> +  
>> >> +  expected_info = []
>> >> +  expected_info = [{
>> >> +      'Path' : '.',
>> >
>> > You probably want re.escape('.') or re.escape(wc_dir) here.
>> >
>> 
>> I think this is not necessary since these paths do not have the
>> separator. Otherwise this should have failed on windows bot. But still
>
> The problem is the dot, not the backslash.
>
>> to be consistent I made the changes and here is the patch.
>> 
>> Log
>> [[[
>> 
>> * subversion/tests/cmdline/info_tests.py
>>   (info_url_special_characters, info_repos_root_url,
>>    info_show_exclude): Escape paths.
>> 
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> ]]]
>> 
>> Thanks and Regards
>> Noorul
>> 
>
>> Index: subversion/tests/cmdline/info_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/info_tests.py	(revision 1143778)
>> +++ subversion/tests/cmdline/info_tests.py	(working copy)
>> @@ -320,7 +320,7 @@
>>    special_urls = [sbox.repo_url + '/A' + '/%2E',
>>                    sbox.repo_url + '%2F' + 'A']
>>  
>> -  expected = {'Path' : 'A',
>> +  expected = {'Path' : re.escape('A'),
>>                'Repository Root' : re.escape(sbox.repo_url),
>>                'Revision' : '1',
>>                'Node Kind' : 'dir',
>> @@ -399,8 +399,8 @@
>>          'Last Changed Rev'  : '1',
>>      },
>>      {
>> -        'Path'              : 'iota',
>> -        'Name'              : 'iota',
>> +        'Path'              : re.escape('iota'),
>> +        'Name'              : re.escape('iota'),
>>          'Repository Root'   : re.escape(sbox.repo_url),
>>          'URL'               : re.escape(sbox.repo_url + '/iota'),
>>          'Revision'          : '1',
>> @@ -426,7 +426,7 @@
>>    
>>    expected_info = []
>>    expected_info = [{
>> -      'Path' : '.',
>> +      'Path' : re.escape(wc_dir),
>>        'Repository Root' : sbox.repo_url,
>>        'Repository UUID' : wc_uuid,
>>    }]
>
> This is the operative hunk and it doesn't apply to HEAD (I already fixed
> this).  The other hunks are correct but I'm not sure the code churn is
> worth it in this case.
>

I agree.

Thanks and Regards
Noorul

Re: [PATCH] Re: svn commit: r1143731 - /subversion/trunk/subversion/tests/cmdline/info_tests.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Noorul Islam K M wrote on Fri, Jul 08, 2011 at 10:08:55 +0530:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> 
> > rhuijben@apache.org wrote on Thu, Jul 07, 2011 at 09:44:12 -0000:
> >
> >> Author: rhuijben
> >> Date: Thu Jul  7 09:44:12 2011
> >> New Revision: 1143731
> >> 
> >> URL: http://svn.apache.org/viewvc?rev=1143731&view=rev
> >> Log:
> >> Add testcase for issue #3787.
> >> 
> >> * subversion/tests/cmdline/info_tests.py
> >>   (info_show_exclude): New testcase.
> >>   (test_list): Add info_show_exclude.
> >> 
> >> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> >> (extended by me)
> >> 
> >> Modified:
> >>     subversion/trunk/subversion/tests/cmdline/info_tests.py
> >> 
> >> Modified: subversion/trunk/subversion/tests/cmdline/info_tests.py
> >> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/info_tests.py?rev=1143731&r1=1143730&r2=1143731&view=diff
> >> ==============================================================================
> >> --- subversion/trunk/subversion/tests/cmdline/info_tests.py (original)
> >> +++ subversion/trunk/subversion/tests/cmdline/info_tests.py Thu Jul  7 09:44:12 2011
> >> @@ -412,6 +412,77 @@ def info_repos_root_url(sbox):
> >>    svntest.actions.run_and_verify_info(expected_info, sbox.repo_url,
> >>                                        '--depth', 'files')
> >>  
> >> +@Issue(3787)
> >> +def info_show_exclude(sbox):
> >> +  "tests 'info --depth' variants on excluded node"
> >> +
> >> +  sbox.build()
> >> +  wc_dir = sbox.wc_dir
> >> +
> >> +  A_path = os.path.join(wc_dir, 'A')
> >> +  iota = os.path.join(wc_dir, 'iota')
> >> +  svntest.main.run_svn(None, 'up', '--set-depth', 'exclude', A_path)
> >> +  wc_uuid = svntest.actions.get_wc_uuid(wc_dir)
> >> +  
> >> +  expected_info = []
> >> +  expected_info = [{
> >> +      'Path' : '.',
> >
> > You probably want re.escape('.') or re.escape(wc_dir) here.
> >
> 
> I think this is not necessary since these paths do not have the
> separator. Otherwise this should have failed on windows bot. But still

The problem is the dot, not the backslash.

> to be consistent I made the changes and here is the patch.
> 
> Log
> [[[
> 
> * subversion/tests/cmdline/info_tests.py
>   (info_url_special_characters, info_repos_root_url,
>    info_show_exclude): Escape paths.
> 
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> ]]]
> 
> Thanks and Regards
> Noorul
> 

> Index: subversion/tests/cmdline/info_tests.py
> ===================================================================
> --- subversion/tests/cmdline/info_tests.py	(revision 1143778)
> +++ subversion/tests/cmdline/info_tests.py	(working copy)
> @@ -320,7 +320,7 @@
>    special_urls = [sbox.repo_url + '/A' + '/%2E',
>                    sbox.repo_url + '%2F' + 'A']
>  
> -  expected = {'Path' : 'A',
> +  expected = {'Path' : re.escape('A'),
>                'Repository Root' : re.escape(sbox.repo_url),
>                'Revision' : '1',
>                'Node Kind' : 'dir',
> @@ -399,8 +399,8 @@
>          'Last Changed Rev'  : '1',
>      },
>      {
> -        'Path'              : 'iota',
> -        'Name'              : 'iota',
> +        'Path'              : re.escape('iota'),
> +        'Name'              : re.escape('iota'),
>          'Repository Root'   : re.escape(sbox.repo_url),
>          'URL'               : re.escape(sbox.repo_url + '/iota'),
>          'Revision'          : '1',
> @@ -426,7 +426,7 @@
>    
>    expected_info = []
>    expected_info = [{
> -      'Path' : '.',
> +      'Path' : re.escape(wc_dir),
>        'Repository Root' : sbox.repo_url,
>        'Repository UUID' : wc_uuid,
>    }]

This is the operative hunk and it doesn't apply to HEAD (I already fixed
this).  The other hunks are correct but I'm not sure the code churn is
worth it in this case.

Thanks for the patch.

> @@ -435,7 +435,7 @@
>                                        wc_dir)
>  
>    expected_info = [{
> -      'Path' : 'A',
> +      'Path' : re.escape('A'),
>        'Repository Root' : sbox.repo_url,
>        'Repository UUID' : wc_uuid,
>        'Depth' : 'exclude',