Den mån 29 maj 2023 kl 12:38 skrev Jun Omae <jun6...@gmail.com>:

> On 2023/05/29 18:58, Daniel Sahlberg wrote:
> > For test 16:
> > [[[
> > W: Unexpected output
> > W: EXPECTED STDERR (regexp, match_all=False):
> > W: | svn: E200007: Cannot move path '.*
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B <
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B>'
> into its own child '.*
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F <
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F>'
> > W: ACTUAL STDERR:
> > W: | svn: E200007: Cannot move path
> 'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B' into
> its own child
> 'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B\F'
> > ]]]
> >
> > There was a call to svn_dirent_local_style on the error message, which
> obviously turned \ to / on Windows. I've added a check in r1910112, please
> check if this works for you.
>
> Thanks. Verified that copy_tests.py 16 goes away with r1910112.
>

Thanks for checking!


> > For test 17:
> > [[[
> >   File "C:\usr\apps\python312\Lib\re\_parser.py", line 383, in _escape
> >     raise source.error("incomplete escape %s" % escape, len(escape))
> > re.error: incomplete escape \u at position 34
> > ]]]
> >
> > I suppose this is because the expected_error is constructed like this:
> > [[[
> >   expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> >                    "child '%s'" % (from_path, to_path)
> > ]]]
> >
> > With a from_path C:\usr\subversion\[...], I suppose Python interpret \
> as an escape character. I'm not proficient enough in Python to have an idea
> of how to escape from_path and to_path in an appropriate way. @Jun, do you
> have a suggestion?
>
> The following patch could fix it and verified (applying `re.escape` to the
> paths).
>
> [[[
> Index: subversion/tests/cmdline/copy_tests.py
> ===================================================================
> --- subversion/tests/cmdline/copy_tests.py      (revision 1910112)
> +++ subversion/tests/cmdline/copy_tests.py      (working copy)
> @@ -1295,7 +1295,7 @@ def wc_move_parent_into_child(sbox):
>    os.chdir(wc_dir)
>
>    expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> -                   "child '%s'" % (from_path, to_path)
> +                   "child '%s'" % (re.escape(from_path),
> re.escape(to_path))
>    svntest.actions.run_and_verify_svn(None, expected_error,
>                                       'mv',
>                                       '.', 'F/B')
> ]]]
>

Sounds good, thanks for your suggestion. Would you like to commit?


> > (Sidenote: There is an error message in the test log about line 1424.
> This was added way back in r846892 and has essentially been unchanged ever
> since. I believe this is unrelated and I will send this in a separate
> thread when I've had some more time to look at it).
>
> The `SyntaxWarning` from line 1424 is displayed since Python 3.12, even
> without -Wdefault option.
>
> $ python3.12 -c '"\$"'
> <string>:1: SyntaxWarning: invalid escape sequence '\$'
> $ python3.11 -c '"\$"'   # no warnings
> $ python3.11 -Wdefault -c '"\$"'
> <string>:1: DeprecationWarning: invalid escape sequence '\$'
>
> See also:
> https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes


If I understand this correctly, the string should be declared as raw
instead of binary:
[[[
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py      (revision 1910111)
+++ subversion/tests/cmdline/copy_tests.py      (working copy)
@@ -1421,7 +1421,7 @@
   if re.match(b'[^\\r]\\n', raw_contents):
     raise svntest.Failure

-  if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
line_contents[3]):
+  if not re.match(r'.*\$LastChangedRevision:\s*\d+\s*\$',
line_contents[3]):
     raise svntest.Failure

 #-------------------------------------------------------------
]]]

Maybe the previous re.match (first context line in the diff) should be
changed as well?

Kind regards,
Daniel

Reply via email to