[MERGE] Add support for previewing the result of a merge
John Arbash Meinel
john at arbash-meinel.com
Fri Jan 4 14:37:54 GMT 2008
Andrew Bennetts wrote:
> John Arbash Meinel wrote:
>> Aaron Bentley wrote:
> [...]
>>>> Is it always going to be implemented that way? I realize these are
>>>> directly testing PreviewTree. It just seems like we should be testing
>>>> the general API for that function.
>>> That seems like over-testing to me. All of the functionality unique to
>>> _PreviewTree._iter_changes is tested. All of the functionality of
>>> TreeTransform._iter_changes is also tested.
>>>
>>> At some point, we may try to make _PreviewTree into a more general tree
>>> type. And then, this test will break, and rightly so. But then we'll
>>> be able to add it to the InterTree._iter_changes tests.
>>>
>>> So no, I don't think testing the API makes sense.
>> I think as a general rule, testing the API leads to better (less
>> fragile) tests. It doesn't always lead to as much coverage, but that
>> might indicate the API needs to be designed differently. (Look at my
>> recent discussions with Vincent on SFTP readv, and how it needs to be
>> broken up, which is partly exposed because testing different code paths
>> is difficult.)
>>
>> Nothing I would strictly block on, but I would be interested to hear
>> what Andrew or Robert have to say on the subject.
>
> It sounds like the key question here is “if/when the implementation of this
> changes, is it likely that there will be an unnoticed gap in the test
> coverage?”
>
> Aaron seems confident that the likely changes to this method won't do that;
> i.e. that the risk of future bugs going unnoticed is low. I'm not familiar
> enough with this code to have any idea if that's a sound assessment or not,
> (although I have written a generator elsewhere in bzrlib that won't fail
> until the first iteration, but that doesn't necessarily have any relevance
> to this code).
>
> So John, do you think there's a real risk that a bug could be introduced and
> unnoticed?
>
> (yes, this is basically a cop-out “no opinion” from me ;)
>
> -Andrew.
>
>
It was more a discussion of testing strategies than anything specific to
this code.
He seems to be adequately covering the cases, and if it breaks in the
future, we could switch it to 'assertListRaises' at that point.
John
=:->
More information about the bazaar
mailing list