[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1362739 - /subversion/trunk/subversion/tests/cmdline/basic_tests.py

From: Branko Čibej <brane_at_wandisco.com>
Date: Wed, 18 Jul 2012 11:31:29 +0200

On 18.07.2012 09:47, Bert Huijben wrote:
>
>> -----Original Message-----
>> From: brane_at_apache.org [mailto:brane_at_apache.org]
>> Sent: woensdag 18 juli 2012 03:46
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1362739 -
>> /subversion/trunk/subversion/tests/cmdline/basic_tests.py
>>
>> Author: brane
>> Date: Wed Jul 18 01:46:05 2012
>> New Revision: 1362739
>>
>> URL: http://svn.apache.org/viewvc?rev=1362739&view=rev
>> Log:
>> Add some XFAIL tests for issue #4193.
>>
>> * subversion/tests/cmdline/basic_tests.py
>> (status_through_unversioned_symlink,
>> status_through_versioned_symlink,
>> add_through_unversioned_symlink, add_through_versioned_symlink):
>> New tests.
>>
>> Modified:
>> subversion/trunk/subversion/tests/cmdline/basic_tests.py
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/basic_tests.py
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/
>> basic_tests.py?rev=1362739&r1=1362738&r2=1362739&view=diff
>> ==========================================================
>> ====================
>> --- subversion/trunk/subversion/tests/cmdline/basic_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/basic_tests.py Wed Jul 18
>> 01:46:05 2012
>> @@ -2983,6 +2983,54 @@ def delete_conflicts_one_of_many(sbox):
>> verify_file_deleted("failed to remove conflict file",
>> sbox.ospath('A/D/G/rho.mine'))
>>
>> +@XFail()
>> +@Issue(4193)
>> +@SkipUnless(svntest.main.is_posix_os)
>> +def status_through_unversioned_symlink(sbox):
>> + """file status through unversioned symlink"""
>> +
>> + sbox.build(read_only = True)
>> + state = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
>> + os.symlink('A', sbox.ospath('Z'))
>> + svntest.actions.run_and_verify_status(sbox.ospath('Z/mu'), state)
>> +
>> +@XFail()
>> +@Issue(4193)
>> +@SkipUnless(svntest.main.is_posix_os)
>> +def status_through_versioned_symlink(sbox):
>> + """file status through versioned symlink"""
>> +
>> + sbox.build()
>> + state = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
>> + os.symlink('A', sbox.ospath('Z'))
>> + sbox.simple_add('Z')
>> + state.add({'Z': Item(status='A ')})
>> + svntest.actions.run_and_verify_status(sbox.ospath('Z/mu'), state)
>> +
>> +@XFail()
>> +@Issue(4193)
>> +@SkipUnless(svntest.main.is_posix_os)
>> +def add_through_unversioned_symlink(sbox):
>> + """add file through unversioned symlink"""
>> +
>> + sbox.build()
>> + os.symlink('A', sbox.ospath('Z'))
>> + sbox.simple_append('A/kappa', 'xyz', True)
>> + sbox.simple_add('Z/kappa')
>> +
>> +@XFail()
>> +@Issue(4193)
>> +@SkipUnless(svntest.main.is_posix_os)
>> +def add_through_versioned_symlink(sbox):
>> + """add file through versioned symlink"""
>> +
>> + sbox.build()
>> + os.symlink('A', sbox.ospath('Z'))
>> + sbox.simple_add('Z')
>> + sbox.simple_append('A/kappa', 'xyz', True)
>> + sbox.simple_add('Z/kappa')
>> +
>> +
> All these test, exercise exactly the same code in libsvn_wc, where we transform an absolute path into a wcroot, working copy relative path by looking it up in the database.

I figured as much, but I wanted the tests in first. Though you'll notice
that the error messages are different for status, add and add with
versioned symlink.

> In 1.6 all these cases worked because we found a .svn directory with metadata in the immediate parent (=the symlinked directory).

Yup, that's obvious.

> I'm not sure if we really want to support all these corner cases (including those where this symlink is versioned itself). We never intentionally broke this feature.
>
>
> I think we can only fix this problem by starting reading symlink targets and handle/parse them ourself. Which is something we never did in the generic case before and which will probably open a lot of new issues.

I actually wanted to start this discussion. This is clearly a regression
from 1.6, and whilst we didn't intentionally break the feature and
didn't intentionally support it in 1.6, the fact remains that --
apparently -- people are using it.

The question now is, what to do? We probably don't want to always
normalize the targets, since the performance hit would be noticeable. On
the other hand, normalizing explicit targets when they cannot be
resolved will only affect users who actually do this.

> For this class of issues I would have used one of the less common *_tests.py files than basic_tests, as that is also used by many as the light version of running our entire test suite.
>
> sbox has a few optional arguments that would make the tests cheaper to run, by not duplicating the repository etc. These would apply to all 4 tests.

I'll look into that, thanks for the pointer. And I'll move these four
tests to a new file, wc_tests.py.

-- Brane

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Received on 2012-07-18 11:32:09 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.