[MERGE] Improve test_sprout_preserves_kind

[John Arbash Meinel]

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

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>> I believe your contention that it "uses it if it has been specified" is
>> incorrect, because AFAIK there is no way to leave a BzrDir.branch_format
>> unspecified.
> 
> Sure there is.  You can accept the default.  This test is specifically
> doing self.make_repository('target', format=self.bzrdir_format)
> 

Unspecified to me, would mean bzrdir.branch_format = None, and then
Branch.sprout(bzrdir) could see that there isn't a format, and then use
self.

>> If you create a BzrDir it auto-specifies the default.
> 
> "auto-specifies the default" seems like a contradiction in terms to me,
> which is why I phrased it that way earlier.

The difference is when "bzrdir.branch_format" gets set. As it stands, it
doesn't say None to indicate that Branch.sprout should set it. It always
has a value (and if someone else didn't get to it first, it is set to
BranchFormat.get_default_format):

class BzrDirMetaFormat1(BzrDirFormat):
...
  def get_branch_format(self):
      if self._branch_format is None:
          from bzrlib.branch import BranchFormat
          self._branch_format = BranchFormat.get_default_format()
      return self._branch_format


> 
>> And
>> Branch.sprout() always takes an existing BzrDir to sprout into.
> 
> I hadn't twigged that we were actually testing Branch.sprout, not
> BzrDir.sprout.  I don't think it's a very clear test of the format
> support, because it's assuming that the BzrDir from the repo has the
> same branch format as self.bzrdir_format.
> 
> I think it would be much better to explicitly create a bzrdir, make a
> repo in it, and sprout into it.
> 
> i.e.
> target_bzrdir = self.make_bzrdir('target') # no redundant format=
> target_bzrdir.create_repository()
> target = source.sprout(target_bzrdir)

I might prefer an explicit "make_bzrdir('target',
format=self.bzrdir_format", but I can certainly go along with this.


> 
>> We discussed it a bit in Sydney, and I think an ideal api would have the
>> primary "sprout" be on Branch rather than on BzrDir
> 
> Agreed.  Sprouting a repository doesn't make sense.
> 
> Actually, I think a command object is the way to go, but it should
> definitely take a branch and produce a branch.
> 
>> Does the term:
>>
>>   test_sprout_uses_bzrdir_branch_format(self): ?
>>
>> Sound better to you?
> 
> Yes, it sounds great.
> 
> Aaron
> 

I'll clean up the patch and submit it. As it seems like we've reached
consensus.

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

iEYEARECAAYFAkkjA8UACgkQJdeBCYSNAANxLQCff/Cvgbk6BUjHU8cyoNKbm3SF
B/EAnixuuqdFQVEe5V9Re0OR6KAEW+wx
=pkPg
-----END PGP SIGNATURE-----