Code review comment for lp:~vila/bzr/move-test-servers

On 11 February 2010 12:48, Robert Collins <email address hidden> wrote:
> On Wed, 2010-02-10 at 23:51 +0000, Andrew Bennetts wrote:
>>
>> I think MemoryServer is useful for more than just testing, so I think
>> it should
>> stay in bzrlib.transport.memory.  It's useful for constructing a
>> virtual
>> filesystem, e.g. Launchpad uses it in non-test code for the automatic
>> bzrdirs
>> with stacking directives.
>>
>> This implies to me that the Server base class should remain in
>> bzrlib.transport
>> too.
>
> I agree with this. I'll go further and say that *most* of our servers
> are really quite useful for non-test code.  I don't think they should
> move under tests at all.
>
> I think the following *should* move under tests:
>  - servers that are unsafe to use outside of tests
>  - servers that are deliberately broken (and thus their raison d'etre is
> for testing)
>
> And these *could* move under tests:
>  - the mapping of 'To test transport T use servers A, B and C'

Perhaps we should move them all to bzrlib.servers. Then we can assert
that module is not loaded when it is not needed, and we can allow for
currently limited servers to grow into being more generally useful. I
think we have some intentionally limited transports in b.transport and
I don't see that as essentially problematic. It's more important that
the class name make its limitations clear.

--
Martin <http://launchpad.net/~mbp/>