-
Notifications
You must be signed in to change notification settings - Fork 408
Description
The problem
When initialising a factory using a Trait param, Factory Boy handles it in the same manner as any other attribute error (ie the attribute does not exist on the model. It just so happens that the Trait params have a default value and the exception is therefore handled and the default value is returned.
The problem is that the AttributeError that is raised contains the stringified version of the model. It becomes especially problematic when the factory is a django model factory using a related object. Then the AttributeError contains the stringified version of the related model. A lot of django projects override the __str__ method of models to display meaningful information in django admin and this sometimes contains data of other related models.
This means during normal factory initialisations of django model factories using Trait params it will cause undesired database calls.
The exception is raised here:
factory_boy/factory/builder.py
Lines 391 to 393 in 9b256e4
| raise AttributeError( | |
| "The parameter %r is unknown. Evaluated attributes are %r, " | |
| "definitions are %r." % (name, self.__values, self.__declarations)) |
The self.__values call calls the __repr__ method of DeclerationSet
factory_boy/factory/builder.py
Lines 133 to 134 in 68feb45
| def __repr__(self): | |
| return '<DeclarationSet: %r>' % self.as_dict() |
which returns itself as a dict, which in turns calls the __repr__ method of any object it has a reference to, which may be a django model
The exception is then handled here:
factory_boy/factory/declarations.py
Lines 191 to 195 in 68feb45
| except AttributeError: | |
| if default is _UNSPECIFIED: | |
| raise | |
| else: | |
| return default |
Proposed solution
I suggest the error message is changed from:
raise AttributeError(
"The parameter %r is unknown. Evaluated attributes are %r, "
"definitions are %r." % (name, self.__values, self.__declarations))to:
raise AttributeError(
"The parameter %r is unknown. "
"Definitions are %r." % (name, list(self.__declarations))
)Then for this factory:
class FieldFactory(factory.Factory):
name = factory.fuzzy.FuzzyText(length=5)
value = factory.fuzzy.FuzzyText(length=5)
bar = factory.SelfAttribute("foo")The error message goes from:
AttributeError: The parameter 'foo' is unknown. Evaluated attributes are {'name': 'PwkIJ', 'value': 'hcfJd'}, Definitions are <DeclarationSet: {'name': <factory.fuzzy.FuzzyText object at 0x13a34aa90>, 'value': <factory.fuzzy.FuzzyText object at 0x13a34b190>, 'bar': <SelfAttribute('foo', default=<class 'factory.declarations._UNSPECIFIED'>)>}>.
to:
AttributeError: The parameter 'foo' is unknown. Definitions are ['name', 'value', 'bar'].
Which is way more readable and useful. I don’t see any reason to include the detailed declarations and all the values already evaluated in the factory.
Another solution could be to identify Trait params and handle them differently instead of relying on try/except.
Extra notes
This was initially raised in the now closed issue #1009. I had put my discoveries and proposed solutions there (and a hacky workaround), but haven’t gotten any feedback there. So I decided to open a new issue to make sure it’s properly raised again.