> [9] > > 580 + self.assertEqual( > 581 + (legacy_user, legacy_user, legacy_user, legacy_user), > > "(legacy_user, legacy_user, legacy_user, legacy_user)" => "[legacy_user] * 4" > ? > > I think it's more readable but this is really a detail.
We'll agree to differ on this one :) > [10] > > 558 + self.assertEqual(user1, reload_object(node1).owner) > 559 + self.assertEqual(user1, reload_object(node2).owner) > > Probably better to make that one check. Done. > > [11] > > 472 +class TestMigrateToUser(TestCase): > 473 + > 474 + def test_migrate(self): > > That test is a bit… huge :) . Don't you think it could test for behavior > instead of patching methods… and maybe share some code with > TestMigrate.test_migrate_ancillary_data_to_legacy_user_when_multiple_users ? The behaviour of the individual components is tested elsewhere, and the overall behaviour is tested in TestMigrate, so I sought only to show that things were wired up correctly. In a way it's also a reminder not to put more logic into migrate_to_user(); it should be factored into separate functions. It was a bit of a what-if experiment too. I've added comments to the function to explain its rationale, but I'm open to more discussion. It's interesting to consider the value of this kind of test; perhaps not very much. -- https://code.launchpad.net/~allenap/maas/shared-to-per-tenant-storage/+merge/151858 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/shared-to-per-tenant-storage into lp:maas. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
