[MERGE] cleanup blackbox tests

[Vincent Ladeuil]

>>>>> "mbp" == Martin Pool <mbp at sourcefrog.net> writes:

    mbp> On 6/27/07, Martin Pool <mbp at sourcefrog.net> wrote:
    >> It's also good when we need to build up a command from constituent
    >> parts - I would say it's safer never to be doing a string join but
    >> always a list join:
    >> 
    >> not  run_bzr("commit " + filename)
    >> but  run_bzr(["commit", filename])

    mbp> OK, I see that's what you're still doing.

Yes. I think your definitions were quite clear:

- use string syntax when it's convenient,

- use list syntax when you *build* the command or the command
  contain some white spaces.

    >> > Note: using commit messages without embedded spaces is the most
    >> > useful trick-of-the-day :-)
    >> 
    >> I don't think we should change them to not use spaces in the test case
    >> as part of this patch, if that's what you're doing.  If you just mean
    >> that when you're interactively committing you can leave off the quotes
    >> then yes, that's true.

    mbp> OK, I see you did do that in the tests.  Personally I might have left
    mbp> them as lists or escaped the space with a backslash.  But we're not
    mbp> actually testing the parameter in those cases, so I don't object.

On a second thought, I should have use the list syntax. If
anybody objects I will revert those.

    mbp> @@ -88,23 +88,23 @@
    mbp>     def test_remove_tree_lightweight_checkout_explicit(self):
    mbp>         self.tree.branch.create_checkout('branch2', lightweight=True)
    mbp>         self.failUnlessExists('branch2/foo')
    mbp> -        output = self.run_bzr(
    mbp> -            ["Cannot remove working tree from lightweight checkout"],
    mbp> -            'remove-tree', 'branch2', retcode=3)
    mbp> +        output = self.run_bzr_error(
    mbp> +            ["You cannot remove the working tree from a lightweight checkout"],
    mbp> +            'remove-tree branch2', retcode=3)
    mbp>         self.failUnlessExists('branch2/foo')
    mbp>         self.failUnlessExists('branch1/foo')

    mbp> (In passing) I don't understand how the old code ever worked - it's
    mbp> passing the expected error first but run_bzr expects it to be a kwarg.

I think it interpreted the error message it as the command to
run, fails to run it, get a '3' retcode and was happy (or
something like that).

That's a pretty strong argument against the varargs syntax.

    mbp> Well, it's a good reason to clean it up.

Yup.

    mbp> I'll merge mine now as John reviewed it, and give other people a
    mbp> chance to comment on Vincent's changes.


Ok. That way I can send aaron my bzrtools patch as soon as your
patch is merged.

      Vincent