You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2022/01/25 11:31:22 UTC

svntest.main.run_svn() used incorrectly

Look:

     1	% ag --py 'run_svn[(](?!None|False|True|0|1|$)' subversion/tests/cmdline/  
     2	subversion/tests/cmdline/blame_tests.py:131:  svntest.main.run_svn(binary_mime_type_on_text_file_warning,
     3	subversion/tests/cmdline/blame_tests.py:135:  exit_code, output, errput = svntest.main.run_svn(2, 'blame', iota)
     4	subversion/tests/cmdline/blame_tests.py:143:  exit_code, output, errput = svntest.main.run_svn(2, 'blame', iota)
     5	subversion/tests/cmdline/blame_tests.py:155:  exit_code, output, errput = svntest.main.run_svn(2, 'blame', iota + '@3')
     6	subversion/tests/cmdline/blame_tests.py:160:  exit_code, output, errput = svntest.main.run_svn(2, 'blame', '--force',
     7	subversion/tests/cmdline/svnlook_tests.py:572:  svntest.main.run_svn(binary_mime_type_on_text_file_warning,
     8	subversion/tests/cmdline/changelist_tests.py:302:  exit_code, output, errput = svntest.main.run_svn(".*", "changelist", "bar",
     9	subversion/tests/cmdline/changelist_tests.py:318:  exit_code, output, errput = svntest.main.run_svn(".*", "changelist", "baz",
    10	subversion/tests/cmdline/changelist_tests.py:332:  exit_code, output, errput = svntest.main.run_svn(".*", "changelist",
    11	subversion/tests/cmdline/tree_conflict_tests.py:1537:  main.run_svn("Tree conflict on '%s'" % sbox.ospath("A1/B2"),
    12	subversion/tests/cmdline/prop_tests.py:601:  svntest.main.run_svn(binary_mime_type_on_text_file_warning,
    13	subversion/tests/cmdline/redirect_tests.py:81:  exit_code, out, err = svntest.main.run_svn('.*moved temporarily.*',
    14	subversion/tests/cmdline/redirect_tests.py:83:  exit_code, out, err = svntest.main.run_svn('.*moved temporarily.*',
    15	subversion/tests/cmdline/redirect_tests.py:85:  exit_code, out, err = svntest.main.run_svn('.*moved temporarily.*',
    16	subversion/tests/cmdline/redirect_tests.py:88:  exit_code, out, err = svntest.main.run_svn('.*moved temporarily.*',
    17	subversion/tests/cmdline/schedule_tests.py:453:  svntest.main.run_svn(svntest.verify.AnyOutput, 'rm', file1_path)
    18	subversion/tests/cmdline/basic_tests.py:2666:  svntest.main.run_svn(wc_dir, 'rm', sbox.ospath('A/B/E/alpha'))
    19	subversion/tests/cmdline/basic_tests.py:2679:  svntest.main.run_svn(wc_dir, 'rm', sbox.ospath('A/B/E'))
    20	subversion/tests/cmdline/externals_tests.py:2787:  svntest.main.run_svn("W205011: Error handling externals definition for '%s'"
    21	subversion/tests/cmdline/diff_tests.py:1737:  svntest.main.run_svn(binary_mime_type_on_text_file_warning,
    22	subversion/tests/cmdline/commit_tests.py:218:  svntest.main.run_svn(binary_mime_type_on_text_file_warning,
    23	subversion/tests/cmdline/svntest/actions.py:344:  """Invoke main.run_svn() with *VARARGS. Return exit code as int; stdout,
    24	subversion/tests/cmdline/svntest/actions.py:378:  exit_code, out, err = main.run_svn(want_err, *varargs)
    25	subversion/tests/cmdline/svntest/actions.py:1353:    exit_code, out_dry, err_dry = main.run_svn(error_re_string,
    26	subversion/tests/cmdline/svntest/actions.py:1432:  exit_code, out, err = main.run_svn(error_re_string, *mergeinfo_command)
    27	subversion/tests/cmdline/svntest/actions.py:2195:    exit_code, out, err = main.run_svn(expected_re_string, *propset)
    28	subversion/tests/cmdline/svntest/factory.py:493:    code, out, err = main.run_svn("Maybe", *runargs)
    29	subversion/tests/cmdline/svntest/factory.py:577:    code, output, err = main.run_svn("Maybe", 'ci',
    30	subversion/tests/cmdline/svntest/factory.py:619:    code, output, err = main.run_svn('Maybe', 'up', *runargs)
    31	subversion/tests/cmdline/svntest/factory.py:680:    code, output, err = main.run_svn('Maybe', 'sw',
    32	subversion/tests/cmdline/svntest/factory.py:747:    code, output, err = main.run_svn('Maybe', 'co',
    33	subversion/tests/cmdline/merge_tests.py:1732:  svntest.main.run_svn('Working copy not locked',
    34	subversion/tests/cmdline/merge_tests.py:1735:  svntest.main.run_svn('Working copy not locked',
    35	subversion/tests/cmdline/merge_tests.py:17194:    svntest.main.run_svn(binary_mime_type_on_text_file_warning,
    36	subversion/tests/cmdline/svntest/main.py:814:def run_svn(error_expected, *varargs):
    37	subversion/tests/cmdline/info_tests.py:499:  svntest.main.run_svn(binary_mime_type_on_text_file_warning,
    38	subversion/tests/cmdline/copy_tests.py:4882:  svntest.main.run_svn(wc_dir, 'copy',
    39	subversion/tests/cmdline/copy_tests.py:4893:  svntest.main.run_svn(wc_dir, 'rm', sbox.ospath('A/B/E-copied/alpha'))
    40	subversion/tests/cmdline/copy_tests.py:4899:    svntest.main.run_svn(wc_dir, 'revert', '--recursive',
    41	subversion/tests/cmdline/copy_tests.py:4903:    svntest.main.run_svn(wc_dir, 'rm', '--force', sbox.ospath('A/B/E-copied'))
    42	subversion/tests/cmdline/copy_tests.py:4912:  svntest.main.run_svn(wc_dir, 'copy',
    43	subversion/tests/cmdline/copy_tests.py:4937:  svntest.main.run_svn(wc_dir, 'rm', sbox.ospath('A/B/E'))
    44	subversion/tests/cmdline/copy_tests.py:4943:  svntest.main.run_svn(wc_dir, 'copy',
    45	subversion/tests/cmdline/copy_tests.py:4955:  svntest.main.run_svn(wc_dir, 'rm', '--force', sbox.ospath('A/B/E'))

The problem?  run_svn()'s first parameter is a boolean.  Passing an
expected error message or wc_dir to it doesn't have the expected
results.  Most of the above uses are buggy or misleading.

We should audit uses of run_svn() and related functions.

Cheers,

Daniel

P.S. Note that uses of run_svn() where the opening parentheses is
followed by a newline were excluded from the grep.

Re: svntest.main.run_svn() used incorrectly

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Tue, Jan 25, 2022 at 11:31:22 +0000:
> The problem?  run_svn()'s first parameter is a boolean.  Passing an
> expected error message or wc_dir to it doesn't have the expected
> results.  Most of the above uses are buggy or misleading.
> 
> We should audit uses of run_svn() and related functions.

And afterwards, perhaps something like this:

[[[
Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py	(revision 1897453)
+++ subversion/tests/cmdline/svntest/main.py	(working copy)
@@ -668,6 +668,9 @@ def run_command_stdin(command, error_expected, buf
       brief_command = ' '.join(((command,) + varargs)[:4]) + ' ...'
     raise Failure('Command failed: "' + brief_command +
                   '"; exit code ' + str(exit_code))
+  if error_expected and (not stderr_lines) and exit_code == 0:
+    raise Failure('Command unexpectedly succeeded: ' +
+                  repr(brief_command) + '; exit code ' + str(exit_code))
 
   return exit_code, \
          filter_dbg(stdout_lines, binary_mode), \
]]]

Or perhaps assert that ERROR_EXPECTED is one of None/True/False.