Re: [PATCH] Allow testing of svn client exit codes

Here is the revised patch for review. Corresponding changes to the
test scripts aren't included yet (though a few are in the attachment,
so that these changes can actually be executed).

This has all of the lower-level process-running functions returning
the exit code, all of the "run_and_verify_*()" functions guessing the
expected exit code, and new "run_and_verify_*2()" functions allowing
the expected exit code to be provided explicitly.

[[[
Allow testing of application exit codes.

* subversion/tests/cmdline/svntest/main.py
 (run_command, run_svn, run_svnadmin, run_svnlook, run_svnsync,
  run_svnversion): Include exit_code in the returned tuple.

 (create_repos): Capture exit_code returned from run_command.

* subversion/tests/cmdline/svntest/actions.py
 (run_and_verify_svnlook2, run_and_verify_svnadmin2,
  run_and_verify_svnversion2, run_and_verify_svn2,
  run_and_verify_svn_match_any2): New, execute the indicated binary
  and check actual outputs and exit code against the expected value
  parameters.

 (run_and_verify_svnlook, run_and_verify_svnadmin,
  run_and_verify_svnversion, run_and_verify_svn,
  run_and_verify_svn_match_any): Guess whether the expected exit should
  be 0 or 1 based on whether output is expected on stderr. Then invoke
  the coresponding run_and_verify_*2 function with that value. Return
  exit_code, stdout_lines, stderr_lines.

 (setup_pristine_repository, run_and_verify_load, run_and_verify_dump,
  run_and_verify_checkout, run_and_verify_update, run_and_verify_commit,
  run_and_verify_status, run_and_verify_unquiet_status,
  run_and_verify_diff_summarize, check_prop, inject_conflict_into_wc):
  Capture exit_code value returned from main.run_svn, main.run_command,
  main.run_svnadmin, main.run_command_stdin

* subversion/tests/cmdline/svntest/tree.py
 (get_props): Capture exit code returned by main.run_svn.

* subversion/tests/cmdline/svntest/verify.py
 (SVNUnexpectedExitCode): New exception raised when the exit code was not
 what was expected.

 (verify_exit_code): New, compares expected and actual exit code and raises
 an exception if they are different.
]]]

Index: subversion/tests/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/cmdline/svntest/actions.py (revision 29604)
+++ subversion/tests/cmdline/svntest/actions.py (working copy)
@@ -54,9 +54,10 @@
     # import the greek tree, using l:foo/p:bar
     ### todo: svn should not be prompting for auth info when using
     ### repositories with no auth/auth requirements
- output, errput = main.run_svn(None, 'import',
- '-m', 'Log message for revision 1.',
- main.greek_dump_dir, main.pristine_url)
+ exit_code, output, errput = main.run_svn(None, 'import',
+ '-m', 'Log message for
revision 1.',
+ main.greek_dump_dir,
+ main.pristine_url)

     # check for any errors from the import
     if len(errput):
@@ -119,34 +120,76 @@

 def run_and_verify_svnlook(message, expected_stdout,
                            expected_stderr, *varargs):
- "Run svnlook command and check its output"
- out, err = main.run_svnlook(*varargs)
+ """Run svnlook command and check its output and exit code. The
+ expected exit code is assumed to be 0 if no output is expected on
+ stderr, and 1 otherwise."""
+ expected_exit = 0
+ if expected_stderr is not None and expected_stderr != []:
+ expected_exit = 1
+ return run_and_verify_svnlook2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs)
+
+def run_and_verify_svnlook2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs):
+ "Run svnlook command and check its output and exit code."
+ exit_code, out, err = main.run_svnlook(*varargs)
   verify.verify_outputs("Unexpected output", out, err,
                         expected_stdout, expected_stderr)
- return out, err
+ verify.verify_exit_code(message, exit_code, expected_exit)
+ return exit_code, out, err

-
 def run_and_verify_svnadmin(message, expected_stdout,
                             expected_stderr, *varargs):
+ """Run svnadmin command and check its output and exit code. The
+ expected exit code is assumed to be 0 if no output is expected on
+ stderr, and 1 otherwise."""
+ expected_exit = 0
+ if expected_stderr is not None and expected_stderr != []:
+ expected_exit = 1
+ return run_and_verify_svnadmin2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs)
+
+def run_and_verify_svnadmin2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs):
   "Run svnadmin command and check its output"
- out, err = main.run_svnadmin(*varargs)
+ exit_code, out, err = main.run_svnadmin(*varargs)
   verify.verify_outputs("Unexpected output", out, err,
                         expected_stdout, expected_stderr)
- return out, err
+ verify.verify_exit_code(message, exit_code, expected_exit)
+ return exit_code, out, err

 def run_and_verify_svnversion(message, wc_dir, repo_url,
                               expected_stdout, expected_stderr):
+ expected_exit = 0
+ if expected_stderr is not None and expected_stderr != []:
+ expected_exit = 1
+ return run_and_verify_svnversion2(message, wc_dir, repo_url,
+ expected_stdout, expected_stderr)
+
+def run_and_verify_svnversion2(message, wc_dir, repo_url,
+ expected_stdout, expected_stderr):
   "Run svnversion command and check its output"
- out, err = main.run_svnversion(wc_dir, repo_url)
+ exit_code, out, err = main.run_svnversion(wc_dir, repo_url)
   verify.verify_outputs("Unexpected output", out, err,
                         expected_stdout, expected_stderr)
- return out, err
+ verify.verify_exit_code(message, exit_code, expected_exit)
+ return exit_code, out, err

+def run_and_verify_svn(message, expected_stdout, expected_stderr, *varargs):
+ """like run_and_verify_svn2, but the expected exit code is assumed to
+ be 0 if no output is expected on stderr, and 1 otherwise."""

-def run_and_verify_svn(message, expected_stdout, expected_stderr, *varargs):
- """Invokes main.run_svn() with *VARARGS, return stdout and stderr as
- lists of lines. For both EXPECTED_STDOUT and EXPECTED_STDERR,
+ expected_exit = 0
+ if expected_stderr is not None and expected_stderr != []:
+ expected_exit = 1
+ return run_and_verify_svn2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs)
+
+def run_and_verify_svn2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs):
+ """Invokes main.run_svn() with *VARARGS, returns exit code as int, stdout
+ and stderr as lists of lines. For both EXPECTED_STDOUT and EXPECTED_STDERR,
   create an appropriate instance of verify.ExpectedOutput (if necessary):

      - If it is an array of strings, create a vanilla ExpectedOutput.
@@ -163,6 +206,8 @@
   If EXPECTED_STDOUT is None, do not check stdout.
   EXPECTED_STDERR may not be None.

+ If output checks pass, the expected and actual codes are compared.
+
   If a comparison fails, a Failure will be raised."""

   if expected_stderr is None:
@@ -172,15 +217,28 @@
   if expected_stderr is not None and expected_stderr is not []:
     want_err = True

- out, err = main.run_svn(want_err, *varargs)
+ exit_code, out, err = main.run_svn(want_err, *varargs)
   verify.verify_outputs(message, out, err, expected_stdout, expected_stderr)
- return out, err
+ verify.verify_exit_code(message, exit_code, expected_exit)
+ return exit_code, out, err

 def run_and_verify_svn_match_any(message, expected_stdout, expected_stderr,
                                  *varargs):
   """Like run_and_verify_svn, except that only one stdout line must match
   EXPECTED_STDOUT."""
+ expected_exit = 0
+ if expected_stderr is not None and expected_stderr != []:
+ expected_exit = 1
+ return run_and_verify_svn_match_any2(message, expected_stdout,
+ expected_stderr, expected_exit,
+ *varargs)
+

+def run_and_verify_svn_match_any2(message, expected_stdout, expected_stderr,
+ expected_exit, *varargs):
+ """Like run_and_verify_svn, except that only one stdout line must match
+ EXPECTED_STDOUT."""
+
   if expected_stderr is None:
     raise verify.SVNIncorrectDatatype("expected_stderr must not be None")

@@ -188,26 +246,27 @@
   if expected_stderr is not None and expected_stderr is not []:
     want_err = True

- out, err = main.run_svn(want_err, *varargs)
+ exit_code, out, err = main.run_svn(want_err, *varargs)
   verify.verify_outputs(message, out, err, expected_stdout, expected_stderr,
                         False)
- return out, err
+ verify.verify_exit_code(message, exit_code, expected_exit)
+ return exit_code, out, err

 def run_and_verify_load(repo_dir, dump_file_content):
   "Runs 'svnadmin load' and reports any errors."
   expected_stderr = []
- output, errput = \
+ exit_code, output, errput = \
           main.run_command_stdin(main.svnadmin_binary,
- expected_stderr, 1, dump_file_content,
- 'load', '--force-uuid', '--quiet', repo_dir)
+ expected_stderr, 1, dump_file_content,
+ 'load', '--force-uuid', '--quiet', repo_dir)
   verify.verify_outputs("Unexpected stderr output", None, errput,
                         None, expected_stderr)

 def run_and_verify_dump(repo_dir):
   "Runs 'svnadmin dump' and reports any errors, returning the dump content."
- output, errput = main.run_svnadmin('dump', repo_dir)
+ exit_code, output, errput = main.run_svnadmin('dump', repo_dir)
   verify.verify_outputs("Missing expected output(s)", output, errput,
                         verify.AnyOutput, verify.AnyOutput)
   return output
@@ -281,8 +340,8 @@
   # Checkout and make a tree of the output, using l:foo/p:bar
   ### todo: svn should not be prompting for auth info when using
   ### repositories with no auth/auth requirements
- output, errput = main.run_svn (None, 'co',
- URL, wc_dir_name, *args)
+ exit_code, output, errput = main.run_svn (None, 'co',
+ URL, wc_dir_name, *args)
   actual = tree.build_tree_from_checkout (output)

   # Verify actual output against expected output.
@@ -590,9 +649,11 @@

   # Update and make a tree of the output.
   if len(args):
- output, errput = main.run_svn (error_re_string, 'up', *args)
+ exit_code, output, errput = main.run_svn (error_re_string, 'up', *args)
   else:
- output, errput = main.run_svn (error_re_string, 'up', wc_dir_name, *args)
+ exit_code, output, errput = main.run_svn (error_re_string,
+ 'up', wc_dir_name,
+ *args)

   if (error_re_string):
     rm = re.compile(error_re_string)
@@ -885,9 +946,9 @@
     status_tree = status_tree.old_tree()

   # Commit.
- output, errput = main.run_svn(error_re_string, 'ci',
- '-m', 'log msg',
- *args)
+ exit_code, output, errput = main.run_svn(error_re_string, 'ci',
+ '-m', 'log msg',
+ *args)

   if error_re_string:
     if not error_re_string.startswith(".*"):
@@ -957,8 +1018,8 @@
   if isinstance(output_tree, wc.State):
     output_tree = output_tree.old_tree()

- output, errput = main.run_svn (None, 'status', '-v', '-u', '-q',
- wc_dir_name)
+ exit_code, output, errput = main.run_svn (None, 'status', '-v', '-u', '-q',
+ wc_dir_name)

   actual = tree.build_tree_from_status (output)

@@ -988,7 +1049,8 @@
   if isinstance(output_tree, wc.State):
     output_tree = output_tree.old_tree()

- output, errput = main.run_svn (None, 'status', '-v', '-u', wc_dir_name)
+ exit_code, output, errput = main.run_svn (None, 'status', '-v',
+ '-u', wc_dir_name)

   actual = tree.build_tree_from_status (output)

@@ -1092,8 +1154,8 @@
   if isinstance(output_tree, wc.State):
     output_tree = output_tree.old_tree()

- output, errput = main.run_svn (None, 'diff', '--summarize',
- *args)
+ exit_code, output, errput = main.run_svn (None, 'diff', '--summarize',
+ *args)

   if error_re_string:
     if not error_re_string.startswith(".*"):
@@ -1271,9 +1333,9 @@
 def check_prop(name, path, exp_out):
   """Verify that property NAME on PATH has a value of EXP_OUT"""
   # Not using run_svn because binary_mode must be set
- out, err = main.run_command(main.svn_binary, None, 1, 'pg', '--strict',
- name, path, '--config-dir',
- main.default_config_dir)
+ exit_code, out, err = main.run_command(main.svn_binary, None, 1, 'pg',
+ '--strict', name, path,
'--config-dir',
+ main.default_config_dir)
   if out != exp_out:
     print "svn pg --strict", name, "output does not match expected."
     print "Expected standard output: ", exp_out, "\n"
@@ -1327,8 +1389,8 @@
                         None, file_path)

   # Backdate the file.
- output, errput = main.run_svn(None, "up", "-r", str(prev_rev),
- file_path)
+ exit_code, output, errput = main.run_svn(None, "up", "-r", str(prev_rev),
+ file_path)
   if expected_status:
     expected_status.tweak(state_path, wc_rev=prev_rev)

@@ -1346,8 +1408,9 @@
                                       expected_disk, expected_status,
                                       conflicting_contents, contents,
                                       merged_rev)
- output, errput = main.run_svn(None, "up", "-r", str(merged_rev),
- sbox.repo_url + "/" + state_path, file_path)
+ exit_code, output, errput = main.run_svn(None, "up", "-r", str(merged_rev),
+ sbox.repo_url + "/" + state_path,
+ file_path)
   if expected_status:
     expected_status.tweak(state_path, wc_rev=merged_rev)

Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py (revision 29604)
+++ subversion/tests/cmdline/svntest/main.py (working copy)
@@ -304,9 +304,10 @@

   return os.path.join(repo_dir, "conf", "svnserve.conf")

-# Run any binary, logging the command line (TODO: and return code)
+# Run any binary, logging the command line and return code
 def run_command(command, error_expected, binary_mode=0, *varargs):
- """Run COMMAND with VARARGS; return stdout, stderr as lists of lines.
+ """Run COMMAND with VARARGS; return exit code as int; stdout, stderr
+ as lists of lines.
   If ERROR_EXPECTED is None, any stderr also will be printed."""

   return run_command_stdin(command, error_expected, binary_mode,
@@ -415,7 +416,7 @@
   should not be very large, as if the program outputs more than the OS
   is willing to buffer, this will deadlock, with both Python and
   COMMAND waiting to write to each other for ever.
- Return stdout, stderr as lists of lines.
+ Return exit code as int; stdout, stderr as lists of lines.
   If ERROR_EXPECTED is None, any stderr also will be printed."""

   if verbose_mode:
@@ -434,7 +435,7 @@
     map(sys.stdout.write, stderr_lines)
     raise Failure

- return stdout_lines, stderr_lines
+ return exit_code, stdout_lines, stderr_lines

 def create_config_dir(cfgdir, config_contents=None, server_contents=None):
   "Create config directories and files"
@@ -486,7 +487,8 @@

 # For running subversion and returning the output
 def run_svn(error_expected, *varargs):
- """Run svn with VARARGS; return stdout, stderr as lists of lines.
+ """Run svn with VARARGS; return exit code as int; stdout, stderr as
+ lists of lines.
   If ERROR_EXPECTED is None, any stderr also will be printed. If
   you're just checking that something does/doesn't come out of
   stdout/stderr, you might want to use actions.run_and_verify_svn()."""
@@ -495,20 +497,24 @@

 # For running svnadmin. Ignores the output.
 def run_svnadmin(*varargs):
- "Run svnadmin with VARARGS, returns stdout, stderr as list of lines."
+ """Run svnadmin with VARARGS, returns exit code as int; stdout, stderr as
+ list of lines."""
   return run_command(svnadmin_binary, 1, 0, *varargs)

 # For running svnlook. Ignores the output.
 def run_svnlook(*varargs):
- "Run svnlook with VARARGS, returns stdout, stderr as list of lines."
+ """Run svnlook with VARARGS, returns exit code as int; stdout, stderr as
+ list of lines."""
   return run_command(svnlook_binary, 1, 0, *varargs)

 def run_svnsync(*varargs):
- "Run svnsync with VARARGS, returns stdout, stderr as list of lines."
+ """Run svnsync with VARARGS, returns exit code as int; stdout, stderr as
+ list of lines."""
   return run_command(svnsync_binary, 1, 0, *(_with_config_dir(varargs)))

 def run_svnversion(*varargs):
- "Run svnversion with VARARGS, returns stdout, stderr as list of lines."
+ """Run svnversion with VARARGS, returns exit code as int; stdout, stderr
+ as list of lines."""
   return run_command(svnversion_binary, 1, 0, *varargs)

 # Chmod recursively on a whole subtree
@@ -595,7 +601,8 @@
     opts += ("--pre-1.5-compatible",)
   if fs_type is not None:
     opts += ("--fs-type=" + fs_type,)
- stdout, stderr = run_command(svnadmin_binary, 1, 0, "create", path, *opts)
+ exit_code, stdout, stderr = run_command(svnadmin_binary, 1, 0, "create",
+ path, *opts)

   # Skip tests if we can't create the repository.
   if stderr:
Index: subversion/tests/cmdline/svntest/tree.py
===================================================================
--- subversion/tests/cmdline/svntest/tree.py (revision 29604)
+++ subversion/tests/cmdline/svntest/tree.py (working copy)
@@ -321,7 +321,7 @@
   # respecting the black-box paradigm.

   props = {}
- output, errput = main.run_svn(1, "proplist", path, "--verbose")
+ exit_code, output, errput = main.run_svn(1, "proplist", path, "--verbose")

   first_value = 0
   for line in output:
Index: subversion/tests/cmdline/svntest/verify.py
===================================================================
--- subversion/tests/cmdline/svntest/verify.py (revision 29604)
+++ subversion/tests/cmdline/svntest/verify.py (working copy)
@@ -49,6 +49,11 @@
   STDERR when output was expected."""
   pass

+class SVNUnexpectedExitCode(SVNUnexpectedOutput):
+ """Exception raised if an invocation of svn exits with a value other
+ than what was expected."""
+ pass
+
 class SVNIncorrectDatatype(SVNUnexpectedOutput):
   """Exception raised if invalid input is passed to the
   run_and_verify_* API"""
@@ -324,3 +329,13 @@
       raisable = main.SVNLineUnequal

     compare_and_display_lines(message, label, expected, actual, raisable)
+
+def verify_exit_code(message, actual, expected,
+ raisable=SVNUnexpectedExitCode):
+ """Compare and display expected vs. actual exit codes:
+ if they don't match, print the difference (preceded by MESSAGE iff
+ not None) and raise an exception."""
+
+ if expected != actual:
+ display_lines(message, "Exit Code", str(expected) + '\n',
str(actual) + '\n')
+ raise raisable

On Tue, Feb 26, 2008 at 7:36 AM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> jeremy hinds wrote:
> > I am hoping to take a crack at issue 3097 (exit non-zero when running
> > `svn pg` on an unset property) in the near future. And there are a
> > few similar exit-code issues in there. So I thought a good place to
> > start is with the ability to test this sort of thing.
> >
> > I don't know if exit codes are as interesting for the other binaries,
> > so this patch only deals with the client.
>
> In my view, exit codes are just as important for the other binaries.
>
>
> > And since I am still trying
> > to familiarize myself with the conventions, and I'm no Python expert,
> > please let me know what I can do to make this better.
>
>
> > [[[
> > Allow testing of svn client exit codes.
> >
> > * subversion/tests/cmdline/svntest/actions.py
> > (run_and_verify_svn_with_exit): New function like run_and_verify_svn, but
> > with an expected exit code parameter which is checked against the actual
> > exit code.
>
> OK. Callers that expect failure or don't know what they expect should use this.
> Callers that expect success can continue to use run_and_verify_svn().
>
>
> > * subversion/tests/cmdline/svntest/main.py
> > (run_command_2): New, like run_command but also returns the exit code.
> > (run_command_stdin_2): New, like run_command_stdin, but also returns
> > the exit code.
> > (run_command_stdin): Reimplemented as a wrapper around run_command_stdin_2.
> > (run_svn_2): New, invokes svn client using run_command_2.
>
> These low level functions that just run a command (including run_command,
> run_svn, run_svnlook, etc.) should always return the exit code as well as the
> stdout and stderr lines. There's no point in having versions of these that
> don't. You can immediately change the callers that care about any of the return
> values to expect this third return value as well, even if they will currently
> ignore it (with a TODO comment). (Many of them already ignore the stderr return
> value. This is poor but we wouldn't really be making it worse.) There are only
> in the order of 100 such calls that assign the return values, I think.
>
>
> > * subversion/tests/cmdline/svntest/verify.py
> > (SVNUnexpectedExitCode): New exception raised when the exit code was not
> > what was expected.
> > (verify_exit_code): New, compares expected and actual exit code and fails
> > the test if they are different.
>
> OK
>
> Another thought:
>
> Functions that run a specific command and check the results with the
> possibility of expecting an error (like run_and_verify_commit()) could check
> for a zero exit code if they're expecting no error output, and check for
> non-zero otherwise. That would probably match the "svn" exit code policy
> closely enough for almost all tests, and very few exceptions would be needed. I
> think that would be better than having a separate exit-checking version of
> run_and_verify_commit_...() and only checking the exit code when the caller is
> specifically modified to use it.
>
>
> I'm not looking at the code, as I see other people are already doing so.
>
> - Julian
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org