Repository: FactoryBoy/factory_boy
Pull request #536 (2018-10-30 14:08:13)

Fix final List order for lists longer than 10 items.
When creating a factory with a `List` the list items are converted into a dict, to allow easily overwriting specific items when calling the factory. For keys of the dictionary the string-converted indexes are used. When finally converted back into a list the items of the dict are sorted by the keys. However, since these are strings `'10'` comes before `'2'`. In order to preserve the correct order the keys should be converted back to integers when sorting. For Python 3.7+ (and 3.6) dictionaries are already ordered, so not additional sorting would be required, but since older Python versions are also supported sorting is still required.

I fixed it by adding a sorting key to the `sorted` function. A test is also added which checks if the order for long lists are preserved.

This does assume that classes such as `List` use param keys which are integers (but as strings).

So when using this:
`factory.List(list(range(100)))`
The old result would be:
`[0, 1, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, …, 99]`
The new result is:
`[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, …, 99]`

153957: Test failure not caused by changes in this PR, see #537 for the fix.
153957: Done and done.
francoisfreitag: Thanks! There's a typo in the new commit message:

```diff
-Add comment about changes after dropping Python >3.6 support.
+Add comment about changes after dropping Python <3.6 support.
```

I believe both commits can be squashed.
153957: Can you not do a squash merge when merging the PR?
francoisfreitag: `factory_boy` uses the "Rebase and merge" option to merge PR. I am asking to squash the commits so that the PR is in a merge-ready state. I may not be the one to merge it.

Patch:
diff --git a/README.rst b/README.rst
index c48d6b9..b8a385c 100644
--- a/README.rst
+++ b/README.rst
@@ -204,8 +204,19 @@ For this, factory_boy relies on the excellent `faker <https://faker.readthedocs.
     <User: Lucy Murray>
 
 
-.. note:: Use of fully randomized data in tests is quickly a problem for reproducing broken builds.
-          To that purpose, factory_boy provides helpers to handle the random seeds it uses.
+Reproducible random values
+""""""""""""""""""""""""""
+
+The use of fully randomized data in tests is quickly a problem for reproducing broken builds.
+To that purpose, factory_boy provides helpers to handle the random seeds it uses, located in the ``factory.random`` module:
+
+.. code-block:: python
+
+    import factory.random
+
+    def setup_test_environment():
+        factory.random.reseed_random('my_awesome_project')
+        # Other setup here
 
 
 Lazy Attributes
diff --git a/docs/fuzzy.rst b/docs/fuzzy.rst
index 987d2b9..34a174a 100644
--- a/docs/fuzzy.rst
+++ b/docs/fuzzy.rst
@@ -359,32 +359,9 @@ They should inherit from the :class:`BaseFuzzyAttribute` class, and override its
         The method responsible for generating random values.
         *Must* be overridden in subclasses.
 
+    .. warning::
 
-Managing randomness
--------------------
-
-Using :mod:`random` in factories allows to "fuzz" a program efficiently.
-However, it's sometimes required to *reproduce* a failing test.
-
-:mod:`factory.fuzzy` uses a separate instance of :class:`random.Random`,
-and provides a few helpers for this:
-
-.. method:: get_random_state()
-
-    Call :meth:`get_random_state` to retrieve the random generator's current
-    state.
-
-.. method:: set_random_state(state)
-
-    Use :meth:`set_random_state` to set a custom state into the random generator
-    (fetched from :meth:`get_random_state` in a previous run, for instance)
-
-.. method:: reseed_random(seed)
-
-    The :meth:`reseed_random` function allows to load a chosen seed into the random generator.
-
-
-Custom :class:`BaseFuzzyAttribute` subclasses **SHOULD**
-use :obj:`factory.fuzzy._random` as a randomness source; this ensures that
-data they generate can be regenerated using the simple state from
-:meth:`get_random_state`.
+        Custom :class:`BaseFuzzyAttribute` subclasses **MUST**
+        use :obj:`factory.fuzzy._random` as a randomness source; this ensures that
+        data they generate can be regenerated using the simple state from
+        :meth:`get_random_state`.
diff --git a/docs/recipes.rst b/docs/recipes.rst
index 8663158..c0f98c7 100644
--- a/docs/recipes.rst
+++ b/docs/recipes.rst
@@ -455,6 +455,56 @@ Forcing the initial value for all projects
         >>> AccountFactory.create()  # Sets up the account number based on the latest uid
         <Account uid=43, name=Test>
 
+.. _recipe-random-management:
+
+Using reproducible randomness
+-----------------------------
+
+Although using random values is great, it can provoke test flakyness.
+factory_boy provides a few helpers for this.
+
+.. note:: Those methods will seed the random engine used in both :class:`factory.Faker` and :mod:`factory.fuzzy` objects.
+
+
+Seeding the random engine
+    The simplest way to manage randomness is to push a selected seed when starting tests:
+
+    .. code-block:: python
+
+        import factory.random
+        # Pass in any value
+        factory.random.reseed_random('my awesome project')
+
+
+Reproducing unseeded tests
+    A project might choose not to use an explicit random seed (for better fuzzing),
+    but still wishes to have reproducible tests.
+
+    For such cases, use a combination of :meth:`factory.random.get_random_state()`
+    and :meth:`factory.random.set_random_state()`.
+
+    Since the random state structure is implementation-specific, we recommand passing it around
+    as a base64-encoded pickle dump.
+
+    .. code-block:: python
+
+        class MyTestRunner:
+
+            def setup_test_environment(self):
+                state = os.environ.get('TEST_RANDOM_STATE')
+                if state:
+                    try:
+                        decoded_state = pickle.loads(base64.b64decode(state.encode('ascii')))
+                    except ValueError:
+                        decoded_state = None
+                if decoded_state:
+                    factory.random.set_random_state(decoded_state)
+                else:
+                    encoded_state = base64.b64encode(pickle.dumps(factory.random.get_random_state()))
+                    print("Current random state: %s" % encoded_state.decode('ascii'))
+                super().setup_test_environment()
+
+
 
 Converting a factory's output to a dict
 ---------------------------------------
@@ -479,7 +529,7 @@ In order to get a dict, we'll just have to swap the model; the easiest way is to
             model = models.User
 
         first_name = factory.Sequence(lambda n: "Agent %03d" % n)
-        username = factory.Faker('username')
+        username = factory.Faker('user_name')
 
 .. code-block:: pycon
 
diff --git a/docs/reference.rst b/docs/reference.rst
index 0ee461e..7909c65 100644
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -311,6 +311,7 @@ Attributes and methods
                 class Meta:
                     abstract = True  # Optional
 
+                @classmethod
                 def _create(cls, model_class, *args, **kwargs):
                     obj = model_class(*args, **kwargs)
                     obj.save()
@@ -1765,7 +1766,7 @@ Decorator
 .. function:: post_generation
 
 A decorator is also provided, decorating a single method accepting the same
-``obj``, ``created``, ``extracted`` and keyword arguments as :class:`PostGeneration`.
+``obj``, ``create``, ``extracted`` and keyword arguments as :class:`PostGeneration`.
 
 
 .. code-block:: python
@@ -1777,7 +1778,7 @@ A decorator is also provided, decorating a single method accepting the same
         login = 'john'
 
         @factory.post_generation
-        def mbox(self, create, extracted, **kwargs):
+        def mbox(obj, create, extracted, **kwargs):
             if not create:
                 return
             path = extracted or os.path.join('/tmp/mbox/', self.login)
@@ -2036,3 +2037,33 @@ extracting instances from them:
     :param int size: Number of instances to generate
     :param kwargs: Declarations to use for the generated factory
     :param FACTORY_CLASS: Alternate base class (instead of :class:`Factory`)
+
+
+Randomness management
+---------------------
+
+.. currentmodule:: factory.random
+
+Using :mod:`random` in factories allows to "fuzz" a program efficiently.
+However, it's sometimes required to *reproduce* a failing test.
+
+:mod:`factory.fuzzy` and :class:`factory.Faker` share a dedicated instance
+of :class:`random.Random`, which can be managed through the :mod:`factory.random` module:
+
+.. method:: get_random_state()
+
+    Call :meth:`get_random_state` to retrieve the random generator's current
+    state.
+    The returned object is implementation-specific.
+
+.. method:: set_random_state(state)
+
+    Use :meth:`set_random_state` to set a custom state into the random generator
+    (fetched from :meth:`get_random_state` in a previous run, for instance)
+
+.. method:: reseed_random(seed)
+
+    The :meth:`reseed_random` function allows to load a chosen seed into the random generator.
+    That seed can be anything accepted by :meth:`random.seed`.
+
+See :ref:`recipe-random-management` for help in using those methods in a test setup.
diff --git a/factory/base.py b/factory/base.py
index ab89db3..9212888 100644
--- a/factory/base.py
+++ b/factory/base.py
@@ -733,7 +733,10 @@ class BaseListFactory(Factory):
             raise ValueError(
                 "ListFactory %r does not support Meta.inline_args.", cls)
 
-        values = [v for k, v in sorted(kwargs.items())]
+        # When support for Python <3.6 is dropped sorting will no longer be required
+        # because dictionaries will already be ordered, this can then be changed to:
+        # values = kwargs.values()
+        values = [v for k, v in sorted(kwargs.items(), key=lambda item: int(item[0]))]
         return model_class(values)
 
     @classmethod


Test patch:
diff --git a/tests/test_using.py b/tests/test_using.py
index f3d8f03..1480922 100644
--- a/tests/test_using.py
+++ b/tests/test_using.py
@@ -2725,6 +2725,15 @@ class ListTestCase(unittest.TestCase):
         o = TestObjectFactory()
         self.assertEqual([1], o.one)
 
+    def test_long_list(self):
+        class TestObjectFactory(factory.Factory):
+            class Meta:
+                model = TestObject
+            one = factory.List(list(range(100)))
+
+        o = TestObjectFactory()
+        self.assertEqual(list(range(100)), o.one)
+
     def test_sequence_list(self):
         class TestObjectFactory(factory.Factory):
             class Meta: