help needed in fixing test failure (#138600)

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Parth Malwankar wrote:
> So I was trying to come up with a fix for #138600 [1]
> i.e. `bzr mkdir` create new dir before it checks if dir is a valid branch.
> 
> This seemed like a simple enough fix[2] so I created the patch and the
> test case (diff below). Note that for the test case there is no run_bzr("init")
> so I expect mkdir to fail (retcode=3).
> 
> Interesting, if I try the patch from the command prompt everything
> works as expected (the dir is not created). However, the test case
> fails with the debug prints:
> isdir foo returns:  False
> isdir abc returns:  True
> 
> The output of the manual run seems to work:
> 
> [138600]% bzr version-info
> bzr: ERROR: Not a branch: "/home/parthm/tmp/b/138600/".
> [138600]% ~/src/bzr.dev/138600/bzr --no-plugins mkdir foo baz baz
> bzr: ERROR: Not a branch: "/home/parthm/tmp/b/138600/foo/".
> bzr: warning: some compiled extensions could not be loaded; see
> <https://answers.launchpad.net/bzr/+faq/703>
> [138600]% ls
> [138600]%
> 
> What I am doing wrong here?

The reason this is failing, is because the test infrastructure
automatically creates a WorkingTree at the root of the temp directory,
so that we don't accidentally have a bzr command 'leak' into the
containing workspace. (We also analyze the WT after each test to make
sure that the test didn't modify the tree, otherwise we fail so we know
to fix that test.)

In your test, it does return 'retcode=3' but it does so because of:
.../work/abc is not versioned.

The actual failure is that it cannot add 'abc' because 'work/' is not
versioned.

The only way I can think of to isolate the way you want is to create a
branch or repository without a working tree inbetween you and the test
infrastructure tree. So something like:

self.make_branch('no_tree')
self.run_bzr('no_tree/abc')

I think that should make the test work for you.

> 
> Thanks,
> Parth
> 
> [1] https://bugs.launchpad.net/bzr/+bug/138600
> [2] https://code.launchpad.net/~parthm/bzr/138600
> 
> [blackbox]% bzr cdiff -r -3
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py	2010-02-12 04:33:05 +0000
> +++ bzrlib/builtins.py	2010-02-16 08:28:57 +0000
> @@ -682,8 +682,8 @@
> 
>      def run(self, dir_list):
>          for d in dir_list:
> +            wt, dd = WorkingTree.open_containing(d)
>              os.mkdir(d)
> -            wt, dd = WorkingTree.open_containing(d)
>              wt.add([dd])
>              self.outf.write('added %s\n' % d)
> 
> 
> === modified file 'bzrlib/tests/blackbox/test_versioning.py'
> --- bzrlib/tests/blackbox/test_versioning.py	2009-08-28 05:00:33 +0000
> +++ bzrlib/tests/blackbox/test_versioning.py	2010-02-16 09:24:13 +0000
> @@ -23,7 +23,7 @@
>  import os
> 
>  from bzrlib.branch import Branch
> -from bzrlib.osutils import pathjoin
> +from bzrlib.osutils import pathjoin, isdir
>  from bzrlib.tests import TestCaseInTempDir, TestCaseWithTransport
>  from bzrlib.trace import mutter
>  from bzrlib.workingtree import WorkingTree
> @@ -121,6 +121,14 @@
>          from bzrlib.check import check
>          check(b, False)
> 
> +    def test_mkdir_invalid(self):
> +        """Basic 'bzr mkdir' operation should fail if !branch. Fix #138600"""
> +
> +        self.run_bzr('mkdir abc', retcode=3)
> +        print "isdir foo returns: ", isdir('foo')
> +        print "isdir abc returns: ", isdir('abc')
> +        self.assertFalse(isdir('foo'))
> +        self.assertFalse(isdir('abc'))

^- Better here would be "self.failIfExists('foo')"

> 
>  class SubdirCommit(TestCaseWithTransport):
> 
> 

John
=:_>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkt6u+kACgkQJdeBCYSNAANoRgCdFMeFypsLVMNYj/9napwCOTvO
UNoAoJa4a2/bBPDbDfqoVR7ZYb+K8XuY
=OXfz
-----END PGP SIGNATURE-----