You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/03/04 13:56:18 UTC

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

Is anyone able to review Jeremy's patch for the test suite, or test it 
(especially on Windows)?

Some brief comments on the log message:
   (1) There's no indication that any of this is platform-specific; is it? 
(Does it work properly on Windows, as a possible problem was mentioned earlier?)
   (2) I'd add your second paragraph below ("This has all...") into the log 
message.
   (3) Where you write "Capture exit_code ..." that kind of implies saving it, 
but in fact it gets ignored; perhaps you could use words like "Accept and 
ignore the exit code" instead.

These things are just niceties for when the patch gets accepted and committed.

- Julian


jeremy hinds wrote:
> 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.
> ]]]
[...]

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by jeremy hinds <je...@gmail.com>.
On Wed, Mar 5, 2008 at 11:43 PM, jeremy hinds <je...@gmail.com> wrote:
> First of all, I must apologize for some of these formatting issues.  I
>  scripted the return-value-acceptance changes with intentions of
>  manually fixing the formatting after I got everything working.

My statement here is unclear.  What I meant to say was not that I
intentionally submitted an incomplete patch, but that many of the
changes did not receive the individual attention I intended to give
them before submitting them for review.  Anyway, thanks for your
patience and diligence.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Julian Foad <ju...@btopenworld.com>.
jeremy hinds wrote:
> First of all, I must apologize for some of these formatting issues.  I
> scripted the return-value-acceptance changes with intentions of
> manually fixing the formatting after I got everything working.

No problem!

> Everything else has been duly noted, and the issues are corrected (I
> hope) in this version.

Yes, it looks good and works for me on Linux, so I've committed it as r29753. 
Thank you very much for doing this.

- Julian


> 
> 
> [[[
> Allow testing of application exit codes.
> 
> This makes all of the lower-level process-running functions in
> cmdline/svntest/main.py and cmdline/svntest/actions.py return
> the exit code, all of the "run_and_verify_*()" functions guess the
> expected exit code based on whether or not output on stderr is
> expected, and new "run_and_verify_*2()" functions allow the
> expected exit code to be provided explicitly.
> 
> On platforms without the Popen3 Python class (e.g. Windows), exit
> codes are returned as None, and therefore disregarded during
> validation.
[...]


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by jeremy hinds <je...@gmail.com>.
First of all, I must apologize for some of these formatting issues.  I
scripted the return-value-acceptance changes with intentions of
manually fixing the formatting after I got everything working.
Everything else has been duly noted, and the issues are corrected (I
hope) in this version.


[[[
Allow testing of application exit codes.

This makes all of the lower-level process-running functions in
cmdline/svntest/main.py and cmdline/svntest/actions.py return
the exit code, all of the "run_and_verify_*()" functions guess the
expected exit code based on whether or not output on stderr is
expected, and new "run_and_verify_*2()" functions allow the
expected exit code to be provided explicitly.

On platforms without the Popen3 Python class (e.g. Windows), exit
codes are returned as None, and therefore disregarded during
validation.


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

 (create_repos): Accept exit_code returned from run_command.

 (run_one): Clarify a comment concerning exit codes

* 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, run_and_verify_diff_summarize_xml,
  run_and_verify_log_xml, run_and_verify_merge2,
  run_and_verify_mergeinfo, run_and_verify_switch, run_and_validate_lock,
  check_prop, inject_conflict_into_wc, run_and_verify_export):
  Accept exit_code value returned from main.run_svn, main.run_command,
  main.run_svnadmin, main.run_command_stdin, actions.run_and_verify_svn

* subversion/tests/cmdline/svntest/tree.py
 (get_props): Accept 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.  Comparison is not performed if expected
  exit code is None.

* subversion/tests/cmdline/cat_tests.py,
  subversion/tests/cmdline/lock_tests.py,
  subversion/tests/cmdline/stat_tests.py:
  Accept exit code returned from run_*(), replacing those calls with the
  run_*2() counterparts for cases where stderr output is produced while
  exiting 0.

* subversion/tests/cmdline/prop_tests.py:
  Accept exit code returned from the run_*() function calls, and verify
  exit codes along with each verify_output().

* subversion/tests/cmdline/authz_tests.py,
  subversion/tests/cmdline/autoprop_tests.py,
  subversion/tests/cmdline/basic_tests.py,
  subversion/tests/cmdline/blame_tests.py,
  subversion/tests/cmdline/changelist_tests.py,
  subversion/tests/cmdline/checkout_tests.py,
  subversion/tests/cmdline/commit_tests.py,
  subversion/tests/cmdline/copy_tests.py,
  subversion/tests/cmdline/depth_tests.py,
  subversion/tests/cmdline/diff_tests.py,
  subversion/tests/cmdline/getopt_tests.py,
  subversion/tests/cmdline/import_tests.py,
  subversion/tests/cmdline/log_tests.py,
  subversion/tests/cmdline/merge_tests.py,
  subversion/tests/cmdline/revert_tests.py,
  subversion/tests/cmdline/schedule_tests.py,
  subversion/tests/cmdline/special_tests.py,
  subversion/tests/cmdline/svnadmin_tests.py,
  subversion/tests/cmdline/svndumpfilter_tests.py,
  subversion/tests/cmdline/svnlook_tests.py,
  subversion/tests/cmdline/svnsync_tests.py,
  subversion/tests/cmdline/svnversion_tests.py,
  subversion/tests/cmdline/switch_tests.py,
  subversion/tests/cmdline/update_tests.py:
  Accept exit code returned from the run_*() function calls.

]]]


On Wed, Mar 5, 2008 at 12:53 PM, Paul Burba <pt...@gmail.com> wrote:
> On Tue, Mar 4, 2008 at 7:29 PM, jeremy hinds <je...@gmail.com> wrote:
>  > Thanks for offering to test these changes on Windows.  I've modified
>  >  the rest of the test scripts to accept the returned exit-code and made
>  >  a few other changes and fixes.  Platforms without the Popen3 class
>  >  have an exit code of "None" passed around, so Windows compatibility
>  >  should be handled by doing no exit-code validation when it is None.
>
>
>  Hi Jeremy,
>
>  The tests work fine on Windows.  The patch looks fine* except for some
>  minor issues listed below.
>
>  *I can't confirm that it works on platforms with Popen3(), it
>  certainly looks as if it should!  Hopefully some other committer can
>  check that.
>
>
>  >  If I generalized too much in the commit message for the individual
>  >  test scripts, let me know and I'll provide more detail about what was
>  >  changed.
>
>  A few minor formatting notes.  As described here,
>  http://subversion.tigris.org/hacking.html#other-conventions, we try to
>  keep lines to 79 columns or less.  There are several places in your
>  patch where you exceed this.  No big deal, but please fix.  Also, some
>  of the lines following your edits are no longer indented properly.  If
>  all the arguments to a method/function don't fit on one 80 character
>  line, then we put the extra args on the following line, indented to
>  line up with the arguments on the first line.
>
>  Here is an example of what I mean in update_tests.py:
>
>  @@ -1822,7 +1823,7 @@
>    url2 = sbox.repo_url + '/A2/mu'
>
>    # Remember the original text of the file
>  -  text_r1, err = svntest.actions.run_and_verify_svn(None, None, [],
>  +  exit_code, text_r1, err = svntest.actions.run_and_verify_svn(None, None, [],
>                                                      'cat', '-r1', url)
>
>  Do this instead:
>
>  +  exit_code, text_r1, err = svntest.actions.run_and_verify_svn(None, None, [],
>  -                                                    'cat', '-r1', url)
>  +                                                               'cat', '-r1',
>  +                                                                url)
>
>  I know there are already some examples where we don't do this, but I'd
>  prefer not to add to these cases.
>
>
>  2nd nitpick: If the opening paren of the function only is almost at 80
>  characters and would necessitate ugly splitting of individual args,
>  you can put the first arg on the next line, for example (also from
>  update_tests.py) instead of this:
>
>  @@ -2062,7 +2063,7 @@
>    I_url = sbox.repo_url + "/A/C/I"
>    os.remove(I_path)
>    os.mkdir(I_path)
>  -  so, se = svntest.actions.run_and_verify_svn("Unexpected error during co",
>  +  exit_code, so, se = svntest.actions.run_and_verify_svn("Unexpected
>  error during co",
>                                                ['Checked out revision 2.\n'],
>                                                [], "co", I_url, I_path)
>
>  This is fine:
>
>  +  exit_code, so, se = svntest.actions.run_and_verify_svn(
>  +    "Unexpected error during co",
>  +    ['Checked out revision 2.\n'],
>  +    [], "co", I_url, I_path)
>
>  Lastly, while I know we occasionally use the line continuation '\' in
>  our Python scripts and we have no specific prohibition against it, a
>  quick survey of committers on #svn-dev showed a decided preference for
>  not using them.  So changes like the following...
>
>  Index: subversion/tests/cmdline/basic_tests.py
>  ===================================================================
>  --- subversion/tests/cmdline/basic_tests.py     (revision 29693)
>  +++ subversion/tests/cmdline/basic_tests.py     (working copy)
>  @@ -189,17 +189,20 @@
>    # Unversioned paths, those that are not immediate children of a versioned
>    # path, are skipped and do not raise an error
>    xx_path = os.path.join(wc_dir, 'xx', 'xx')
>  -  out, err = svntest.actions.run_and_verify_svn("update xx/xx",
>  +  exit_code, out, err = \
>  +             svntest.actions.run_and_verify_svn("update xx/xx",
>                                                  ["Skipped
>  '"+xx_path+"'\n"], [],
>                                                  'update', xx_path)
>
>  ...would be better as:
>
>  +  exit_code, out, err = svntest.actions.run_and_verify_svn(
>  +    "update xx/xx", ["Skipped '"+xx_path+"'\n"], [], 'update', xx_path)
>
>
>  >  [[[
>  >  Allow testing of application exit codes.
>  >
>  >  This makes all of the lower-level process-running functions in
>  >  cmdline/svntest/main.py and cmdline/svntest/actions.py return
>  >  the exit code, all of the "run_and_verify_*()" functions guess the
>  >  expected exit code based on whether or not output on stderr is
>  >  expected, and new "run_and_verify_*2()" functions allow the
>  >
>  > expected exit code to be provided explicitly.
>  >
>  >  On platforms without the Popen3 Python class (e.g. Windows), exit
>  >  codes are returned as None, and therefore disregarded during
>  >  validation.
>  >
>  >
>  >
>  >  * 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.
>
>  Mention run_command_stdin() too.
>
>
>
>  >   (create_repos): Accept exit_code returned from run_command.
>  >
>  >   (run_one): Clarify a comment concerning exit codes
>  >
>  >
>  >  * 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, run_and_verify_diff_summarize_xml,
>  >   run_and_verify_log_xml, run_and_verify_merge2,
>  >   run_and_verify_mergeinfo, run_and_verify_switch, run_and_validate_lock,
>  >   check_prop, inject_conflict_into_wc, run_and_verify_export):
>  >   Accept exit_code value returned from main.run_svn, main.run_command,
>  >   main.run_svnadmin, main.run_command_stdin, actions.run_and_verify_svn
>  >
>  >  * subversion/tests/cmdline/svntest/tree.py
>  >   (get_props): Accept 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.  Comparison is not performed if expected
>  >   exit code is None.
>  >
>  >  * subversion/tests/cmdline/cat_tests.py,
>  >   subversion/tests/cmdline/lock_tests.py,
>  >   subversion/tests/cmdline/stat_tests.py:
>  >   Accept exit code returned from run_*(), replacing those calls with the
>  >   run_*2() counterparts for cases where stderr output is produced while
>  >   exiting 0.
>
>  In the the three preceeding tests you frequently declare
>  'expected_exit = 0', then pass it to
>  svntest.actions.run_and_verify_svn2().  Why not just pass 0 to
>  svntest.actions.run_and_verify_svn2()?
>
>
>  >  * subversion/tests/cmdline/authz_tests.py,
>  >   subversion/tests/cmdline/autoprop_tests.py,
>  >   subversion/tests/cmdline/basic_tests.py,
>  >   subversion/tests/cmdline/blame_tests.py,
>  >   subversion/tests/cmdline/changelist_tests.py,
>  >   subversion/tests/cmdline/checkout_tests.py,
>  >   subversion/tests/cmdline/commit_tests.py,
>  >   subversion/tests/cmdline/copy_tests.py,
>  >   subversion/tests/cmdline/depth_tests.py,
>  >   subversion/tests/cmdline/diff_tests.py,
>  >   subversion/tests/cmdline/getopt_tests.py,
>  >   subversion/tests/cmdline/import_tests.py,
>  >   subversion/tests/cmdline/log_tests.py,
>  >   subversion/tests/cmdline/merge_tests.py,
>  >   subversion/tests/cmdline/prop_tests.py,
>
>  Please break out prop_tests.py separately since you are accepting the
>  exit code returned from the run_*() function calls *and* verifying it.
>
>
>  >   subversion/tests/cmdline/revert_tests.py,
>  >   subversion/tests/cmdline/schedule_tests.py,
>  >   subversion/tests/cmdline/special_tests.py,
>  >   subversion/tests/cmdline/svnadmin_tests.py,
>  >   subversion/tests/cmdline/svndumpfilter_tests.py,
>  >   subversion/tests/cmdline/svnlook_tests.py,
>  >   subversion/tests/cmdline/svnsync_tests.py,
>  >   subversion/tests/cmdline/svnversion_tests.py,
>  >   subversion/tests/cmdline/switch_tests.py,
>  >   subversion/tests/cmdline/update_tests.py:
>  >   Accept exit code returned from the run_*() function calls.
>  >
>  >  ]]]
>
>
>  >  Index: subversion/tests/cmdline/svntest/actions.py
>  >  ===================================================================
>  >  --- subversion/tests/cmdline/svntest/actions.py (revision 29693)
>
> >  +++ 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,78 @@
>
> >
>  >   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."""
>
>  While you mention in verify.py:verify_exit_code() that the exit code
>  is not checked on platforms not supporting Popen3(), I think it is
>  worth mentioning the same in each of the run_and_verify* functions
>  here.  You only have to explain it in detail once (probably in
>  run_and_verify_svn2), then put a note in the other functions comments
>  like "Exit code is not checked on platforms without Popen3 - see note
>  in run_and_verify_svn2".  I just want it obvious what the limitations
>  are and not assume folks are going to drill down to
>  verify_exit_code().
>
>
>
>  >  +  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,
>  >  +                                    expected_exit)
>
> >  +
>  >  +def run_and_verify_svnversion2(message, wc_dir, repo_url,
>  >  +                               expected_stdout, expected_stderr,
>  >  +                               expected_exit):
>
>
> >    "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 +208,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 +219,28 @@
>  >    if expected_stderr is not None and expected_stderr != []:
>
> >      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
>
>                     ^^^
>              run_and_verify_svn2 no?
>
>
>
>  >  match
>  >  +  EXPECTED_STDOUT."""
>  >  +
>  >    if expected_stderr is None:
>  >      raise verify.SVNIncorrectDatatype("expected_stderr must not be
>  >  None")
>  >
>
>  Paul
>

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

Posted by Paul Burba <pt...@gmail.com>.
On Tue, Mar 4, 2008 at 7:29 PM, jeremy hinds <je...@gmail.com> wrote:
> Thanks for offering to test these changes on Windows.  I've modified
>  the rest of the test scripts to accept the returned exit-code and made
>  a few other changes and fixes.  Platforms without the Popen3 class
>  have an exit code of "None" passed around, so Windows compatibility
>  should be handled by doing no exit-code validation when it is None.


Hi Jeremy,

The tests work fine on Windows.  The patch looks fine* except for some
minor issues listed below.

*I can't confirm that it works on platforms with Popen3(), it
certainly looks as if it should!  Hopefully some other committer can
check that.

>  If I generalized too much in the commit message for the individual
>  test scripts, let me know and I'll provide more detail about what was
>  changed.

A few minor formatting notes.  As described here,
http://subversion.tigris.org/hacking.html#other-conventions, we try to
keep lines to 79 columns or less.  There are several places in your
patch where you exceed this.  No big deal, but please fix.  Also, some
of the lines following your edits are no longer indented properly.  If
all the arguments to a method/function don't fit on one 80 character
line, then we put the extra args on the following line, indented to
line up with the arguments on the first line.

Here is an example of what I mean in update_tests.py:

@@ -1822,7 +1823,7 @@
   url2 = sbox.repo_url + '/A2/mu'

   # Remember the original text of the file
-  text_r1, err = svntest.actions.run_and_verify_svn(None, None, [],
+  exit_code, text_r1, err = svntest.actions.run_and_verify_svn(None, None, [],
                                                     'cat', '-r1', url)

Do this instead:

+  exit_code, text_r1, err = svntest.actions.run_and_verify_svn(None, None, [],
-                                                    'cat', '-r1', url)
+                                                               'cat', '-r1',
+                                                                url)

I know there are already some examples where we don't do this, but I'd
prefer not to add to these cases.


2nd nitpick: If the opening paren of the function only is almost at 80
characters and would necessitate ugly splitting of individual args,
you can put the first arg on the next line, for example (also from
update_tests.py) instead of this:

@@ -2062,7 +2063,7 @@
   I_url = sbox.repo_url + "/A/C/I"
   os.remove(I_path)
   os.mkdir(I_path)
-  so, se = svntest.actions.run_and_verify_svn("Unexpected error during co",
+  exit_code, so, se = svntest.actions.run_and_verify_svn("Unexpected
error during co",
                                               ['Checked out revision 2.\n'],
                                               [], "co", I_url, I_path)

This is fine:

+  exit_code, so, se = svntest.actions.run_and_verify_svn(
+    "Unexpected error during co",
+    ['Checked out revision 2.\n'],
+    [], "co", I_url, I_path)

Lastly, while I know we occasionally use the line continuation '\' in
our Python scripts and we have no specific prohibition against it, a
quick survey of committers on #svn-dev showed a decided preference for
not using them.  So changes like the following...

Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py	(revision 29693)
+++ subversion/tests/cmdline/basic_tests.py	(working copy)
@@ -189,17 +189,20 @@
   # Unversioned paths, those that are not immediate children of a versioned
   # path, are skipped and do not raise an error
   xx_path = os.path.join(wc_dir, 'xx', 'xx')
-  out, err = svntest.actions.run_and_verify_svn("update xx/xx",
+  exit_code, out, err = \
+             svntest.actions.run_and_verify_svn("update xx/xx",
                                                 ["Skipped
'"+xx_path+"'\n"], [],
                                                 'update', xx_path)

...would be better as:

+  exit_code, out, err = svntest.actions.run_and_verify_svn(
+    "update xx/xx", ["Skipped '"+xx_path+"'\n"], [], 'update', xx_path)

>  [[[
>  Allow testing of application exit codes.
>
>  This makes all of the lower-level process-running functions in
>  cmdline/svntest/main.py and cmdline/svntest/actions.py return
>  the exit code, all of the "run_and_verify_*()" functions guess the
>  expected exit code based on whether or not output on stderr is
>  expected, and new "run_and_verify_*2()" functions allow the
>
> expected exit code to be provided explicitly.
>
>  On platforms without the Popen3 Python class (e.g. Windows), exit
>  codes are returned as None, and therefore disregarded during
>  validation.
>
>
>
>  * 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.

Mention run_command_stdin() too.

>   (create_repos): Accept exit_code returned from run_command.
>
>   (run_one): Clarify a comment concerning exit codes
>
>
>  * 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, run_and_verify_diff_summarize_xml,
>   run_and_verify_log_xml, run_and_verify_merge2,
>   run_and_verify_mergeinfo, run_and_verify_switch, run_and_validate_lock,
>   check_prop, inject_conflict_into_wc, run_and_verify_export):
>   Accept exit_code value returned from main.run_svn, main.run_command,
>   main.run_svnadmin, main.run_command_stdin, actions.run_and_verify_svn
>
>  * subversion/tests/cmdline/svntest/tree.py
>   (get_props): Accept 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.  Comparison is not performed if expected
>   exit code is None.
>
>  * subversion/tests/cmdline/cat_tests.py,
>   subversion/tests/cmdline/lock_tests.py,
>   subversion/tests/cmdline/stat_tests.py:
>   Accept exit code returned from run_*(), replacing those calls with the
>   run_*2() counterparts for cases where stderr output is produced while
>   exiting 0.

In the the three preceeding tests you frequently declare
'expected_exit = 0', then pass it to
svntest.actions.run_and_verify_svn2().  Why not just pass 0 to
svntest.actions.run_and_verify_svn2()?

>  * subversion/tests/cmdline/authz_tests.py,
>   subversion/tests/cmdline/autoprop_tests.py,
>   subversion/tests/cmdline/basic_tests.py,
>   subversion/tests/cmdline/blame_tests.py,
>   subversion/tests/cmdline/changelist_tests.py,
>   subversion/tests/cmdline/checkout_tests.py,
>   subversion/tests/cmdline/commit_tests.py,
>   subversion/tests/cmdline/copy_tests.py,
>   subversion/tests/cmdline/depth_tests.py,
>   subversion/tests/cmdline/diff_tests.py,
>   subversion/tests/cmdline/getopt_tests.py,
>   subversion/tests/cmdline/import_tests.py,
>   subversion/tests/cmdline/log_tests.py,
>   subversion/tests/cmdline/merge_tests.py,
>   subversion/tests/cmdline/prop_tests.py,

Please break out prop_tests.py separately since you are accepting the
exit code returned from the run_*() function calls *and* verifying it.

>   subversion/tests/cmdline/revert_tests.py,
>   subversion/tests/cmdline/schedule_tests.py,
>   subversion/tests/cmdline/special_tests.py,
>   subversion/tests/cmdline/svnadmin_tests.py,
>   subversion/tests/cmdline/svndumpfilter_tests.py,
>   subversion/tests/cmdline/svnlook_tests.py,
>   subversion/tests/cmdline/svnsync_tests.py,
>   subversion/tests/cmdline/svnversion_tests.py,
>   subversion/tests/cmdline/switch_tests.py,
>   subversion/tests/cmdline/update_tests.py:
>   Accept exit code returned from the run_*() function calls.
>
>  ]]]


>  Index: subversion/tests/cmdline/svntest/actions.py
>  ===================================================================
>  --- subversion/tests/cmdline/svntest/actions.py (revision 29693)
>  +++ 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,78 @@
>
>   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."""

While you mention in verify.py:verify_exit_code() that the exit code
is not checked on platforms not supporting Popen3(), I think it is
worth mentioning the same in each of the run_and_verify* functions
here.  You only have to explain it in detail once (probably in
run_and_verify_svn2), then put a note in the other functions comments
like "Exit code is not checked on platforms without Popen3 - see note
in run_and_verify_svn2".  I just want it obvious what the limitations
are and not assume folks are going to drill down to
verify_exit_code().

>  +  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,
>  +                                    expected_exit)
>  +
>  +def run_and_verify_svnversion2(message, wc_dir, repo_url,
>  +                               expected_stdout, expected_stderr,
>  +                               expected_exit):
>    "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 +208,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 +219,28 @@
>    if expected_stderr is not None and expected_stderr != []:
>      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

                    ^^^
             run_and_verify_svn2 no?

>  match
>  +  EXPECTED_STDOUT."""
>  +
>    if expected_stderr is None:
>      raise verify.SVNIncorrectDatatype("expected_stderr must not be
>  None")
>

Paul

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by jeremy hinds <je...@gmail.com>.
Thanks for offering to test these changes on Windows.  I've modified
the rest of the test scripts to accept the returned exit-code and made
a few other changes and fixes.  Platforms without the Popen3 class
have an exit code of "None" passed around, so Windows compatibility
should be handled by doing no exit-code validation when it is None.

If I generalized too much in the commit message for the individual
test scripts, let me know and I'll provide more detail about what was
changed.

[[[
Allow testing of application exit codes.

This makes all of the lower-level process-running functions in
cmdline/svntest/main.py and cmdline/svntest/actions.py return
the exit code, all of the "run_and_verify_*()" functions guess the
expected exit code based on whether or not output on stderr is
expected, and new "run_and_verify_*2()" functions allow the
expected exit code to be provided explicitly.

On platforms without the Popen3 Python class (e.g. Windows), exit
codes are returned as None, and therefore disregarded during
validation.


* 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): Accept exit_code returned from run_command.

 (run_one): Clarify a comment concerning exit codes

* 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, run_and_verify_diff_summarize_xml,
  run_and_verify_log_xml, run_and_verify_merge2,
  run_and_verify_mergeinfo, run_and_verify_switch, run_and_validate_lock,
  check_prop, inject_conflict_into_wc, run_and_verify_export):
  Accept exit_code value returned from main.run_svn, main.run_command,
  main.run_svnadmin, main.run_command_stdin, actions.run_and_verify_svn

* subversion/tests/cmdline/svntest/tree.py
 (get_props): Accept 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.  Comparison is not performed if expected
 exit code is None.

* subversion/tests/cmdline/cat_tests.py,
  subversion/tests/cmdline/lock_tests.py,
  subversion/tests/cmdline/stat_tests.py:
  Accept exit code returned from run_*(), replacing those calls with the
  run_*2() counterparts for cases where stderr output is produced while
  exiting 0.

* subversion/tests/cmdline/authz_tests.py,
  subversion/tests/cmdline/autoprop_tests.py,
  subversion/tests/cmdline/basic_tests.py,
  subversion/tests/cmdline/blame_tests.py,
  subversion/tests/cmdline/changelist_tests.py,
  subversion/tests/cmdline/checkout_tests.py,
  subversion/tests/cmdline/commit_tests.py,
  subversion/tests/cmdline/copy_tests.py,
  subversion/tests/cmdline/depth_tests.py,
  subversion/tests/cmdline/diff_tests.py,
  subversion/tests/cmdline/getopt_tests.py,
  subversion/tests/cmdline/import_tests.py,
  subversion/tests/cmdline/log_tests.py,
  subversion/tests/cmdline/merge_tests.py,
  subversion/tests/cmdline/prop_tests.py,
  subversion/tests/cmdline/revert_tests.py,
  subversion/tests/cmdline/schedule_tests.py,
  subversion/tests/cmdline/special_tests.py,
  subversion/tests/cmdline/svnadmin_tests.py,
  subversion/tests/cmdline/svndumpfilter_tests.py,
  subversion/tests/cmdline/svnlook_tests.py,
  subversion/tests/cmdline/svnsync_tests.py,
  subversion/tests/cmdline/svnversion_tests.py,
  subversion/tests/cmdline/switch_tests.py,
  subversion/tests/cmdline/update_tests.py:
  Accept exit code returned from the run_*() function calls.

]]]




On Tue, Mar 4, 2008 at 10:56 AM, Paul Burba <pt...@gmail.com> wrote:
> On Tue, Mar 4, 2008 at 8:56 AM, Julian Foad <ju...@btopenworld.com> wrote:
>  > Is anyone able to review Jeremy's patch for the test suite, or test it
>  >  (especially on Windows)?
>
>  I'll try it out on Windows.  But first, Jeremy, could you please
>  resend your patch as an attachment?  When the patch is embedded within
>  the text of the e-mail, the lines get wrapped at 80 characters.  I
>  tried to fix these manually, but I'm still getting strange errors when
>  applying the patch.  Admittedly I'm using a nightly build of
>  TortoiseSVN to do this, so the problem might be there, but it would be
>  nice to eliminate the possibility I've mangled the patch.  In general
>  it's best to send patches as attachments - see
>  http://subversion.tigris.org/hacking.html#patches.
>
>  Thanks!
>
>  Paul
>
>
>
>  >  Some brief comments on the log message:
>  >    (1) There's no indication that any of this is platform-specific; is it?
>  >  (Does it work properly on Windows, as a possible problem was mentioned earlier?)
>  >    (2) I'd add your second paragraph below ("This has all...") into the log
>  >  message.
>  >    (3) Where you write "Capture exit_code ..." that kind of implies saving it,
>  >  but in fact it gets ignored; perhaps you could use words like "Accept and
>  >  ignore the exit code" instead.
>  >
>  >  These things are just niceties for when the patch gets accepted and committed.
>  >
>  >  - Julian
>  >
>  >
>  >
>  >
>  >  jeremy hinds wrote:
>  >  > 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.
>  >  > ]]]
>  >  [...]
>  >
>  >
>  >
>  >  ---------------------------------------------------------------------
>  >  To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>  >  For additional commands, e-mail: dev-help@subversion.tigris.org
>  >
>  >
>

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

Posted by Paul Burba <pt...@gmail.com>.
On Tue, Mar 4, 2008 at 8:56 AM, Julian Foad <ju...@btopenworld.com> wrote:
> Is anyone able to review Jeremy's patch for the test suite, or test it
>  (especially on Windows)?

I'll try it out on Windows.  But first, Jeremy, could you please
resend your patch as an attachment?  When the patch is embedded within
the text of the e-mail, the lines get wrapped at 80 characters.  I
tried to fix these manually, but I'm still getting strange errors when
applying the patch.  Admittedly I'm using a nightly build of
TortoiseSVN to do this, so the problem might be there, but it would be
nice to eliminate the possibility I've mangled the patch.  In general
it's best to send patches as attachments - see
http://subversion.tigris.org/hacking.html#patches.

Thanks!

Paul

>  Some brief comments on the log message:
>    (1) There's no indication that any of this is platform-specific; is it?
>  (Does it work properly on Windows, as a possible problem was mentioned earlier?)
>    (2) I'd add your second paragraph below ("This has all...") into the log
>  message.
>    (3) Where you write "Capture exit_code ..." that kind of implies saving it,
>  but in fact it gets ignored; perhaps you could use words like "Accept and
>  ignore the exit code" instead.
>
>  These things are just niceties for when the patch gets accepted and committed.
>
>  - Julian
>
>
>
>
>  jeremy hinds wrote:
>  > 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.
>  > ]]]
>  [...]
>
>
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>  For additional commands, e-mail: dev-help@subversion.tigris.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org