Merge pull request #2617 from wence-/fix/nondeterministic-fixtures

Fix nondeterminism in fixture collection order

6 files changed, 52 insertions, 10 deletions

diff --git a/AUTHORS b/AUTHORS
index fdcb47690..7e944e478 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -91,6 +91,7 @@ Kale Kundert
 Katarzyna Jachim
 Kevin Cox
 Kodi B. Arfer
+Lawrence Mitchell
 Lee Kamentsky
 Lev Maximov
 Llandy Riveron Del Risco
diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py
index 5505fb4e1..d475dfd1b 100644
--- a/_pytest/fixtures.py
+++ b/_pytest/fixtures.py
@@ -19,6 +19,11 @@ from _pytest.compat import (
 from _pytest.runner import fail
 from _pytest.compat import FuncargnamesCompatAttr
 
+if sys.version_info[:2] == (2, 6):
+    from ordereddict import OrderedDict
+else:
+    from collections import OrderedDict
+
 
 def pytest_sessionstart(session):
     import _pytest.python
@@ -136,10 +141,10 @@ def get_parametrized_fixture_keys(item, scopenum):
     except AttributeError:
         pass
     else:
-        # cs.indictes.items() is random order of argnames but
-        # then again different functions (items) can change order of
-        # arguments so it doesn't matter much probably
-        for argname, param_index in cs.indices.items():
+        # cs.indices.items() is random order of argnames.  Need to
+        # sort this so that different calls to
+        # get_parametrized_fixture_keys will be deterministic.
+        for argname, param_index in sorted(cs.indices.items()):
             if cs._arg2scopenum[argname] != scopenum:
                 continue
             if scopenum == 0:    # session
@@ -161,7 +166,7 @@ def reorder_items(items):
     for scopenum in range(0, scopenum_function):
         argkeys_cache[scopenum] = d = {}
         for item in items:
-            keys = set(get_parametrized_fixture_keys(item, scopenum))
+            keys = OrderedDict.fromkeys(get_parametrized_fixture_keys(item, scopenum))
             if keys:
                 d[item] = keys
     return reorder_items_atscope(items, set(), argkeys_cache, 0)
@@ -196,9 +201,9 @@ def slice_items(items, ignore, scoped_argkeys_cache):
         for i, item in enumerate(it):
             argkeys = scoped_argkeys_cache.get(item)
             if argkeys is not None:
-                argkeys = argkeys.difference(ignore)
-                if argkeys:  # found a slicing key
-                    slicing_argkey = argkeys.pop()
+                newargkeys = OrderedDict.fromkeys(k for k in argkeys if k not in ignore)
+                if newargkeys:  # found a slicing key
+                    slicing_argkey, _ = newargkeys.popitem()
                     items_before = items[:i]
                     items_same = [item]
                     items_other = []
diff --git a/changelog/920.bugfix b/changelog/920.bugfix
new file mode 100644
index 000000000..d2dd2be1b
--- /dev/null
+++ b/changelog/920.bugfix
@@ -0,0 +1 @@
+Fix non-determinism in order of fixture collection.  Adds new dependency (ordereddict) for Python 2.6.
diff --git a/doc/en/getting-started.rst b/doc/en/getting-started.rst
index fb863e4e0..1571e4f6b 100644
--- a/doc/en/getting-started.rst
+++ b/doc/en/getting-started.rst
@@ -9,7 +9,8 @@ Installation and Getting Started
 
 **dependencies**: `py <http://pypi.python.org/pypi/py>`_,
 `colorama (Windows) <http://pypi.python.org/pypi/colorama>`_,
-`argparse (py26) <http://pypi.python.org/pypi/argparse>`_.
+`argparse (py26) <http://pypi.python.org/pypi/argparse>`_,
+`ordereddict (py26) <http://pypi.python.org/pypi/ordereddict>`_.
 
 **documentation as PDF**: `download latest <https://media.readthedocs.org/pdf/pytest/latest/pytest.pdf>`_
 
diff --git a/setup.py b/setup.py
index 751868c04..55607912b 100644
--- a/setup.py
+++ b/setup.py
@@ -46,11 +46,12 @@ def main():
     install_requires = ['py>=1.4.33', 'setuptools']  # pluggy is vendored in _pytest.vendored_packages
     extras_require = {}
     if has_environment_marker_support():
-        extras_require[':python_version=="2.6"'] = ['argparse']
+        extras_require[':python_version=="2.6"'] = ['argparse', 'ordereddict']
         extras_require[':sys_platform=="win32"'] = ['colorama']
     else:
         if sys.version_info < (2, 7):
             install_requires.append('argparse')
+            install_requires.append('ordereddict')
         if sys.platform == 'win32':
             install_requires.append('colorama')
 
diff --git a/testing/python/fixture.py b/testing/python/fixture.py
index 8dd713416..f8aef802f 100644
--- a/testing/python/fixture.py
+++ b/testing/python/fixture.py
@@ -2547,6 +2547,39 @@ class TestFixtureMarker(object):
             '*test_foo*alpha*',
             '*test_foo*beta*'])
 
+    @pytest.mark.issue920
+    def test_deterministic_fixture_collection(self, testdir, monkeypatch):
+        testdir.makepyfile("""
+            import pytest
+
+            @pytest.fixture(scope="module",
+                            params=["A",
+                                    "B",
+                                    "C"])
+            def A(request):
+                return request.param
+
+            @pytest.fixture(scope="module",
+                            params=["DDDDDDDDD", "EEEEEEEEEEEE", "FFFFFFFFFFF", "banansda"])
+            def B(request, A):
+                return request.param
+
+            def test_foo(B):
+                # Something funky is going on here.
+                # Despite specified seeds, on what is collected,
+                # sometimes we get unexpected passes. hashing B seems
+                # to help?
+                assert hash(B) or True
+            """)
+        monkeypatch.setenv("PYTHONHASHSEED", "1")
+        out1 = testdir.runpytest_subprocess("-v")
+        monkeypatch.setenv("PYTHONHASHSEED", "2")
+        out2 = testdir.runpytest_subprocess("-v")
+        out1 = [line for line in out1.outlines if line.startswith("test_deterministic_fixture_collection.py::test_foo")]
+        out2 = [line for line in out2.outlines if line.startswith("test_deterministic_fixture_collection.py::test_foo")]
+        assert len(out1) == 12
+        assert out1 == out2
+
 
 class TestRequestScopeAccess(object):
     pytestmark = pytest.mark.parametrize(("scope", "ok", "error"), [