Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests

2008/4/8 jeremy hinds <jeremy.hinds_at_gmail.com>:
>
> On Mon, Apr 7, 2008 at 9:18 AM, Branko Čibej <brane_at_xbc.nu> wrote:
> > jeremy hinds wrote:
> >
> > >
> > > > * I /think/ i recall that we have the binary-mode parameter for a
> > > > reason. I don't remember the reason offhand, but just killing off
> > > > that option doesn't seem safe. (FWIW, the reason is most likely
> > > > related to text and binary being different on Windows.)
> > > >
> > > >
> > >
> > > I think you're right, in that the binary-mode param was only used when
> > > invoking os.popen3() on platforms without the Popen3 class (Windows).
> > > But having replaced both of those with subprocess.Popen, there's
> > > nowhere to use that parameter anymore. At least, I don't see anything
> > > that looks analogous in creationflags or anywhere like that.
> > >
> > >
> >
> > universal_newlines=True is text (non-binary) mode.
> >
> > -- Brane
>
> Thanks for clearing that up. While rewriting the patch to handle
> universal_newlines and shell=False, another question came up.
>
> I made universal_newlines apply on any platform rather than just
> Windows, and that seemed to work fine on everything except
> import_tests.import_eol_style(). The problem is that
> run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
> sequences were converted to "\n". (Honestly, I don't understand how
> that test worked accurately on Windows before.)
>
> The question is whether it is better to preserve old behavior by never
> using universal_newlines on posix, or should I add a "binary_mode"
> parameter to run_and_verify_svn2()?
>

I decided err on the side of flexibility and add a "binary_mode" param
to run_and_verify_svn2(). So here is my new and improved (and
hopefully good) patch. Again, I've only been able to test this on
Linux.

[[[
In cmdline tests, use subprocess.Popen for executing commands to allow
support for exit-code checks on both Windows and posix systems. This makes
the test suite require Python version >= 2.4.

* subversion/tests/cmdline/svntest/main.py
  (global): Import subprocess instead of popen2. Remove variable
    "platform_with_popen3_class".
  (_quote_arg): Removed, since commands + args to open_pipe are now lists.
  (open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
    value. Use subprocess.Popen for spawning the child process.
  (wait_on_pipe): Interpret the return value of wait() according to
    subprocess.Popen semantics.
  (spawn_process, copy_repos): When calling open_pipe, pass the command as
    a list and binary_mode as a boolean.
  (run_svn2): New, like run_svn but with a "binary_mode" boolean param.

* subversion/tests/cmdline/svntest/actions.py
  (run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
   run_and_verify_svnadmin2, run_and_verify_svnversion,
   run_and_verify_svnversion2,
   run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
    Remove the caveat comment stating that exit-code checks are skipped
    for some platforms.
  (run_and_verify_svn): Remove the caveat comment stating that exit-code
    checks are skipped for some platforms. Pass binary_mode=0 to
    run_and_verify_svn2.
  (run_and_verify_svn2): Remove the caveat comment stating that exit-code
    checks are skipped for some platforms. Add a binary_mode boolean
    parameter. Replace call to run_svn with run_svn2.

* subversion/tests/cmdline/import_tests.py
  (import_eol_style): Call run_and_verify_svn2 with binary_mode=1.

* subversion/tests/cmdline/cat_tests.py,
  subversion/tests/cmdline/lock_tests.py,
  subversion/tests/cmdline/stat_tests.py:
    Pass binary_mode parameter to run_and_verify_svn2.

]]]

Thanks,
Jeremy

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