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 <ptburba_at_gmail.com> wrote:
> On Tue, Mar 4, 2008 at 7:29 PM, jeremy hinds <jeremy.hinds_at_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_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-06 07:43:34 CET