Fix issues based on @charlax review

charlatan/fixtures_manager.py

@@ -252,13 +252,16 @@ def uninstall_fixture(self, fixture_key):
 
         :param str fixture_key:
 
-        :rtype: :data:`fixture_instance`
+        :rtype: :data:`fixture_instance` or None if no instance was installed
+        with the given key
         """
 
         try:
             self._get_hook("before_uninstall")()
-            instance = self.get_fixture(fixture_key)
-            self.delete_instance(instance)
+            instance = self.cache.get(fixture_key)
+            if instance:
+                self.delete_instance(instance)
+                self.cache.pop(fixture_key, None)
 
         except Exception as exc:
             self._get_hook("after_uninstall")(exc)
@@ -269,7 +272,10 @@ def uninstall_fixture(self, fixture_key):
             return instance
 
     def uninstall_fixtures(self, fixture_keys):
-        """Uninstall a list of fixtures.
+        """Uninstall a list of installed fixtures.
+
+        If a given fixture was not previously installed, nothing happens and
+        its instance is not part of the returned list.
 
         :param fixture_keys: fixtures to be uninstalled
         :type fixture_keys: str or list of strs
@@ -283,17 +289,18 @@ def uninstall_fixtures(self, fixture_keys):
         instances = []
         for fixture_key in reversed(fixture_keys):
             instance = self.uninstall_fixture(fixture_key)
-            instances.append(instance)
+            if instance:
+                instances.append(instance)
 
         return instances
 
-    def uninstall_all_fixtures(self,):
-        """Uninstall all fixtures.
+    def uninstall_all_fixtures(self):
+        """Uninstall all installed fixtures.
 
         :rtype: list of :data:`fixture_instance`
         """
 
-        return self.uninstall_fixtures(self.fixtures.keys())
+        return self.uninstall_fixtures(self.cache.keys())
 
     def get_fixture(self, fixture_key, include_relationships=True, attrs=None):
         """Return a fixture instance (but do not save it).
@@ -437,7 +444,8 @@ def install_all_fixtures(self, do_not_save=False,
     @copy_docstring_from(FixturesManager)
     def uninstall_fixture(self, fixture_key):
         instance = FIXTURES_MANAGER.uninstall_fixture(fixture_key)
-        delattr(self, fixture_key, instance)
+        if instance:
+            delattr(self, fixture_key, instance)
         return instance
 
     @copy_docstring_from(FixturesManager)
@@ -452,7 +460,7 @@ def uninstall_fixtures(self, fixture_keys):
 
     @copy_docstring_from(FixturesManager)
     def uninstall_all_fixtures(self):
-        self.uninstall_fixtures(FIXTURES_MANAGER.fixtures.keys())
+        self.uninstall_fixtures(FIXTURES_MANAGER.cache.keys())
 
     def clean_fixtures_cache(self):
         """Clean the cache."""

charlatan/testcase.py

@@ -14,7 +14,7 @@ def use_fixtures_manager(self, fixtures_manager):
         if hasattr(self, "fixtures"):
             self.install_fixtures(self.fixtures)
 
-    def install_fixtures(self, fixtures, do_not_save=False):
+    def install_fixtures(self, fixtures=None, do_not_save=False):
         """Install required fixtures.
 
         :param fixtures: fixtures key
@@ -25,11 +25,7 @@ def install_fixtures(self, fixtures, do_not_save=False):
         """
 
         if fixtures:
-            # Be forgiving
-            if not isinstance(fixtures, (list, tuple)):
-                fixtures = (fixtures, )
-            fixtures_to_install = fixtures
-
+            fixtures_to_install = FixturesMixin._make_list(fixtures)
         else:
             fixtures_to_install = self.fixtures
 
@@ -62,8 +58,8 @@ def get_fixture(self, fixture_name):
         """
         return self.__fixtures_manager.get_fixture(fixture_name)
 
-    def uninstall_fixtures(self, fixtures):
-        """Uninstall required fixtures.
+    def uninstall_fixtures(self, fixtures=None):
+        """Uninstall fixtures.
 
         :param fixtures: fixtures key
         :type fixtures: list of strings
@@ -73,24 +69,27 @@ def uninstall_fixtures(self, fixtures):
         """
 
         if fixtures:
-            # Be forgiving
-            if not isinstance(fixtures, (list, tuple)):
-                fixtures = (fixtures, )
-            fixtures_to_uninstall = fixtures
-
+            fixtures_to_uninstall = FixturesMixin._make_list(fixtures)
         else:
-            fixtures_to_uninstall = self.fixtures
+            fixtures_to_uninstall = self.__fixtures_manager.cache
 
         uninstalled = self.__fixtures_manager.uninstall_fixtures(
             fixtures_to_uninstall)
 
         # Removing fixtures from the class
-        for fixture_name, fixture in zip(fixtures_to_uninstall, uninstalled):
-            delattr(self, fixture_name)
+        for fixture in uninstalled:
+            delattr(self, fixture.key)
 
         # Return list of fixture instances
         return uninstalled
 
     def uninstall_fixture(self, fixture_name):
         """Uninstall a fixture and return it."""
         return self.uninstall_fixtures(fixture_name)[0]
+
+    @staticmethod
+    def _make_list(obj):
+        """Return list of objects if necessary."""
+        if isinstance(obj, (list, tuple)):
+            return obj
+        return (obj, )

charlatan/tests/test_fixtures_manager.py

@@ -36,6 +36,42 @@ def test_uninstall_fixture(self):
             'field2': 2,
         })
 
+    def test_uninstall_non_installed_fixture(self):
+        """
+        uninstall_fixture should return None if the fixture has not been
+        previously installed.
+        """
+
+        fixtures_manager = FixturesManager()
+        fixtures_manager.load(
+            './charlatan/tests/data/relationships_without_models.yaml')
+
+        fixture = fixtures_manager.uninstall_fixture('simple_dict')
+        self.assertEqual(fixture, None)
+
+    def test_uninstall_fixtures(self):
+        """
+        uninstall_fixtures should return the list of previously installed
+        fixtures that are now uninstalled.
+        """
+        fixtures_manager = FixturesManager()
+        fixtures_manager.load(
+            './charlatan/tests/data/relationships_without_models.yaml')
+
+        fixture_keys = ('simple_dict', 'dict_with_nest')
+
+        fixtures_manager.install_fixtures(fixture_keys)
+        self.assertEqual(len(fixtures_manager.cache.keys()), 2)
+
+        fixtures = fixtures_manager.uninstall_fixtures(fixture_keys)
+        self.assertEqual(len(fixtures), 2)
+        self.assertEqual(len(fixtures_manager.cache.keys()), 0)
+
+        # uninstalling non-exiting fixtures should not raise an exception
+        fixtures = fixtures_manager.uninstall_fixtures(fixture_keys)
+        self.assertEqual(len(fixtures), 0)
+        self.assertEqual(len(fixtures_manager.cache.keys()), 0)
+
     def test_dependency_parsing(self):
         fm = FixturesManager()
         fm.load(

docs/quickstart.rst

@@ -65,8 +65,8 @@ Using fixtures
 
 There are multiple ways to require and use fixtures.
 
-For each tests, in setUp
-""""""""""""""""""""""""
+For each tests, in setUp and tearDown
+"""""""""""""""""""""""""""""""""""""
 
 .. code-block:: python
 
@@ -76,16 +76,21 @@ For each tests, in setUp
             # This will create self.client and self.driver
             self.install_fixtures(("client", "driver"))
 
+        def tearDown(self):
+            # This will delete self.client and self.driver
+            self.uninstall_fixtures(("client", "driver"))
+
 For a single test
 """""""""""""""""
 
 .. code-block:: python
 
-  class MyTest(FixturesMixin):
-
-      def test_toaster(self):
-          self.install_fixtures("toaster")
+    class MyTest(FixturesMixin):
 
+        def test_toaster(self):
+            self.install_fixtures("toaster")
+            # do things... and optionally uninstall it once you're done
+            self.uninstall_fixtures("toaster")
 
 Getting a fixture without saving it
 """""""""""""""""""""""""""""""""""