You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2022/01/25 09:59:40 UTC

svn commit: r1897443 - in /subversion/trunk/subversion/tests/cmdline/svntest: actions.py main.py

Author: danielsh
Date: Tue Jan 25 09:59:40 2022
New Revision: 1897443

URL: http://svn.apache.org/viewvc?rev=1897443&view=rev
Log:
Stop encoding a test's number in the svntest library.

* subversion/tests/cmdline/svntest/main.py
  (run_command_stdin): Stop hardcoding an exception for "prop_tests-12".

* subversion/tests/cmdline/svntest/actions.py
  (disable_revprop_changes): Stop emitting pre-revprop-change's argv[1].
    This function is only used by prop_tests.py 12 revprop_change(), and that
    function doesn't care about the value of argv[1].

Modified:
    subversion/trunk/subversion/tests/cmdline/svntest/actions.py
    subversion/trunk/subversion/tests/cmdline/svntest/main.py

Modified: subversion/trunk/subversion/tests/cmdline/svntest/actions.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/actions.py?rev=1897443&r1=1897442&r2=1897443&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svntest/actions.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svntest/actions.py Tue Jan 25 09:59:40 2022
@@ -2134,9 +2134,10 @@ def disable_revprop_changes(repo_dir):
   main.create_python_hook_script(hook_path,
                                  'import sys\n'
                                  'sys.stderr.write("pre-revprop-change %s" %'
-                                                  ' " ".join(sys.argv[1:]))\n'
+                                                  ' " ".join(sys.argv[2:]))\n'
                                  'sys.exit(1)\n',
                                  cmd_alternative=
+                                       '@shift\n'
                                        '@echo pre-revprop-change %* 1>&2\n'
                                        '@exit 1\n')
 

Modified: subversion/trunk/subversion/tests/cmdline/svntest/main.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/main.py?rev=1897443&r1=1897442&r2=1897443&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svntest/main.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Jan 25 09:59:40 2022
@@ -637,8 +637,7 @@ def run_command_stdin(command, error_exp
       break
     # Does the server leak the repository on-disk path?
     # (prop_tests-12 installs a hook script that does that intentionally)
-    if any(map(_line_contains_repos_diskpath, lines)) \
-       and not any(map(lambda arg: 'prop_tests-12' in arg, varargs)):
+    if any(map(_line_contains_repos_diskpath, lines)):
       raise Failure("Repository diskpath in %s: %r" % (name, lines))
 
   valgrind_diagnostic = False



Re: svn commit: r1897443 - in /subversion/trunk/subversion/tests/cmdline/svntest: actions.py main.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jun Omae wrote on Thu, Mar 03, 2022 at 23:41:36 +0900:
> On 2022/02/19 21:27, Daniel Shahaf wrote:
> > Ping?  Any Windows developer has a minute to test & commit either of the
> > patches in the grandparent post?
> 
> I verified prop_tests.py 12 passed on Windows 10 with Python 3.9 after the patch.
> Also, I tried to run tests with windows-2019 environment on GitHub Actions, and
> confirmed the patch.
> 
> Before the patch: https://github.com/jun66j5/subversion/runs/5406933640?check_suite_focus=true#step:7:2986
> 
> Summary of test results:
>   2592 tests PASSED
>   127 tests SKIPPED
>   77 tests XFAILED (17 WORK-IN-PROGRESS)
>   1 test FAILED
> 
> 
> After the patch: https://github.com/jun66j5/subversion/runs/5407731482?check_suite_focus=true#step:7:2985
> 
> Summary of test results:
>   2593 tests PASSED
>   127 tests SKIPPED
>   77 tests XFAILED (17 WORK-IN-PROGRESS)

Thanks.  r1898599.

Re: svn commit: r1897443 - in /subversion/trunk/subversion/tests/cmdline/svntest: actions.py main.py

Posted by Jun Omae <ju...@gmail.com>.
Hi,

On 2022/02/19 21:27, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Tue, Feb 08, 2022 at 11:47:47 +0000:
>> Jun Omae wrote on Sat, Feb 05, 2022 at 00:28:51 +0900:
>>> After r1897443, prop_tests.py 12 is failing on Windows.
>>>
>>> svntest.Failure: Repository diskpath in stderr: [
>>> "svn: E175008: While handling the 'cash-sound' property on '/svn-test-work/repositories/prop_tests-12/!svn/bln/0':\n",
>>> 'svn: E175008: Revprop change blocked by pre-revprop-change hook (exit code 1) with output:\n',
>>> 'pre-revprop-change D:\\a\\subversion\\subversion\\Release\\subversion\\tests\\cmdline\\svn-test-work\\repositories\\prop_tests-12 0 jrandom cash-sound A\n']
>>> FAIL:  prop_tests.py 12: set, get, and delete a revprop change
>>>
>> ⋮
>>> I think we could use %2 .. %5 variables instead of %* variable.
>> ⋮
>>> Or we could simply remove `cmd_alternative` parameter.
>>
>> Thanks, Jun.  As I don't have a Windows build environment and we don't have
>> Windows buildbots either, so I can't debug this myself.
>>
>> Both of your alternative patches seem reasonable.  I like how the latter
>> removes the need to maintain two separate implementations (.py and .bat)
>> of identical functionality.
>>
>> However, at the end of the day, I'll be happy with any solution that
>> works for Windows-based devs.
>>
>> Could someone test and commit those Windows-only patches, please?
> 
> Ping?  Any Windows developer has a minute to test & commit either of the
> patches in the grandparent post?

I verified prop_tests.py 12 passed on Windows 10 with Python 3.9 after the patch.
Also, I tried to run tests with windows-2019 environment on GitHub Actions, and
confirmed the patch.

Before the patch: https://github.com/jun66j5/subversion/runs/5406933640?check_suite_focus=true#step:7:2986

Summary of test results:
   2592 tests PASSED
   127 tests SKIPPED
   77 tests XFAILED (17 WORK-IN-PROGRESS)
   1 test FAILED


After the patch: https://github.com/jun66j5/subversion/runs/5407731482?check_suite_focus=true#step:7:2985

Summary of test results:
   2593 tests PASSED
   127 tests SKIPPED
   77 tests XFAILED (17 WORK-IN-PROGRESS)


-- 
Jun Omae <ju...@gmail.com> (大前 潤)

Re: svn commit: r1897443 - in /subversion/trunk/subversion/tests/cmdline/svntest: actions.py main.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Tue, Feb 08, 2022 at 11:47:47 +0000:
> Jun Omae wrote on Sat, Feb 05, 2022 at 00:28:51 +0900:
> > After r1897443, prop_tests.py 12 is failing on Windows.
> > 
> > svntest.Failure: Repository diskpath in stderr: [
> > "svn: E175008: While handling the 'cash-sound' property on '/svn-test-work/repositories/prop_tests-12/!svn/bln/0':\n",
> > 'svn: E175008: Revprop change blocked by pre-revprop-change hook (exit code 1) with output:\n',
> > 'pre-revprop-change D:\\a\\subversion\\subversion\\Release\\subversion\\tests\\cmdline\\svn-test-work\\repositories\\prop_tests-12 0 jrandom cash-sound A\n']
> > FAIL:  prop_tests.py 12: set, get, and delete a revprop change
> > 
> ⋮
> > I think we could use %2 .. %5 variables instead of %* variable.
> ⋮
> > Or we could simply remove `cmd_alternative` parameter.
> 
> Thanks, Jun.  As I don't have a Windows build environment and we don't have
> Windows buildbots either, so I can't debug this myself.
> 
> Both of your alternative patches seem reasonable.  I like how the latter
> removes the need to maintain two separate implementations (.py and .bat)
> of identical functionality.
> 
> However, at the end of the day, I'll be happy with any solution that
> works for Windows-based devs.
> 
> Could someone test and commit those Windows-only patches, please?

Ping?  Any Windows developer has a minute to test & commit either of the
patches in the grandparent post?

Cheers,

Daniel

Re: svn commit: r1897443 - in /subversion/trunk/subversion/tests/cmdline/svntest: actions.py main.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jun Omae wrote on Sat, Feb 05, 2022 at 00:28:51 +0900:
> After r1897443, prop_tests.py 12 is failing on Windows.
> 
> svntest.Failure: Repository diskpath in stderr: [
> "svn: E175008: While handling the 'cash-sound' property on '/svn-test-work/repositories/prop_tests-12/!svn/bln/0':\n",
> 'svn: E175008: Revprop change blocked by pre-revprop-change hook (exit code 1) with output:\n',
> 'pre-revprop-change D:\\a\\subversion\\subversion\\Release\\subversion\\tests\\cmdline\\svn-test-work\\repositories\\prop_tests-12 0 jrandom cash-sound A\n']
> FAIL:  prop_tests.py 12: set, get, and delete a revprop change
> 
⋮
> I think we could use %2 .. %5 variables instead of %* variable.
⋮
> Or we could simply remove `cmd_alternative` parameter.

Thanks, Jun.  As I don't have a Windows build environment and we don't have
Windows buildbots either, so I can't debug this myself.

Both of your alternative patches seem reasonable.  I like how the latter
removes the need to maintain two separate implementations (.py and .bat)
of identical functionality.

However, at the end of the day, I'll be happy with any solution that
works for Windows-based devs.

Could someone test and commit those Windows-only patches, please?

Thanks!

Daniel
(not 100% here today)

Re: svn commit: r1897443 - in /subversion/trunk/subversion/tests/cmdline/svntest: actions.py main.py

Posted by Jun Omae <ju...@gmail.com>.
Hi,

After r1897443, prop_tests.py 12 is failing on Windows.

svntest.Failure: Repository diskpath in stderr: [
"svn: E175008: While handling the 'cash-sound' property on '/svn-test-work/repositories/prop_tests-12/!svn/bln/0':\n",
'svn: E175008: Revprop change blocked by pre-revprop-change hook (exit code 1) with output:\n',
'pre-revprop-change D:\\a\\subversion\\subversion\\Release\\subversion\\tests\\cmdline\\svn-test-work\\repositories\\prop_tests-12 0 jrandom cash-sound A\n']
FAIL:  prop_tests.py 12: set, get, and delete a revprop change

On 2022/01/25 18:59, danielsh@apache.org wrote:
> Author: danielsh
> Date: Tue Jan 25 09:59:40 2022
> New Revision: 1897443
> 
> URL: http://svn.apache.org/viewvc?rev=1897443&view=rev
> Log:
> Stop encoding a test's number in the svntest library.
> 
> * subversion/tests/cmdline/svntest/main.py
>    (run_command_stdin): Stop hardcoding an exception for "prop_tests-12".
> 
> * subversion/tests/cmdline/svntest/actions.py
>    (disable_revprop_changes): Stop emitting pre-revprop-change's argv[1].
>      This function is only used by prop_tests.py 12 revprop_change(), and that
>      function doesn't care about the value of argv[1].
> 
> Modified:
>      subversion/trunk/subversion/tests/cmdline/svntest/actions.py
>      subversion/trunk/subversion/tests/cmdline/svntest/main.py
> 
> Modified: subversion/trunk/subversion/tests/cmdline/svntest/actions.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/actions.py?rev=1897443&r1=1897442&r2=1897443&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/svntest/actions.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/svntest/actions.py Tue Jan 25 09:59:40 2022
> @@ -2134,9 +2134,10 @@ def disable_revprop_changes(repo_dir):
>     main.create_python_hook_script(hook_path,
>                                    'import sys\n'
>                                    'sys.stderr.write("pre-revprop-change %s" %'
> -                                                  ' " ".join(sys.argv[1:]))\n'
> +                                                  ' " ".join(sys.argv[2:]))\n'
>                                    'sys.exit(1)\n',
>                                    cmd_alternative=
> +                                       '@shift\n'
>                                          '@echo pre-revprop-change %* 1>&2\n'
>                                          '@exit 1\n')

The @shift has been added to the batch script, however it doesn't change %* variable.

See https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/shift

> The shift command has no effect on the %* batch parameter.


I think we could use %2 .. %5 variables instead of %* variable.

[[[
Index: subversion/tests/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/cmdline/svntest/actions.py	(revision 1897758)
+++ subversion/tests/cmdline/svntest/actions.py	(working copy)
@@ -2137,8 +2137,8 @@ def disable_revprop_changes(repo_dir):
                                                    ' " ".join(sys.argv[2:]))\n'
                                   'sys.exit(1)\n',
                                   cmd_alternative=
-                                       '@shift\n'
-                                       '@echo pre-revprop-change %* 1>&2\n'
+                                       '@echo pre-revprop-change %2 %3 %4 %5 '
+                                       '1>&2\n'
                                         '@exit 1\n')
  
  def create_failing_post_commit_hook(repo_dir):
]]]

Or we could simply remove `cmd_alternative` parameter.

[[[
Index: subversion/tests/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/cmdline/svntest/actions.py	(revision 1897758)
+++ subversion/tests/cmdline/svntest/actions.py	(working copy)
@@ -2135,11 +2135,7 @@ def disable_revprop_changes(repo_dir):
                                   'import sys\n'
                                   'sys.stderr.write("pre-revprop-change %s" %'
                                                    ' " ".join(sys.argv[2:]))\n'
-                                 'sys.exit(1)\n',
-                                 cmd_alternative=
-                                       '@shift\n'
-                                       '@echo pre-revprop-change %* 1>&2\n'
-                                       '@exit 1\n')
+                                 'sys.exit(1)\n')
  
  def create_failing_post_commit_hook(repo_dir):
    """Create a post-commit hook script in the repository at REPO_DIR that always
]]]


-- 
Jun Omae <ju...@gmail.com> (大前 潤)