On Sun, Apr 5, 2009 at 14:29, Arfrever Frehtes Taifersar Arahesis
<Arfrever.FTA_at_gmail.com> wrote:
> 2009-04-05 09:58:22 Bert Huijben napisał(a):
>> > -----Original Message-----
>> > From: Arfrever Frehtes Taifersar Arahesis
>> > [mailto:Arfrever.FTA_at_GMail.Com]
>> > Sent: Sunday, April 05, 2009 3:18 AM
>> > To: svn_at_subversion.tigris.org
>> > Subject: svn commit: r37008 - trunk/subversion/tests/cmdline/svntest
>...
>> > +++ trunk/subversion/tests/cmdline/svntest/wc.py Sat Apr 4
>> > 18:17:40 2009 (r37008)
>> > @@ -492,6 +492,8 @@ class State:
>> > node = os.path.join(dirpath, name)
>> > if os.path.isfile(node):
>> > contents = open(node, 'rb').read()
>> > + if sys.platform == 'win32':
>> > + contents = contents.replace('\r\n', '\n')
>> > try:
>> > contents = contents.decode()
>> > except UnicodeDecodeError:
>>
>>
>> -1 on this approach of hiding newline problems. (Not the first time I
>> mention this)
>>
>> On windows in text a "\r\n" is good, a "\n" is an error. And the test suite
>> must error on this condition like it always did.
>
> The following code should be sufficient:
>
> if sys.platform == 'win32':
> for line in contents.splitlines(True):
> if line[-2:] != '\r\n':
> raise ValueError("Invalid line ending: '%s'" % line)
> contents = contents.replace('\r\n', '\n')
No, that won't work. What if we have a test that is *supposed* to
generate \n, rather than \r\n? The above code would error. And the
.replace() would throw away information on what kind of newline was
generated.
>> This hides all these possible errors in our test suite. Each line MUST END
>> with a "\r\n", or most other tools will fail.
>>
>> E.g. you would hide errors like the ones you introduced by merging the svn
>> patch branch..
>> or with properties that contain invalid end of line sequences by just
>> ignoring the "\r"..
>> or by svn diff that sometimes uses the wrong line endings (Some cases with
>> svn:eol-style native).
>>
>> You are still trying to fix things for a handful of python 3 users, by
>> reducing the usefulness of the test suite for millions of Windows users.
>>
>>
>> The approach of moving the tests to Unicode, instead of comparing the actual
>> bytes from svn is the wrong approach..
>
> It would be very hard to maintain the branch with all "strings" replaced
> with b"strings".
Frankly? I really don't care at all. The test suite MUST WORK. Your
branch is completely secondary to that.
Personally, I don't see us *ever* merging that branch. I don't see
much impetus or benefit to that. Do what you want on the branch, but
there are limits to the impact you can make on trunk. This whole
"Python 3.0 compatibility" work on trunk has been introducing
complexity where it is not needed. Making the test suite less
effective, more difficult to work with, or limited in the kinds of
things it can do... in the name of Python 3.0 compatibility? No. That
just doesn't make sense.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1553743
Received on 2009-04-05 21:40:10 CEST