Merge pull request #41 from uber/ca/fix_missing_fk_from_db

Fix missing foreign key generated from db

CHANGES.rst

@@ -1,6 +1,15 @@
 Changelog for Charlatan
 =======================
 
+0.4.1 (unreleased)
+------------------
+
+- Fixed bug where ``!rel a_database_model.id``, where ``id`` is a primary key
+  generated by the database, would be ``None`` because of how fixtures are
+  cached.
+- Removed ``as_list`` and ``as_dict`` feature. It was unnecessarily complex and
+  would not play well with caching fixtures.
+
 0.4.0 (2015-02-18)
 ------------------
 

charlatan/fixture.py

@@ -46,7 +46,7 @@ def inherit_from_parent(self):
 
     def get_parent_values(self):
         """Return parent values."""
-        parent, _ = self.fixture_manager.collection.get(self.inherit_from)
+        parent = self.fixture_manager.collection.get(self.inherit_from)
         # Recursive to make sure everything is updated.
         parent.inherit_from_parent()
 

charlatan/fixture_collection.py

@@ -1,9 +1,6 @@
-from charlatan import _compat
+from charlatan import _compat, utils
 from charlatan.fixture import Inheritable
 
-LIST_FORMAT = "as_list"
-DICT_FORMAT = "as_dict"
-
 
 def _sorted_iteritems(dct):
     """Iterate over the items in the dict in a deterministic fashion."""
@@ -53,47 +50,53 @@ def get_instance(self, path=None, overrides=None, builder=None):
         :param dict overrides:
         :param func builder:
         """
-        if not path or path in (DICT_FORMAT, LIST_FORMAT):
-            return self.get_all_instances(overrides=overrides,
-                                          format=path,
-                                          builder=builder,
-                                          )
-
-        # First get the fixture
-        fixture, remaining_path = self.get(path)
+        if not path:
+            return self.get_all_instances(overrides=overrides, builder=builder)
+
+        remaining_path = ''
+        if isinstance(path, _compat.string_types):
+            path = path.split(".")
+            first_level = path[0]
+            remaining_path = ".".join(path[1:])
+
+        # First try to get the fixture from the cache
+        instance = self.fixture_manager.cache.get(first_level)
+        if (not overrides
+                and instance
+                and not isinstance(instance, FixtureCollection)):
+            if not remaining_path:
+                return instance
+            return utils.richgetter(instance, remaining_path)
+
+        # Or just get it
+        fixture = self.get(first_level)
         # Then we ask it to return an instance.
         return fixture.get_instance(path=remaining_path,
                                     overrides=overrides,
                                     builder=builder,
                                     )
 
-    def get_all_instances(self, overrides=None, format=None, builder=None):
-        """Get all instance.
+    def get_all_instances(self, overrides=None, builder=None):
+        """Get all instances.
 
         :param dict overrides:
-        :param str format:
         :param func builder:
-        """
-        if not format:
-            format = self.default_format
-
-        if format == LIST_FORMAT:
-            returned = []
-        elif format == DICT_FORMAT:
-            returned = {}
-        else:
-            raise ValueError("Unknown format: %r" % format)
 
+        .. deprecated:: 0.4.0
+            Removed format argument.
+        """
+        returned = []
         for name, fixture in self:
             instance = fixture.get_instance(overrides=overrides,
                                             builder=builder)
+            returned.append((name, instance))
 
-            if format == LIST_FORMAT:
-                returned.append(instance)
-            else:
-                returned[name] = instance
-
-        return returned
+        if self.container is dict:
+            return dict(returned)
+        elif self.container is list:
+            return list(map(lambda f: f[1], returned))
+        else:
+            raise ValueError('Unknown container')
 
     def extract_relationships(self):
         # Just proxy to fixtures in this collection.
@@ -103,7 +106,6 @@ def extract_relationships(self):
 
 
 class DictFixtureCollection(FixtureCollection):
-    default_format = DICT_FORMAT
     iterator = staticmethod(_sorted_iteritems)
     container = dict
 
@@ -115,19 +117,13 @@ def get(self, path):
 
         :param str path:
         """
-        # Note: this can't be used with 'as_list' etc.
-
-        if isinstance(path, _compat.string_types):
-            path = path.split(".")
-
-        if not path[0] in self.fixtures:
-            raise KeyError("No such fixtures: '%s'" % path[0])
+        if path not in self.fixtures:
+            raise KeyError("No such fixtures: '%s'" % path)
 
-        return self.fixtures[path[0]], ".".join(path[1:])
+        return self.fixtures[path]
 
 
 class ListFixtureCollection(FixtureCollection):
-    default_format = LIST_FORMAT
     iterator = enumerate
     container = list
 
@@ -139,5 +135,4 @@ def get(self, path):
 
         :param str path:
         """
-        path = path.split(".")
-        return self.fixtures[int(path[0])], ".".join(path[1:])
+        return self.fixtures[int(path)]

charlatan/tests/data/relationships.yaml

@@ -1,6 +1,6 @@
 model:
   fields:
-    color: !rel relationship_alone
+    color: !rel color
     name: "toaster1"
   model: charlatan.tests.fixtures.models:Toaster
 
@@ -9,7 +9,13 @@ model_1:
   fields:
     name: "toaster2"
 
-relationship_alone:
+model_with_explicit_fk:
+  model: charlatan.tests.fixtures.models:Toaster
+  fields:
+    name: 'toaster'
+    color_id: !rel color.id
+
+color:
   fields:
     name: "red"
   model: charlatan.tests.fixtures.models:Color

charlatan/tests/fixtures/models.py

@@ -14,7 +14,7 @@ class Toaster(Base):
 
     id = Column(Integer, primary_key=True)
     name = Column(String)
-    color_name = Column(String, ForeignKey('colors.name'))
+    color_id = Column(String, ForeignKey('colors.name'))
 
     color = relationship("Color", backref='toasters')
 
@@ -23,4 +23,5 @@ class Color(Base):
 
     __tablename__ = "colors"
 
-    name = Column(String, primary_key=True)
+    id = Column(Integer, primary_key=True)
+    name = Column(String)

charlatan/tests/test_fixture_collection.py

@@ -1,8 +1,6 @@
-import mock
 import pytest
 
 from charlatan import FixturesManager
-from charlatan.tests.fixtures.simple_models import User
 
 
 def get_collection(collection):
@@ -12,7 +10,7 @@ def get_collection(collection):
     """
     manager = FixturesManager()
     manager.load("docs/examples/collection.yaml")
-    return manager.collection.get(collection)[0]
+    return manager.collection.get(collection)
 
 
 @pytest.fixture(scope="module")
@@ -21,31 +19,12 @@ def collection():
     return get_collection("toasters")
 
 
-@pytest.fixture(scope="module")
-def heterogeneous_collection():
-    """Return heterogeneous FixtureCollection."""
-    return get_collection("overridden_toasters")
-
-
 def test_repr(collection):
     """Verify the repr."""
     assert repr(collection) == "<DictFixtureCollection 'toasters'>"
 
 
-def test_cant_get_invalid_format(collection):
-    """Verify that we can't get an invalid format."""
-    with pytest.raises(ValueError):
-        collection.get_all_instances(format="invalid")
-
-
 def test_cant_get_missing_fixture(collection):
     """Verify that we can't get a missing fixture."""
     with pytest.raises(KeyError):
         collection.get("missing")
-
-
-def test_collection_members_can_override_model(heterogeneous_collection):
-    """Verify that we can override the inherited model for a collection."""
-    builder = mock.Mock()
-    heterogeneous_collection.get_instance("user", builder=builder)
-    assert builder.call_args[0][1] is User

charlatan/tests/test_fixtures_manager.py

@@ -9,76 +9,84 @@
 from charlatan import FixturesManager
 
 
+def test_overrides_and_in_cache():
+    manager = FixturesManager()
+    manager.load('./docs/examples/simple_fixtures.yaml')
+    # Add it to the cache
+    manager.install_fixture("toaster")
+    toaster = manager.install_fixture("toaster", overrides={"color": "blue"})
+    assert toaster.color == 'blue'
+
+
 class TestFixturesManager(testing.TestCase):
 
     def test_load_two_files(self):
         """Verify we can load two files."""
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load(
+        manager = FixturesManager()
+        manager.load(
             './charlatan/tests/data/relationships_without_models.yaml')
-        fixtures_manager.load(
+        manager.load(
             './charlatan/tests/data/simple.yaml')
-        assert 'simple_dict' in fixtures_manager.keys()
+        assert 'simple_dict' in manager.keys()
 
     def test_install_fixture(self):
         """install_fixture should return the fixture."""
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load(
+        manager = FixturesManager()
+        manager.load(
             './charlatan/tests/data/relationships_without_models.yaml')
 
-        fixture = fixtures_manager.install_fixture('simple_dict')
+        fixture = manager.install_fixture('simple_dict')
 
         self.assertEqual(fixture, {
             'field1': 'lolin',
             'field2': 2,
         })
 
     def test_get_all_fixtures(self):
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load('./charlatan/tests/data/simple.yaml')
-        assert len(fixtures_manager.get_all_fixtures()) == 1
+        manager = FixturesManager()
+        manager.load('./charlatan/tests/data/simple.yaml')
+        assert len(manager.get_all_fixtures()) == 1
 
     def test_uninstall_all_fixtures(self):
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load('./charlatan/tests/data/simple.yaml')
-        assert len(fixtures_manager.install_all_fixtures()) == 1
-        fixtures_manager.uninstall_all_fixtures()
-        assert len(fixtures_manager.installed_keys) == 0
+        manager = FixturesManager()
+        manager.load('./charlatan/tests/data/simple.yaml')
+        assert len(manager.install_all_fixtures()) == 1
+        manager.uninstall_all_fixtures()
+        assert len(manager.installed_keys) == 0
 
     @freeze_time("2014-12-31 11:00:00")
     def test_install_fixture_with_now(self):
         """Verify that we can install a fixture with !now tag."""
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load('./charlatan/tests/data/simple.yaml')
-        fixture = fixtures_manager.install_fixture('fixture')
+        manager = FixturesManager()
+        manager.load('./charlatan/tests/data/simple.yaml')
+        fixture = manager.install_fixture('fixture')
         self.assertEqual(fixture,
                          {'now': datetime(2014, 12, 30, 11, 0)})
 
     def test_install_fixture_override(self):
         """Verify that we can override a fixture field."""
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load('./charlatan/tests/data/simple.yaml')
-        fixture = fixtures_manager.install_fixture('fixture',
-                                                   overrides={'now': None})
+        manager = FixturesManager()
+        manager.load('./charlatan/tests/data/simple.yaml')
+        fixture = manager.install_fixture('fixture', overrides={'now': None})
         self.assertEqual(fixture, {'now': None})
 
     def test_uninstall_fixture(self):
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load(
+        manager = FixturesManager()
+        manager.load(
             './charlatan/tests/data/relationships_without_models.yaml')
 
-        fixtures_manager.install_fixture('simple_dict')
-        fixtures_manager.uninstall_fixture('simple_dict')
+        manager.install_fixture('simple_dict')
+        manager.uninstall_fixture('simple_dict')
 
         # verify we are forgiving with list inputs
-        fixtures_manager.install_fixtures('simple_dict')
-        fixtures_manager.uninstall_fixtures('simple_dict')
+        manager.install_fixtures('simple_dict')
+        manager.uninstall_fixtures('simple_dict')
 
     def test_uninstall_non_installed_fixture(self):
-        fixtures_manager = FixturesManager()
-        fixtures_manager.load(
+        manager = FixturesManager()
+        manager.load(
             './charlatan/tests/data/relationships_without_models.yaml')
-        fixtures_manager.uninstall_fixture('simple_dict')
+        manager.uninstall_fixture('simple_dict')
 
     def test_dependency_parsing(self):
         fm = FixturesManager()

charlatan/tests/test_lists_of_fixtures.py

@@ -1,4 +1,5 @@
 from __future__ import absolute_import
+import operator as op
 
 from charlatan import testing
 from charlatan import FixturesManager
@@ -18,7 +19,6 @@ def test_get_list_by_name(self):
 
     def test_one_to_many_relationship(self):
         """Verify that relations to lists of fixtures work"""
-
         fixture = self.fm.install_fixture('related_fixture')
         self.assertEqual(
             fixture['elements'],
@@ -27,9 +27,6 @@ def test_one_to_many_relationship(self):
 
     def test_override(self):
         """Verify that we can override attributes on a list of fixtures."""
-        fixtures = self.fm.install_fixture(
-            'fixture_list',
-            overrides={"field1": 12})
-
-        for fixture in fixtures:
-            self.assertEqual(fixture["field1"], 12)
+        fixtures = self.fm.install_fixture('fixture_list',
+                                           overrides={"field1": 12})
+        assert list(map(op.itemgetter('field1'), fixtures)) == [12, 12]