On 2020/04/23 2:05, Yasuhito FUTATSUKI wrote:
> On 2020/04/22 23:04, Johan Corveleyn wrote:
>> On Wed, Apr 22, 2020 at 3:50 PM Yasuhito FUTATSUKI <futat...@poem.co.jp> 
>> wrote:
>  
>>> ... and what is worse, at least
>>>
>>>> FAIL:  merge_tests.py 34: conflict markers should match the file's eol 
>>>> style
>>>
>>> this one seems to be broken even with Python 2.7, on Windows.
>>> (I'll post about it later).
>>
>> That's strange. It does succeed on my system when running with Python
>> 2.7.17. I had "All successful" test runs for [fsfs] x [ra_local,
>> ra_serf, ra_svn].
> 
> Yes, I don't doubt this test is passed on Windows with Python 2.7,
> but I doubt this test does not check just what we want to check.
> 
> As far as I read the description of the test and about KEEP_EOL_STYLE
> option introduced in r1743445.
> 
> merge_tests.py (merge_conflict_markers_matching_eol):
> [[[
> # eol-style handling during merge with conflicts, scenario 1:
> # when a merge creates a conflict on a file, make sure the file and files
> # r<left>, r<right> and .mine are in the eol-style defined for that file.
> ]]]
> 
> So I think comparion of expected and actual should be done without
> eol-style translation. However it try to check with translation on
> Windows environment.

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).

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_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.

Index: subversion/tests/cmdline/merge_tests.py
===================================================================
--- subversion/tests/cmdline/merge_tests.py     (revision 1876908)
+++ 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)
@@ -3445,7 +3441,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 +3464,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 +3506,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 +3537,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 +3566,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 1876908)
+++ 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:
@@ -1666,7 +1658,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 +1674,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):
@@ -1786,7 +1775,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 +1789,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:
@@ -1899,7 +1884,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 1876908)
+++ 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,9 +687,9 @@
         if os.path.isfile(node):
           try:
             if keep_eol_style:
-              contents = open(node, 'r', newline='').read()
+              contents = io.open(node, 'r', newline='').read()
             else:
-              contents = open(node, 'r').read()
+              contents = io.open(node, 'r').read()
           except:
             contents = open(node, 'rb').read()
         else:
Index: subversion/tests/cmdline/update_tests.py
===================================================================
--- subversion/tests/cmdline/update_tests.py    (revision 1876908)
+++ 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)
@@ -1764,7 +1760,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 +1781,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 +1813,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 +1839,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 +1864,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.

Reply via email to