[merge][0.11] run_bzr(working_dir='foo')

[Martin Pool]

On 21 Sep 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
> The attached patch adds 'working_dir' as parameters for most of the
> run_bzr* functions (run_bzr, run_bzr_captured, run_bzr_subprocess,
> start_bzr_subprocess).
> 
> It also cleans up test_selftest a little bit, because some tests were
> added to the wrong base class. And tests that all parameters from
> run_bzr get passed to run_bzr_captured.
> 
> Robert mentioned that he would like something like this. And since I've
> been writing a few patches which could make use of it, I figured I would
> implement it before I wrote too many more patches.

I'd like to add it too.

On the whole I would say this isn't the sort of thing that's a candidate
for coming in during freeze week though, unless it's needed to test
something else that must be fixed.

> @@ -822,6 +824,12 @@
>              stdout=stdout,
>              stderr=stderr)
>          bzrlib.ui.ui_factory.stdin = stdin
> +
> +        cwd = None
> +        if working_dir is not None:
> +            cwd = osutils.getcwd()
> +            os.chdir(working_dir)
> +
>          try:
>              result = self.apply_redirected(stdin, stdout, stderr,
>                                             bzrlib.commands.run_bzr_catch_errors,
> @@ -829,6 +837,8 @@
>          finally:
>              logger.removeHandler(handler)
>              bzrlib.ui.ui_factory = old_ui_factory
> +            if cwd is not None:
> +                os.chdir(cwd)
>  
>          out = stdout.getvalue()
>          err = stderr.getvalue()

It's somewhat cleaner to unwind different resources from different
finally blocks -- just in case an exception is raised inside
removeHandler.  (Unlikely, so this case isn't a big deal.)

> === modified file bzrlib/tests/blackbox/test_selftest.py
> --- bzrlib/tests/blackbox/test_selftest.py
> +++ bzrlib/tests/blackbox/test_selftest.py
> @@ -86,8 +86,47 @@
>  
>  class TestRunBzr(ExternalBase):
>  

> -    def run_bzr_captured(self, argv, retcode=0, encoding=None, stdin=None):
> +    def run_bzr_captured(self, argv, retcode=0, encoding=None, stdin=None,
> +                         working_dir=None):
> +        self.argv = argv
> +        self.retcode = retcode
> +        self.encoding = encoding
>          self.stdin = stdin
> +        self.working_dir = working_dir

Personally I'd really like a comment or docstring here explaining that
it's being overridden and why, e.g.

  Intercept calls to run_bzr_captured.

  Normally this runs bzr's commandline interface in-process.  But in
  these tests, we just record how it was called and don't actually run it,
  to observe how run_bzr calls it.


-- 
Martin