launchpad-reviewers team mailing list archive

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

Raphaël Badin has proposed merging lp:~rvb/maas/contains-all-matcher-bug-989385 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #989385 in MAAS: "Test matcher: ContainsAll"
  https://bugs.launchpad.net/maas/+bug/989385

For more details, see:
https://code.launchpad.net/~rvb/maas/contains-all-matcher-bug-989385/+merge/103885

No pre-imp call for this but the bug report is very explicit and this is a tiny improvement.

This branch adds a test matcher: ContainsAll and uses it in tests instead of MatchesAll(*[Contains(mac) for item in items])).
-- 
https://code.launchpad.net/~rvb/maas/contains-all-matcher-bug-989385/+merge/103885
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/contains-all-matcher-bug-989385 into lp:maas.

=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py	2012-04-23 14:02:51 +0000
+++ src/maasserver/tests/test_views.py	2012-04-27 14:16:59 +0000
@@ -34,11 +34,8 @@
     )
 from maasserver.views import HelpfulDeleteView
 from maasserver.views.nodes import NodeEdit
+from maastesting.matchers import ContainsAll
 from provisioningserver.enum import PSERV_FAULT
-from testtools.matchers import (
-    Contains,
-    MatchesAll,
-    )
 
 
 class Test404500(LoggedInTestCase):
@@ -271,7 +268,5 @@
             response = self.client.get(link)
             self.assertThat(
                 response.content,
-                MatchesAll(
-                    *[Contains(
-                          escape(error.faultString))
-                     for error in errors]))
+                ContainsAll(
+                    [escape(error.faultString) for error in errors]))

=== modified file 'src/maasserver/tests/test_views_nodes.py'
--- src/maasserver/tests/test_views_nodes.py	2012-04-27 10:56:37 +0000
+++ src/maasserver/tests/test_views_nodes.py	2012-04-27 14:16:59 +0000
@@ -26,11 +26,11 @@
     )
 from maasserver.exceptions import NoRabbit
 from maasserver.forms import NodeActionForm
-from maasserver.node_action import StartNode
 from maasserver.models import (
     MACAddress,
     Node,
     )
+from maasserver.node_action import StartNode
 from maasserver.testing import (
     get_content_links,
     reload_object,
@@ -45,12 +45,9 @@
     )
 from maasserver.views import nodes as nodes_views
 from maasserver.views.nodes import get_longpoll_context
+from maastesting.matchers import ContainsAll
 from maastesting.rabbit import uses_rabbit_fixture
 from provisioningserver.enum import POWER_TYPE_CHOICES
-from testtools.matchers import (
-    Contains,
-    MatchesAll,
-    )
 
 
 class NodeViewsTest(LoggedInTestCase):
@@ -228,9 +225,7 @@
         ]
         node_edit_link = reverse('node-edit', args=[node.system_id])
         response = self.client.get(node_edit_link)
-        self.assertThat(
-            response.content,
-            MatchesAll(*[Contains(mac) for mac in macs]))
+        self.assertThat(response.content, ContainsAll(macs))
 
     def test_edit_nodes_contains_links_to_delete_the_macaddresses(self):
         node = factory.make_node(owner=self.logged_in_user)
@@ -242,10 +237,9 @@
         response = self.client.get(node_edit_link)
         self.assertThat(
             response.content,
-            MatchesAll(
-                *[Contains(
-                    reverse('mac-delete', args=[node.system_id, mac]))
-                    for mac in macs]))
+            ContainsAll(
+                [reverse('mac-delete', args=[node.system_id, mac])
+                for mac in macs]))
 
     def test_edit_nodes_contains_link_to_add_a_macaddresses(self):
         node = factory.make_node(owner=self.logged_in_user)

=== added file 'src/maastesting/matchers.py'
--- src/maastesting/matchers.py	1970-01-01 00:00:00 +0000
+++ src/maastesting/matchers.py	2012-04-27 14:16:59 +0000
@@ -0,0 +1,25 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""testtools custom matchers"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'ContainsAll',
+]
+
+from testtools.matchers import (
+    MatchesAll,
+    Contains,
+    )
+
+
+def ContainsAll(items):
+    """Matches if the matchee contains all the provided items."""
+    return MatchesAll(*[Contains(item) for item in items], first_only=False)

=== added file 'src/maastesting/tests/test_matchers.py'
--- src/maastesting/tests/test_matchers.py	1970-01-01 00:00:00 +0000
+++ src/maastesting/tests/test_matchers.py	2012-04-27 14:16:59 +0000
@@ -0,0 +1,34 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test matchers."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from maastesting.matchers import ContainsAll
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from testtools.matchers import MismatchError
+
+
+class TestContainsAll(TestCase):
+
+    def test_ContainsAll_passes_if_all_elements_are_present(self):
+        items = [factory.getRandomString() for i in range(3)]
+        self.assertThat(items, ContainsAll([items[0], items[2]]))
+        # No exception.
+
+    def test_ContainsAll_raises_if_one_element_is_missing(self):
+        items = [factory.getRandomString() for i in range(3)]
+        self.assertRaises(
+            MismatchError,
+            self.assertThat,
+            items,
+            ContainsAll([items[0], factory.getRandomString()]))