launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

[7]

433	+    def test_give(self):
434	+        user1 = factory.make_user()
435	+        user2 = factory.make_user()
436	+        profile = user1.get_profile()
437	+        consumer, token = profile.create_authorisation_token()
438	+        give_credentials_to_user(user1, user2)
439	+        self.assertEqual(user2, reload_object(consumer).user)
440	+        self.assertEqual(user2, reload_object(token).user)
441	+
442	+    def test_consumer_and_token_saved(self):
443	+        user1 = factory.make_user()
444	+        user2 = factory.make_user()
445	+        profile = user1.get_profile()
446	+        consumer, token = profile.create_authorisation_token()
447	+        self.patch(consumer, "save")
448	+        self.patch(token, "save")
449	+        give_credentials_to_user(user1, user2)
450	+        consumer.save.assert_called_once()
451	+        token.save.assert_called_once()

I don't really see what the second test adds… since you're reloading the objects in the first test and you're getting what you expect, you can be sure the objects have been saved… ?

[8]

456	+    def test_give(self):
457	+        user1 = factory.make_user()
458	+        user2 = factory.make_user()
459	+        node = factory.make_node(owner=user1)
460	+        give_node_to_user(node, user2)
461	+        self.assertEqual(user2, node.owner)
462	+
463	+    def test_node_saved(self):
464	+        user1 = factory.make_user()
465	+        user2 = factory.make_user()
466	+        node = factory.make_node(owner=user1)
467	+        save = self.patch(node, "save")
468	+        give_node_to_user(node, user2)
469	+        save.assert_called_once()

Similar remark here.  Although I'd refactor the first test to reload the object before checking the owner, and I'd get rid of the second test.
-- 
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.