> Because the mock is returned from patch(), you can avoid importing
> MagickMock and just do:
>
>[...]
Well, actually, the mock object is *not* returned by patch(…). In MAAS it is, see src/maastesting/testcase.py:MAASTestCase.patch. But in maas-test, we don't have that utility
>
> > + maas_fixture.install_maas() + + self.assertEqual( +
> > ( + [mock.call()], + [ +
> > mock.call("Installing MAAS (version %s)..." % version), +
> > mock.call("Done installing MAAS."), + ]), +
> > (mock_get_maas_version.mock_calls, mock_logging.mock_calls))
>
> I think this is definitely a case where having more than one assertion
> is better than compounding them into a single call. This is extremely
> hard to read and digest. And ease of reading trumps everything else!
All right, you know I'm a sucker for compound assertions but in this instance, you're definitely right.
> [...]
> You could make this easier to read by assigning a variable to the
> result of mock.call() and using that in the assert_has_call.
Actually, I've refactored that code a bit and it's now a bit simpler.
> Because the mock is returned from patch(), you can avoid importing
> MagickMock and just do:
>
>[...]
Well, actually, the mock object is *not* returned by patch(…). In MAAS it is, see src/maastesting /testcase. py:MAASTestCase .patch. But in maas-test, we don't have that utility
> install_ maas() + + self.assertEqual( + "Installing MAAS (version %s)..." % version), + maas_version. mock_calls, mock_logging. mock_calls) )
> > + maas_fixture.
> > ( + [mock.call()], + [ +
> > mock.call(
> > mock.call("Done installing MAAS."), + ]), +
> > (mock_get_
>
> I think this is definitely a case where having more than one assertion
> is better than compounding them into a single call. This is extremely
> hard to read and digest. And ease of reading trumps everything else!
All right, you know I'm a sucker for compound assertions but in this instance, you're definitely right.
> [...]
> You could make this easier to read by assigning a variable to the
> result of mock.call() and using that in the assert_has_call.
Actually, I've refactored that code a bit and it's now a bit simpler.
Thanks for the review!