You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan <lu...@gmx.de> on 2016/10/14 15:30:57 UTC

[PATCH] Testcase for issue #4642

Hi,

following patch adds a test case for issue #4642: ""svn update
--set-depth=exclude" exits prematurely, leaving repo in need of cleanup"
[1].

[[[
Add an XFail test for issue #4642 (Setting depth to exclude for a path
containing unversioned files requires a cleanup afterwards).

* subversion/tests/cmdline/depth_tests.py
  (fold_tree_with_deleted_moved_items): New XFail test for issue #4642.
]]]

[1] https://issues.apache.org/jira/browse/SVN-4642

Regards,
Stefan


Re: [PATCH] Testcase for issue #4642

Posted by Stefan <lu...@posteo.de>.
On 11/17/2016 12:19, Stefan Sperling wrote:
> On Thu, Nov 17, 2016 at 12:58:48AM +0100, Stefan wrote:
>> Is there something else I'd do to improve the patch, or does anybody
>> feel like approving it for commit?
> +1

Thanks. Committed in r1770295.



Re: [PATCH] Testcase for issue #4642

Posted by Stefan Sperling <st...@elego.de>.
On Thu, Nov 17, 2016 at 12:58:48AM +0100, Stefan wrote:
> Is there something else I'd do to improve the patch, or does anybody
> feel like approving it for commit?

+1

Re: [PATCH] Testcase for issue #4642

Posted by Stefan <lu...@posteo.de>.
On 11/8/2016 09:54, Stefan wrote:
> On 10/18/2016 12:14, Daniel Shahaf wrote:
>> Stefan Hett wrote on Tue, Oct 18, 2016 at 11:56:29 +0200:
>>> Thanks for the review Daniel. Just a quick heads up that I'll get back to
>>> this asap. Might take a few days until I can free up some time though.
>> Thanks for coordinating our expectations.
> Finally got to incorporate your feedback into a revised patch (see
> attached).
>
> I tested it against current trunk on Windows.
>
Is there something else I'd do to improve the patch, or does anybody
feel like approving it for commit?

Regards,
Stefan



Re: [PATCH] Testcase for issue #4642

Posted by Stefan <lu...@posteo.de>.
On 10/18/2016 12:14, Daniel Shahaf wrote:
> Stefan Hett wrote on Tue, Oct 18, 2016 at 11:56:29 +0200:
>> Thanks for the review Daniel. Just a quick heads up that I'll get back to
>> this asap. Might take a few days until I can free up some time though.
> Thanks for coordinating our expectations.

Finally got to incorporate your feedback into a revised patch (see
attached).

I tested it against current trunk on Windows.

Regards,
Stefan


Re: [PATCH] Testcase for issue #4642

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Hett wrote on Tue, Oct 18, 2016 at 11:56:29 +0200:
> Thanks for the review Daniel. Just a quick heads up that I'll get back to
> this asap. Might take a few days until I can free up some time though.

Thanks for coordinating our expectations.

Re: [PATCH] Testcase for issue #4642

Posted by Stefan Hett <st...@egosoft.com>.
On 10/14/2016 8:53 PM, Daniel Shahaf wrote:
> Stefan wrote on Fri, Oct 14, 2016 at 17:30:57 +0200:
>> Add an XFail test for issue #4642 (Setting depth to exclude for a path
>> containing unversioned files requires a cleanup afterwards).
> Looks good overall.  Just a few minor questions:
>
>> +def fold_tree_with_unversioned_items(sbox):
>> +  "unversioned files in excluded directory"
>> +  ign_a, ign_b, ign_c, wc_dir = set_up_depthy_working_copies(sbox,
>> +                                                             infinity=True)
> I see that "ign_a" is a pre�xisting convention throughout the file.
> It's unfortunate we have our own convention, since there's already
> a Python convention for an ignored value in a tuple unpacking: using "_"
> as the assigned-to variable name.
>
>> +  # create an unversioned directory within a versioned one
>> +  A_path = sbox.ospath('A')
>> +  A_local_path = os.path.join(A_path, 'A_local')
>> +  os.mkdir(A_local_path)
>> +
>> +  # Set A to be excluded.
>> +  svntest.main.run_svn(None, 'update', '--set-depth=exclude', A_path)
>> +
>> +  # try a simple update afterwards
>> +  svntest.main.run_svn(None, 'update', wc_dir)
> Don't we prefer �sbox.simple_update()� for new code?
>
>> @@ -3009,6 +3027,7 @@
>>                 spurious_nodes_row,
>>                 commit_excluded,
>>   			  fold_tree_with_deleted_moved_items,
>> +			  fold_tree_with_unversioned_items,
> Both of these lines use tabs instead of spaces.
Thanks for the review Daniel. Just a quick heads up that I'll get back 
to this asap. Might take a few days until I can free up some time though.

-- 
Regards,
Stefan Hett, Developer/Administrator

EGOSOFT GmbH, Heidestrasse 4, 52146 W�rselen, Germany
Tel: +49 2405 4239970, www.egosoft.com
Gesch�ftsf�hrer: Bernd Lehahn, Handelsregister Aachen HRB 13473


Re: [PATCH] Testcase for issue #4642

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan wrote on Fri, Oct 14, 2016 at 17:30:57 +0200:
> Add an XFail test for issue #4642 (Setting depth to exclude for a path
> containing unversioned files requires a cleanup afterwards).

Looks good overall.  Just a few minor questions:

> +def fold_tree_with_unversioned_items(sbox):
> +  "unversioned files in excluded directory"
> +  ign_a, ign_b, ign_c, wc_dir = set_up_depthy_working_copies(sbox,
> +                                                             infinity=True)

I see that "ign_a" is a pre�xisting convention throughout the file.
It's unfortunate we have our own convention, since there's already
a Python convention for an ignored value in a tuple unpacking: using "_"
as the assigned-to variable name.

> +  # create an unversioned directory within a versioned one
> +  A_path = sbox.ospath('A')
> +  A_local_path = os.path.join(A_path, 'A_local')
> +  os.mkdir(A_local_path)
> +
> +  # Set A to be excluded.
> +  svntest.main.run_svn(None, 'update', '--set-depth=exclude', A_path)
> +  
> +  # try a simple update afterwards
> +  svntest.main.run_svn(None, 'update', wc_dir)

Don't we prefer �sbox.simple_update()� for new code?

> @@ -3009,6 +3027,7 @@
>                spurious_nodes_row,
>                commit_excluded,
>  			  fold_tree_with_deleted_moved_items,
> +			  fold_tree_with_unversioned_items,

Both of these lines use tabs instead of spaces.