[MERGE] Fix stacking tests applicability
Vincent Ladeuil
v.ladeuil+lp at free.fr
Mon Dec 29 11:33:24 GMT 2008
>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:
jam> John Arbash Meinel has voted comment.
jam> Status is now: Semi-approved
jam> Comment:
jam> (sorry, I hit enter by accident)
jam> If I understand correctly, the basic change you made is to switch:
jam> target = source.bzrdir.sprout('target').open_branch()
jam> so that if it raises UnstackableBranchFormat it translates that
jam> error into "TestNotApplicable". And you did this for several
jam> different tests. Is that correct?
No.
I tried to make the loom plugin implements unstackable formats
based on the failing tests with the idea that our tests may be
buggy because we didn't really exercise them against such
formats.
Doing so I came across cases where some exceptions where raised
and were the same exceptions caught in the following lines.
And without trying to diagnose further I made the slight changes
you point out.
jam> IMO, that is not the correct fix. Consider the causes:
jam> 1) source.bzrdir.sprout('target')
jam> This doesn't seem like something that should *ever raise
jam> UnstackableBranchFormat.
But it was...
jam> If there is a "default_stack_on" policy, but the branch
jam> format doesn't support stacking, it should just not
jam> stack. And if it does support stacking, then everything
jam> should just work.
jam> Note that the individual tests can then later say stuff like
jam> "get_stacked_on_url()" should either raise an exception, or give
jam> this specific
jam> result. But, IMO, sprout() should not be failing.
jam> 2) source.bzrdir.sprout('target').open_branch()
jam> This should *definitely* never raise
jam> UnstackableBranchFormat. Once you have
jam> created the branch from sprout() you *better* be able to open
jam> it. Or we have
jam> created something explicitly broken as a result of sprout(). And
jam> I would *want*
jam> a failing test letting me know that "sprout()" is broken for this
jam> format.
Both remarks sound perfectly valid, so I suspect my rude
implementation of an unstackable loom format was buggy and the
tests caught that.
Note that Andrew proposed another approach to fix loom regarding
the failing tests for which I sent a patch that is pending review
in the loom project ;-)
So I think we should forget that approach and look at
https://bugs.launchpad.net/bugs/309730
and its consequences for looms instead.
BB:vetoed
Vincent
More information about the bazaar
mailing list