launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:macaroon-verifies-matcher into launchpad:master.

Commit message:
Add a new MacaroonVerifies matcher

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/401311

I generalized this from CodeImportJobMacaroonVerifies.  In some cases this is more convenient than the helper methods in MacaroonTestMixin, since it's easier to compose it with other matchers.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:macaroon-verifies-matcher into launchpad:master.

diff --git a/lib/lp/code/model/tests/test_codeimportjob.py b/lib/lp/code/model/tests/test_codeimportjob.py
index d31d32f..a5f3e6e 100644
--- a/lib/lp/code/model/tests/test_codeimportjob.py
+++ b/lib/lp/code/model/tests/test_codeimportjob.py
@@ -18,10 +18,8 @@ from pymacaroons import Macaroon
 from pytz import UTC
 from testtools.matchers import (
     Equals,
-    Matcher,
     MatchesListwise,
     MatchesStructure,
-    Mismatch,
     )
 import transaction
 from zope.component import getUtility
@@ -66,7 +64,10 @@ from lp.services.macaroons.interfaces import (
     BadMacaroonContext,
     IMacaroonIssuer,
     )
-from lp.services.macaroons.testing import MacaroonTestMixin
+from lp.services.macaroons.testing import (
+    MacaroonTestMixin,
+    MacaroonVerifies,
+    )
 from lp.services.webapp import canonical_url
 from lp.testing import (
     ANONYMOUS,
@@ -95,19 +96,6 @@ def login_for_code_imports():
     return login_celebrity('vcs_imports')
 
 
-class CodeImportJobMacaroonVerifies(Matcher):
-    """Matches if a code-import-job macaroon can be verified."""
-
-    def __init__(self, code_import):
-        self.code_import = code_import
-
-    def match(self, macaroon_raw):
-        issuer = getUtility(IMacaroonIssuer, 'code-import-job')
-        macaroon = Macaroon.deserialize(macaroon_raw)
-        if not issuer.verifyMacaroon(macaroon, self.code_import.import_job):
-            return Mismatch("Macaroon '%s' does not verify" % macaroon_raw)
-
-
 class TestCodeImportJob(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
@@ -154,7 +142,7 @@ class TestCodeImportJob(TestCaseWithFactory):
                 Equals('git'), Equals('git'),
                 Equals('git://git.example.com/project.git'),
                 Equals('--macaroon'),
-                CodeImportJobMacaroonVerifies(code_import),
+                MacaroonVerifies('code-import-job', code_import.import_job),
                 Equals('--exclude-host'), Equals('launchpad.test'),
                 ]),
             # Start the job so that the macaroon can be verified.
diff --git a/lib/lp/services/macaroons/testing.py b/lib/lp/services/macaroons/testing.py
index ec987f0..dd94b3b 100644
--- a/lib/lp/services/macaroons/testing.py
+++ b/lib/lp/services/macaroons/testing.py
@@ -9,10 +9,20 @@ __metaclass__ = type
 __all__ = [
     'find_caveats_by_name',
     'MacaroonTestMixin',
+    'MacaroonVerifies',
     ]
 
+from pymacaroons import Macaroon
+
 from testtools.content import text_content
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+    Matcher,
+    MatchesStructure,
+    Mismatch,
+    )
+from zope.component import getUtility
+
+from lp.services.macaroons.interfaces import IMacaroonIssuer
 
 
 def find_caveats_by_name(macaroon, caveat_name):
@@ -21,24 +31,45 @@ def find_caveats_by_name(macaroon, caveat_name):
         if caveat.caveat_id.startswith(caveat_name + " ")]
 
 
+class MacaroonVerifies(Matcher):
+    """Matches if a macaroon can be verified."""
+
+    def __init__(self, issuer_name, context, matcher=None, **verify_kwargs):
+        super(MacaroonVerifies, self).__init__()
+        self.issuer_name = issuer_name
+        self.context = context
+        self.matcher = matcher
+        self.verify_kwargs = verify_kwargs
+
+    def match(self, macaroon_raw):
+        issuer = getUtility(IMacaroonIssuer, self.issuer_name)
+        macaroon = Macaroon.deserialize(macaroon_raw)
+        errors = []
+        verified = issuer.verifyMacaroon(
+            macaroon, self.context, errors=errors, **self.verify_kwargs)
+        if not verified:
+            return Mismatch(
+                "Macaroon '%s' does not verify" % macaroon_raw,
+                {"errors": text_content("\n".join(errors))})
+        mismatch = MatchesStructure.byEquality(
+            issuer_name=self.issuer_name).match(verified)
+        if mismatch is not None:
+            return mismatch
+        if self.matcher is not None:
+            return self.matcher.match(verified)
+
+
 class MacaroonTestMixin:
 
     def assertMacaroonVerifies(self, issuer, macaroon, context, **kwargs):
-        errors = []
-        try:
-            verified = issuer.verifyMacaroon(
-                macaroon, context, errors=errors, **kwargs)
-            self.assertIsNotNone(verified)
-            self.assertThat(verified, MatchesStructure.byEquality(
-                issuer_name=issuer.identifier))
-        except Exception:
-            if errors:
-                self.addDetail("errors", text_content("\n".join(errors)))
-            raise
+        self.assertThat(
+            macaroon.serialize(),
+            MacaroonVerifies(issuer.identifier, context, **kwargs))
 
     def assertMacaroonDoesNotVerify(self, expected_errors, issuer, macaroon,
                                     context, **kwargs):
-        errors = []
-        self.assertIsNone(issuer.verifyMacaroon(
-            macaroon, context, errors=errors, **kwargs))
+        matcher = MacaroonVerifies(issuer.identifier, context, **kwargs)
+        mismatch = matcher.match(macaroon.serialize())
+        self.assertIsNotNone(mismatch)
+        errors = mismatch.get_details()["errors"].as_text().splitlines()
         self.assertEqual(expected_errors, errors)