Code review comment for lp:~pwlars/lava-test/fixtures

Revision history for this message
James Westby (james-w) wrote :

Hi,

Looks good, thanks.

I would call them "FakeOutput" and "FakeConfig" or similar, as they aren't
really Tests anymore. I would also rename the module to "fixtures" or "testing".

87 + def cleanup(self):
88 + for fixture in self._fixtures:
89 + fixture.tearDown()
90 + self._fixtures = []

Why not make that the tearDown method, to save subclasses having to do
it explicitly?

111 class ListInstalled(FakeConfigTests):

I don't think that's what you want any more is it?

138 + def setUp(self):

You should upcall from the setUp and tearDown whenever you implement
them, to save hitting weird issues if the hierarchy changes.

Thanks,

James

review: Approve

« Back to merge proposal