Repository: FactoryBoy/factory_boy (python)

Issue:
'Options' object has no attribute 'pre_declarations' is too ambiguous
#### The problem
Typo incorrectly specifying the Model for a Factory.

#### Proposed solution
Check if any specified factories are actually Models, in which case give a more clear error message.

#### Extra notes
Thanks for the good work on this neat lib!


Pull request description:
Better error when feeding SubFactory a non-factory
Before this change, calling `factory.SubFactory(UserModel)` would raise
an unexpected error, `Options object has no attribute
'pre_declarations'`.

That message isn't very helpful; instead, a `AssociatedClassError` will
be raised, pointing at both the calling factory and called model.

This change also introduces a module for collecting tests related to the
developer experience: error messages, debugging, etc.

Closes #561.

Patch:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 0c6043a..5e919bc 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -43,6 +43,10 @@ The following aliases were removed:
     - :py:meth:`get_random_state()` now represents the state of Faker and ``factory_boy`` fuzzy attributes.
     - Add SQLAlchemy ``get_or_create`` support
 
+*Improvements:*
+
+    - :issue:`561`: Display a developer-friendly error message when providing a model instead of a factory in a :class:`~factory.declarations.SubFactory` class.
+
 *Bugfix:*
 
     - Fix issue with SubFactory not preserving signal muting behaviour of the used factory, thanks `Patrick Stein <https://github.com/PFStein>`_.
diff --git a/factory/base.py b/factory/base.py
index 2f1d122..bd00bf6 100644
--- a/factory/base.py
+++ b/factory/base.py
@@ -375,7 +375,7 @@ class FactoryOptions:
         return self.model
 
     def __str__(self):
-        return "<%s for %s>" % (self.__class__.__name__, self.factory.__class__.__name__)
+        return "<%s for %s>" % (self.__class__.__name__, self.factory.__name__)
 
     def __repr__(self):
         return str(self)
diff --git a/factory/builder.py b/factory/builder.py
index b494cc3..09153f7 100644
--- a/factory/builder.py
+++ b/factory/builder.py
@@ -225,9 +225,17 @@ class BuildStep:
         return (self.stub,) + parent_chain
 
     def recurse(self, factory, declarations, force_sequence=None):
+        from . import base
+        if not issubclass(factory, base.BaseFactory):
+            raise errors.AssociatedClassError(
+                "%r: Attempting to recursing into a non-factory object %r"
+                % (self, factory))
         builder = self.builder.recurse(factory._meta, declarations)
         return builder.build(parent_step=self, force_sequence=force_sequence)
 
+    def __repr__(self):
+        return "<BuildStep for {!r}>".format(self.builder)
+
 
 class StepBuilder:
     """A factory instantiation step.
@@ -305,6 +313,9 @@ class StepBuilder:
         """Recurse into a sub-factory call."""
         return self.__class__(factory_meta, extras, strategy=self.strategy)
 
+    def __repr__(self):
+        return "<StepBuilder(%r, strategy=%r)>" % (self.factory_meta, self.strategy)
+
 
 class Resolver:
     """Resolve a set of declarations.


Test patch:
diff --git a/tests/test_dev_experience.py b/tests/test_dev_experience.py
new file mode 100644
index 0000000..6677b08
--- /dev/null
+++ b/tests/test_dev_experience.py
@@ -0,0 +1,58 @@
+# Copyright: See the LICENSE file.
+
+"""Tests about developer experience: help messages, errors, etc."""
+
+import collections
+import unittest
+
+import factory
+import factory.errors
+
+Country = collections.namedtuple('Country', ['name', 'continent', 'capital_city'])
+City = collections.namedtuple('City', ['name', 'population'])
+
+
+class DeclarationTests(unittest.TestCase):
+    def test_subfactory_to_model(self):
+        """A helpful error message occurs when pointing a subfactory to a model."""
+        class CountryFactory(factory.Factory):
+            class Meta:
+                model = Country
+
+            name = factory.Faker('country')
+            continent = "Antarctica"
+
+            # Error here: pointing the SubFactory to a model, not a factory.
+            capital_city = factory.SubFactory(City)
+
+        with self.assertRaises(factory.errors.AssociatedClassError) as raised:
+            CountryFactory()
+
+        self.assertIn('City', str(raised.exception))
+        self.assertIn('Country', str(raised.exception))
+
+    def test_subfactory_to_factorylike_model(self):
+        """A helpful error message occurs when pointing a subfactory to a model.
+
+        This time with a model that looks more like a factory (ie has a `._meta`)."""
+
+        class CityModel:
+            _meta = None
+            name = "Coruscant"
+            population = 0
+
+        class CountryFactory(factory.Factory):
+            class Meta:
+                model = Country
+
+            name = factory.Faker('country')
+            continent = "Antarctica"
+
+            # Error here: pointing the SubFactory to a model, not a factory.
+            capital_city = factory.SubFactory(CityModel)
+
+        with self.assertRaises(factory.errors.AssociatedClassError) as raised:
+            CountryFactory()
+
+        self.assertIn('CityModel', str(raised.exception))
+        self.assertIn('Country', str(raised.exception))