From cf00bc851033dd1dac42db838ee4cabedb378e3e Mon Sep 17 00:00:00 2001
From: Ian Cordasco <graffatcolmingov@gmail.com>
Date: Tue, 7 Mar 2017 08:49:24 -0600
Subject: [PATCH] Allow users to specify a serializer easily

Betamax allows users to specify serializers without having to provide a
class that needs registration. The BetamaxFixture previously assumed
that any user would want to specify a custom serializer or just use the
built-in (for keystoneauth1) YamlJsonSerializer instead of one of the
other serializers provided by Betamax or other packages. Instead, the
BetamaxFixture now accepts `serializer_name` which allows users to
simply provide the name of the serializer they want to use, e.g.,
'json'. This also constrains the determination of the name of the
serializer specified by the user to the BetamaxFixture class and hides
it beneath a property to simplify the patcher's logic.

Change-Id: Ibed341014c864c5f557ab3e80ce6acc687781ede
Closes-bug: #1670699
---
 keystoneauth1/fixture/keystoneauth_betamax.py | 19 +++++---
 .../tests/unit/test_betamax_fixture.py        | 45 +++++++++++++++++++
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/keystoneauth1/fixture/keystoneauth_betamax.py b/keystoneauth1/fixture/keystoneauth_betamax.py
index 724d217d..38afd0ab 100644
--- a/keystoneauth1/fixture/keystoneauth_betamax.py
+++ b/keystoneauth1/fixture/keystoneauth_betamax.py
@@ -28,16 +28,26 @@ class BetamaxFixture(fixtures.Fixture):
 
     def __init__(self, cassette_name, cassette_library_dir=None,
                  serializer=None, record=False,
-                 pre_record_hook=hooks.pre_record_hook):
+                 pre_record_hook=hooks.pre_record_hook,
+                 serializer_name=None):
         self.cassette_library_dir = cassette_library_dir
         self.record = record
         self.cassette_name = cassette_name
-        if not serializer:
+        if not (serializer or serializer_name):
             serializer = yaml_serializer.YamlJsonSerializer
+            serializer_name = serializer.name
+        if serializer:
+            betamax.Betamax.register_serializer(serializer)
         self.serializer = serializer
-        betamax.Betamax.register_serializer(serializer)
+        self._serializer_name = serializer_name
         self.pre_record_hook = pre_record_hook
 
+    @property
+    def serializer_name(self):
+        if self.serializer:
+            return self.serializer.name
+        return self._serializer_name
+
     def setUp(self):
         super(BetamaxFixture, self).setUp()
         self.mockpatch = mock.patch.object(
@@ -70,8 +80,7 @@ def _construct_session_with_betamax(fixture, session_obj=None):
     if fixture.record in ['once', 'all', 'new_episodes']:
         record = fixture.record
 
-    if fixture.serializer:
-        serializer = fixture.serializer.name
+    serializer = fixture.serializer_name
 
     fixture.recorder.use_cassette(fixture.cassette_name,
                                   serialize_with=serializer,
diff --git a/keystoneauth1/tests/unit/test_betamax_fixture.py b/keystoneauth1/tests/unit/test_betamax_fixture.py
index a1632ec9..f1b03cce 100644
--- a/keystoneauth1/tests/unit/test_betamax_fixture.py
+++ b/keystoneauth1/tests/unit/test_betamax_fixture.py
@@ -12,9 +12,12 @@
 
 import testtools
 
+import betamax
 from betamax import exceptions
+import mock
 
 from keystoneauth1.fixture import keystoneauth_betamax
+from keystoneauth1.fixture import serializer
 from keystoneauth1.fixture import v2 as v2Fixtures
 from keystoneauth1.identity import v2
 from keystoneauth1 import session
@@ -58,3 +61,45 @@ class TestBetamaxFixture(testtools.TestCase):
             tenant_name=self.TEST_TENANT_NAME)
         s = session.Session()
         self.assertRaises(exceptions.BetamaxError, s.get_token, auth=plugin)
+
+
+class TestBetamaxFixtureSerializerBehaviour(testtools.TestCase):
+    """Test the fixture's logic, not its monkey-patching.
+
+    The setUp method of our BetamaxFixture monkey-patches the function to
+    construct a session. We don't need to test that particular bit of logic
+    here so we do not need to call useFixture in our setUp method.
+    """
+
+    @mock.patch.object(betamax.Betamax, 'register_serializer')
+    def test_can_pass_custom_serializer(self, register_serializer):
+        serializer = mock.Mock()
+        serializer.name = 'mocked-serializer'
+        fixture = keystoneauth_betamax.BetamaxFixture(
+            cassette_name='fake',
+            cassette_library_dir='keystoneauth1/tests/unit/data',
+            serializer=serializer,
+        )
+
+        register_serializer.assert_called_once_with(serializer)
+        self.assertIs(serializer, fixture.serializer)
+        self.assertEqual('mocked-serializer', fixture.serializer_name)
+
+    def test_can_pass_serializer_name(self):
+        fixture = keystoneauth_betamax.BetamaxFixture(
+            cassette_name='fake',
+            cassette_library_dir='keystoneauth1/tests/unit/data',
+            serializer_name='json',
+        )
+
+        self.assertIsNone(fixture.serializer)
+        self.assertEqual('json', fixture.serializer_name)
+
+    def test_no_serializer_options_provided(self):
+        fixture = keystoneauth_betamax.BetamaxFixture(
+            cassette_name='fake',
+            cassette_library_dir='keystoneauth1/tests/unit/data',
+        )
+
+        self.assertIs(serializer.YamlJsonSerializer, fixture.serializer)
+        self.assertEqual('yamljson', fixture.serializer_name)