On 2020/05/04 4:49, Johan Corveleyn wrote:
> On Sun, May 3, 2020 at 6:23 PM Yasuhito FUTATSUKI <futat...@poem.co.jp> wrote:
>>
>> On 2020/05/03 10:02, Johan Corveleyn wrote:
>>> On Fri, May 1, 2020 at 7:46 PM Yasuhito FUTATSUKI <futat...@poem.co.jp> 
>>> wrote:
>>>>
>>>> On 2020/04/23 2:05, Yasuhito FUTATSUKI wrote:

>>>> It seems following tests are EOL style sensitive, but they didn't
>>>> check without strict EOL style on Windows:
>>>>
>>>>   merge_tests.merge_conflict_markers_matching_eol
>>>>   merge_tests.merge_eolstyle_handling
>>>>   patch_tests.patch_no_svn_eol_style
>>>>   patch_tests.patch_with_svn_eol_style
>>>>   patch_tests.patch_with_svn_eol_style_uncommitted
>>>>   update_tests.conflict_markers_matching_eol
>>>>   update_tests.update_eol_style_handling
>>>>
>>>> I've not check that each tests depend that native EOL is '\n' or not.
>>>> However, I think if some tests depend on it, other test cases for
>>>> those aims are needed on Windows, or at least explicitly skip the tests
>>>> on Windows to clarify that those tests check nothing for the aims.
>>>>
>>>> The updated patch attached only make these tests EOL style sensitive
>>>> (and relax check of EOL on Python 2 if keep_eol_style optional
>>>> parameter is not specified).
>>>
>>> Thank you for continuing to work on this, Yasuhito.
>>>
>>> This patch indeed fixes the above 7 tests when running with Python 3 on 
>>> Windows.
>>> However, when running with Python 2 there are now 6 tests that fail
>>> (without this patch, all tests are successful with Py2):
>>
>> Thank you for testing.
>>
>> As far as this test result, those test cases don't depend on specific
>> native EOL.
>>
>>> FAIL:  merge_tests.py 34: conflict markers should match the file's eol style
>>> FAIL:  patch_tests.py 13: patch target with no svn:eol-style
>>> FAIL:  patch_tests.py 14: patch target with svn:eol-style
>>> FAIL:  patch_tests.py 15: patch target with uncommitted svn:eol-style
>>> FAIL:  patch_tests.py 57: patch a binary file
>>> FAIL:  update_tests.py 26: conflict markers should match the file's eol 
>>> style
>>>
>>> See fails.log in attachment.
>>
>> I overlooked that result of io.TextIO.read() is unicode on Python 2.
>> I hope updated patch may resolve this issue.
> 
> Okay, that latest version fixes this test for Python 2:
> 
> patch_tests.py 57: patch a binary file
> 
> But the five other failures still remain for Py2:
> 
> FAIL:  merge_tests.py 34: conflict markers should match the file's eol style
> FAIL:  patch_tests.py 13: patch target with no svn:eol-style
> FAIL:  patch_tests.py 14: patch target with svn:eol-style
> FAIL:  patch_tests.py 15: patch target with uncommitted svn:eol-style
> FAIL:  update_tests.py 26: conflict markers should match the file's eol style
> 
> See fails_py2.log in attachment.

I read those test code again, and I found that they don't distinct
text I/O and binary I/O when they write to files in some case, So I
updated the patch address them. (sain_keep_eol_style_win_patch_20200506.txt)

> For Python 3 we're getting close. After this patch and the one for
> svnrdump_tests, we only have 2 failures left with Python 3:

Please don't forget our goal is not to make the tests be passed but
to make the tests check what we want to check, on Windows both with
py2 and py3.

I've fixed the test codes to make them what the author intended I think.
If the results of py2 and py3 are still different, the test code are
still broken. With a broken test, we can't warrant ether the test target
code is correct or the test case is correct, even if the tests can be
passed. So we should also check the test cases (scenarios and expected
results) themselves are correct on Windows(, and I didn't it... at
least, yet).
 
> FAIL:  svnadmin_tests.py 35: detect denormalized names and name collisions

This is caused by output message of svnadmin, containing non UTF-8
character. Attached patch fix_svnadmin_tests_patch.txt address it,
however I don't have confidence because I don't know what charset/encoding
svnadmin on Windows use. With Python 2, output message is treated
as bytes as is, this is not affected.

> FAIL:  svndumpfilter_tests.py 7: svndumpfilter with an empty prefix

It seems this is just same reason as svnrdump_tests.py was broken.
(fix_svndumpfilter_tests_patch.txt)

Cheers,
-- 
Yasuhito FUTATSUKI <futat...@poem.co.jp>/<futat...@yf.bsdclub.org>
Fix eol style treatment in command tests on Windows.

[in subversion/tests/cmdline]

* merge_tests.py (merge_conflict_markers_matching_eol,
                  merge_eolstyle_handling),
* patch_tests.py (patch_no_svn_eol_style,
                  patch_with_svn_eol_style,
                  patch_with_svn_eol_style_uncommitted),
* update_tests.py (conflict_markers_matching_eol,
                   update_eol_style_handling):
 Specify keep_eol_style = True evne if platform is Windows.

* merge_tests.py (merge_conflict_markers_matching_eol),
* patch_tests.py (patch_no_svn_eol_style,
                  patch_with_svn_eol_style,
                  patch_with_svn_eol_style_uncommitted),
* update_tests.py (conflict_markers_matching_eol):
 Use binary mode to write file contents for strict eol style.

* merge_tests.py (merge_conflict_markers_matching_eol),
* patch_tests.py (patch_with_svn_eol_style,
                  patch_with_svn_eol_style_uncommitted),
* conflict_markers_matching_eol):
 Switch per platform eol value for 'native' svn:eol-style

* svntest/wc.py (State.from_wc):
 Use io.open() explicitly to specify 'newline' parameter for universal newline,
 even on Python 2.  With this change, '\r' end of line chracters in files
 are also translated to '\n' if keep_eol_style=False (or unspicified) on
 Python 2.  Also explicitly specify encoding to 'utf-8' not to be affected
 by Python's file system encoding.

Index: subversion/tests/cmdline/merge_tests.py
===================================================================
--- subversion/tests/cmdline/merge_tests.py     (revision 1877407)
+++ subversion/tests/cmdline/merge_tests.py     (working copy)
@@ -3323,16 +3323,12 @@
 
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
   if os.name == 'nt':
-    crlf = '\n'
+    native_nl = '\r\n'
   else:
-    crlf = '\r\n'
+    native_nl = '\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
   # Checkout a second working copy
   wc_backup = sbox.add_wc_path('backup')
   svntest.actions.run_and_verify_svn(None, [], 'checkout',
@@ -3349,8 +3345,8 @@
   path_backup = os.path.join(wc_backup, 'A', 'mu')
 
   # do the test for each eol-style
-  for eol, eolchar in zip(['CRLF', 'CR', 'native', 'LF'],
-                          [crlf, '\015', '\n', '\012']):
+  for eol, eolchar in zip(['CRLF', 'CR','native', 'LF'],
+                          [crlf, '\015', native_nl, '\012']):
     # rewrite file mu and set the eol-style property.
     svntest.main.file_write(mu_path, "This is the file 'mu'."+ eolchar, 'wb')
     svntest.main.run_svn(None, 'propset', 'svn:eol-style', eol, mu_path)
@@ -3375,8 +3371,8 @@
     svntest.main.run_svn(None, 'update', wc_backup)
 
     # Make a local mod to mu
-    svntest.main.file_append(mu_path,
-                             'Original appended text for mu' + eolchar)
+    svntest.main.file_append_binary(mu_path,
+                                    'Original appended text for mu' + eolchar)
 
     # Commit the original change and note the 'theirs' revision number
     svntest.main.run_svn(None, 'commit', '-m', 'test log', wc_dir)
@@ -3384,8 +3380,9 @@
     theirs_rev = cur_rev
 
     # Make a local mod to mu, will conflict with the previous change
-    svntest.main.file_append(path_backup,
-                             'Conflicting appended text for mu' + eolchar)
+    svntest.main.file_append_binary(path_backup,
+                                    'Conflicting appended text for mu'
+                                    + eolchar)
 
     # Create expected output tree for an update of the wc_backup.
     expected_backup_output = svntest.wc.State(wc_backup, {
@@ -3445,7 +3442,7 @@
                                           expected_backup_disk,
                                           expected_backup_status,
                                           expected_backup_skip,
-                                          keep_eol_style=keep_eol_style)
+                                          keep_eol_style=True)
 
     # cleanup for next run
     svntest.main.run_svn(None, 'revert', '-R', wc_backup)
@@ -3468,16 +3465,8 @@
 
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
-  if os.name == 'nt':
-    crlf = '\n'
-  else:
-    crlf = '\r\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
   # Checkout a second working copy
   wc_backup = sbox.add_wc_path('backup')
   svntest.actions.run_and_verify_svn(None, [], 'checkout',
@@ -3518,7 +3507,7 @@
                                         expected_backup_disk,
                                         expected_backup_status,
                                         expected_backup_skip,
-                                        keep_eol_style=keep_eol_style)
+                                        keep_eol_style=True)
 
   # Test 2: now change the eol-style property to another value and commit,
   # merge this revision in the still changed mu in the second working copy;
@@ -3549,7 +3538,7 @@
                                         expected_backup_disk,
                                         expected_backup_status,
                                         expected_backup_skip,
-                                        keep_eol_style=keep_eol_style)
+                                        keep_eol_style=True)
 
   # Test 3: now delete the eol-style property and commit, merge this revision
   # in the still changed mu in the second working copy; there should be no
@@ -3578,7 +3567,7 @@
                                         expected_backup_disk,
                                         expected_backup_status,
                                         expected_backup_skip,
-                                        keep_eol_style=keep_eol_style)
+                                        keep_eol_style=True)
 
 #----------------------------------------------------------------------
 def create_deep_trees(wc_dir):
Index: subversion/tests/cmdline/patch_tests.py
===================================================================
--- subversion/tests/cmdline/patch_tests.py     (revision 1877407)
+++ subversion/tests/cmdline/patch_tests.py     (working copy)
@@ -1574,16 +1574,8 @@
   patch_file_path = sbox.get_tempname('my.patch')
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
-  if os.name == 'nt':
-    crlf = '\n'
-  else:
-    crlf = '\r\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
   eols = [crlf, '\015', '\n', '\012']
   for target_eol in eols:
     for patch_eol in eols:
@@ -1603,7 +1595,7 @@
       ]
 
       # Set mu contents
-      svntest.main.file_write(mu_path, ''.join(mu_contents))
+      svntest.main.file_write(mu_path, ''.join(mu_contents), mode='wb')
 
       unidiff_patch = [
         "Index: mu",
@@ -1647,7 +1639,8 @@
         target_eol,
       ]
 
-      svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
+      svntest.main.file_write(patch_file_path, ''.join(unidiff_patch),
+                              mode='wb')
 
       expected_output = wc.State(wc_dir, {
         'A/mu' : Item(status='U '),
@@ -1666,7 +1659,8 @@
                                             expected_disk,
                                             expected_status,
                                             expected_skip,
-                                            [], True, True, keep_eol_style)
+                                            [], True, True,
+                                            keep_eol_style=True)
 
       expected_output = ["Reverted '" + mu_path + "'\n"]
       svntest.actions.run_and_verify_svn(expected_output, [],
@@ -1681,17 +1675,13 @@
   patch_file_path = sbox.get_tempname('my.patch')
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
   if os.name == 'nt':
-    crlf = '\n'
+    native_nl = '\r\n'
   else:
-    crlf = '\r\n'
+    native_nl = '\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
-  eols = [crlf, '\015', '\n', '\012']
+  eols = [crlf, '\015', native_nl, '\012']
   eol_styles = ['CRLF', 'CR', 'native', 'LF']
   rev = 1
   for target_eol, target_eol_style in zip(eols, eol_styles):
@@ -1714,7 +1704,7 @@
       # Set mu contents
       svntest.main.run_svn(None, 'rm', mu_path)
       svntest.main.run_svn(None, 'commit', '-m', 'delete mu', mu_path)
-      svntest.main.file_write(mu_path, ''.join(mu_contents))
+      svntest.main.file_write(mu_path, ''.join(mu_contents), mode='wb')
       svntest.main.run_svn(None, 'add', mu_path)
       svntest.main.run_svn(None, 'propset', 'svn:eol-style', target_eol_style,
                            mu_path)
@@ -1762,7 +1752,8 @@
         target_eol,
       ]
 
-      svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
+      svntest.main.file_write(patch_file_path, ''.join(unidiff_patch),
+                              mode='wb')
 
       expected_output = [
         'U         %s\n' % sbox.ospath('A/mu'),
@@ -1786,7 +1777,7 @@
                                             None, # expected err
                                             1, # check-props
                                             1, # dry-run
-                                            keep_eol_style) # keep-eol-style
+                                            keep_eol_style=True)
 
       expected_output = ["Reverted '" + mu_path + "'\n"]
       svntest.actions.run_and_verify_svn(expected_output, [], 'revert', '-R', 
wc_dir)
@@ -1800,17 +1791,13 @@
   patch_file_path = sbox.get_tempname('my.patch')
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
   if os.name == 'nt':
-    crlf = '\n'
+    native_nl = '\r\n'
   else:
-    crlf = '\r\n'
+    native_nl = '\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
-  eols = [crlf, '\015', '\n', '\012']
+  eols = [crlf, '\015', native_nl, '\012']
   eol_styles = ['CRLF', 'CR', 'native', 'LF']
   for target_eol, target_eol_style in zip(eols, eol_styles):
     for patch_eol in eols:
@@ -1830,7 +1817,7 @@
       ]
 
       # Set mu contents
-      svntest.main.file_write(mu_path, ''.join(mu_contents))
+      svntest.main.file_write(mu_path, ''.join(mu_contents), mode='wb')
       svntest.main.run_svn(None, 'propset', 'svn:eol-style', target_eol_style,
                            mu_path)
 
@@ -1876,7 +1863,8 @@
         target_eol,
       ]
 
-      svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
+      svntest.main.file_write(patch_file_path, ''.join(unidiff_patch),
+                              mode='wb')
 
       expected_output = wc.State(wc_dir, {
         'A/mu' : Item(status='U '),
@@ -1899,7 +1887,7 @@
                                             None, # expected err
                                             1, # check-props
                                             1, # dry-run
-                                            keep_eol_style) # keep-eol-style
+                                            keep_eol_style=True)
 
       expected_output = ["Reverted '" + mu_path + "'\n"]
       svntest.actions.run_and_verify_svn(expected_output, [], 'revert', '-R', 
wc_dir)
Index: subversion/tests/cmdline/svntest/wc.py
===================================================================
--- subversion/tests/cmdline/svntest/wc.py      (revision 1877407)
+++ subversion/tests/cmdline/svntest/wc.py      (working copy)
@@ -28,6 +28,7 @@
 import re
 import logging
 import pprint
+import io
 
 if sys.version_info[0] >= 3:
   # Python >=3.0
@@ -686,10 +687,18 @@
         if os.path.isfile(node):
           try:
             if keep_eol_style:
-              contents = open(node, 'r', newline='').read()
+              
+              contents = io.open(node, 'r', newline='',
+                                 encoding='utf-8').read()
             else:
-              contents = open(node, 'r').read()
+              contents = io.open(node, 'r', encoding='utf-8').read()
+            if not isinstance(contents, str):
+              # Python 2: contents is read as an unicode object,
+              # but we expect it is a str.
+              contents = contents.encode()
           except:
+            # If the file contains non UTF-8 character, we treat its
+            # content as binary represented as a bytes object.
             contents = open(node, 'rb').read()
         else:
           contents = None
Index: subversion/tests/cmdline/update_tests.py
===================================================================
--- subversion/tests/cmdline/update_tests.py    (revision 1877407)
+++ subversion/tests/cmdline/update_tests.py    (working copy)
@@ -1650,16 +1650,12 @@
 
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
   if os.name == 'nt':
-    crlf = '\n'
+    native_nl = '\r\n'
   else:
-    crlf = '\r\n'
+    native_nl = '\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
   # Checkout a second working copy
   wc_backup = sbox.add_wc_path('backup')
   svntest.actions.run_and_verify_svn(None, [], 'checkout',
@@ -1677,7 +1673,7 @@
 
   # do the test for each eol-style
   for eol, eolchar in zip(['CRLF', 'CR', 'native', 'LF'],
-                          [crlf, '\015', '\n', '\012']):
+                          [crlf, '\015', native_nl, '\012']):
     # rewrite file mu and set the eol-style property.
     svntest.main.file_write(mu_path, "This is the file 'mu'."+ eolchar, 'wb')
     svntest.main.run_svn(None, 'propset', 'svn:eol-style', eol, mu_path)
@@ -1704,8 +1700,8 @@
     svntest.main.run_svn(None, 'update', wc_backup)
 
     # Make a local mod to mu
-    svntest.main.file_append(mu_path,
-                             'Original appended text for mu' + eolchar)
+    svntest.main.file_append_binary(mu_path,
+                                    'Original appended text for mu' + eolchar)
 
     # Commit the original change and note the 'theirs' revision number
     svntest.main.run_svn(None, 'commit', '-m', 'test log', wc_dir)
@@ -1713,8 +1709,9 @@
     theirs_rev = cur_rev
 
     # Make a local mod to mu, will conflict with the previous change
-    svntest.main.file_append(path_backup,
-                             'Conflicting appended text for mu' + eolchar)
+    svntest.main.file_append_binary(path_backup,
+                                    'Conflicting appended text for mu'
+                                    + eolchar)
 
     # Create expected output tree for an update of the wc_backup.
     expected_backup_output = svntest.wc.State(wc_backup, {
@@ -1764,7 +1761,7 @@
                                            expected_backup_output,
                                            expected_backup_disk,
                                            expected_backup_status,
-                                           keep_eol_style=keep_eol_style)
+                                           keep_eol_style=True)
 
     # cleanup for next run
     svntest.main.run_svn(None, 'revert', '-R', wc_backup)
@@ -1785,16 +1782,8 @@
 
   mu_path = sbox.ospath('A/mu')
 
-  # CRLF is a string that will match a CRLF sequence read from a text file.
-  # ### On Windows, we assume CRLF will be read as LF, so it's a poor test.
-  if os.name == 'nt':
-    crlf = '\n'
-  else:
-    crlf = '\r\n'
+  crlf = '\r\n'
 
-  # Strict EOL style matching breaks Windows tests at least with Python 2
-  keep_eol_style = not svntest.main.is_os_windows()
-
   # Checkout a second working copy
   wc_backup = sbox.add_wc_path('backup')
   svntest.actions.run_and_verify_svn(None, [], 'checkout',
@@ -1825,7 +1814,7 @@
                                          expected_backup_output,
                                          expected_backup_disk,
                                          expected_backup_status,
-                                         keep_eol_style=keep_eol_style)
+                                         keep_eol_style=True)
 
   # Test 2: now change the eol-style property to another value and commit,
   # update the still changed mu in the second working copy; there should be
@@ -1851,7 +1840,7 @@
                                          expected_backup_output,
                                          expected_backup_disk,
                                          expected_backup_status,
-                                         keep_eol_style=keep_eol_style)
+                                         keep_eol_style=True)
 
   # Test 3: now delete the eol-style property and commit, update the still
   # changed mu in the second working copy; there should be no conflict!
@@ -1876,7 +1865,7 @@
                                          expected_backup_output,
                                          expected_backup_disk,
                                          expected_backup_status,
-                                         keep_eol_style=keep_eol_style)
+                                         keep_eol_style=True)
 
 # Bug in which "update" put a bogus revision number on a schedule-add file,
 # causing the wrong version of it to be committed.
* subversion/tests/cmdline/svntest/main.py (wait_on_pipe):
Accept any sequence of bytes even if it is not valid UTF-8 character sequence.
Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py    (revision 1877407)
+++ subversion/tests/cmdline/svntest/main.py    (working copy)
@@ -527,10 +527,10 @@
 
   # We always expect STDERR to be strings, not byte-arrays.
   if not isinstance(stderr, str):
-    stderr = stderr.decode("utf-8")
+    stderr = stderr.decode("utf-8", 'surrogateescape')
   if not binary_mode:
     if not isinstance(stdout, str):
-      stdout = stdout.decode("utf-8")
+      stdout = stdout.decode("utf-8", 'surrogateescape')
 
     # Normalize Windows line endings if in text mode.
     if windows:
* subversion/tests/cmdline/svndumpfilter_tests.py (filter_and_return_output)
 Use list complehension instead of map function, which returns iterator
 on Python 3, to get EOL translated list of lines on Windows.

Index: subversion/tests/cmdline/svndumpfilter_tests.py
===================================================================
--- subversion/tests/cmdline/svndumpfilter_tests.py     (revision 1877407)
+++ subversion/tests/cmdline/svndumpfilter_tests.py     (working copy)
@@ -71,7 +71,7 @@
   # Since we call svntest.main.run_command_stdin() in binary mode,
   # normalize the stderr line endings on Windows ourselves.
   if sys.platform == 'win32':
-    errput = map(lambda x : x.replace('\r\n', '\n'), errput)
+    errput = [x.replace('\r\n', '\n') for x in errput]
 
   return output, errput
 

Reply via email to