I will be posting an updated patch in a separate email. While working
on it, I found a couple of things that I wanted to bring up:
1) In cmdline/svntest/main.py (function: run_one), there is a comment
that says "don't trust the exitcode [from spawn_process()], will not
be correct on Windows". Python docs say that wait() is supported on
Unix only, so this may be the reason. I'll try to dig around for more
info, but if exit codes cannot be reliably gotten on Windows, must I
wrap a check against "main.is_posix_os()" around the exit-code
verification?
2) There is an existing passage in run_and_verify_svn() that reads:
want_err = None
if expected_stderr is not None and expected_stderr is not []:
want_err = True
I'm pretty sure this is a bug, and the second condition should be
"expected_stderr != []". ("[] is not []" returns true.) But the
effect appears to be that the place where the test fails is moved from
main.run_command_stdin() to verify.verify_outputs(), so it probably
isn't too big of a deal. Still, should I provide a separate patch for
this?
Thanks,
Jeremy
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
Received on 2008-02-28 16:34:43 CET