Code review comment for lp:~jml/testtools/extract-factory

Revision history for this message
Robert Collins (lifeless) wrote :

I meant 'does the factory need to be a test case attribute'. To me, the factory is a multipurpose fixture, and fixtures are very usable (e.g. via scenarios) without being exposed on the testcase by default.

From the point of view of compat with prior testtools releases we can't drop our additional methods, but we can avoid adding an overly specific factory instance on every TestCase object. So putting it on e.g. __factory. That would let the new factory be used by other testing styles (e.g. nose), or evolved separately to TestCase.

Beyond that there is an interesting discussion about whether global factories are a good idea, or whether per test state is a better idea, and a little orthogonally, whether the factory should be a Fixture (which can then provide details etc) or an adhoc API.

So the .factory is the only thing that really needs fixing, I'll mark this approved if you change factory to __factory.

review: Approve

« Back to merge proposal