Merge pull request #4752 from Zac-HD/claude/blissful-hypatia-cw4TU

Vary iteration order of fixed_dictionaries output

Author: Zac-HD

hypothesis/RELEASE.rst

@@ -0,0 +1,6 @@
+RELEASE_TYPE: minor
+
+:func:`~hypothesis.strategies.fixed_dictionaries` now varies the iteration
+order of the dicts it generates, rather than always placing the required keys
+first, to help find bugs in code which is sensitive to key order
+(:issue:`3906`). If you need a stable order, we recommend using `fixed_dictionaries(...).map(stable_sort_function)` or similar.

hypothesis/src/hypothesis/internal/conjecture/utils.py

@@ -77,6 +77,19 @@ def identity(v: T) -> T:
     return v
 
 
+def fisher_yates_shuffle(data: "ConjectureData", ls: list[T]) -> None:
+    """Shuffle ``ls`` in place, drawing from ``data``.
+
+    Reversed Fisher-Yates shuffle: swap each element with itself or with a
+    later element.  This shrinks i==j for each element, i.e. towards no change,
+    so a shuffled sequence shrinks back to its original order.  We don't
+    consider the last element as it's always a no-op.
+    """
+    for i in range(len(ls) - 1):
+        j = data.draw_integer(i, len(ls) - 1)
+        ls[i], ls[j] = ls[j], ls[i]
+
+
 def check_sample(
     values: type[enum.Enum] | Sequence[T], strategy_name: str
 ) -> Sequence[T]:

hypothesis/src/hypothesis/strategies/_internal/collections.py

@@ -396,11 +396,11 @@ def __init__(
     def do_draw(self, data: ConjectureData) -> dict[Any, Any]:
         context = current_build_context()
         arg_labels: ArgLabelsT = {}
-        value = type(self.mapping)()
+        pairs: list[tuple[Any, Any]] = []
 
         for key, strategy in self.mapping.items():
             with context.track_arg_label(str(key)) as arg_label:
-                value[key] = data.draw(strategy)
+                pairs.append((key, data.draw(strategy)))
             arg_labels |= arg_label
 
         if self.optional is not None:
@@ -416,9 +416,14 @@ def do_draw(self, data: ConjectureData) -> dict[Any, Any]:
                 remaining[-1], remaining[j] = remaining[j], remaining[-1]
                 key = remaining.pop()
                 with context.track_arg_label(str(key)) as arg_label:
-                    value[key] = data.draw(self.optional[key])
+                    pairs.append((key, data.draw(self.optional[key])))
                 arg_labels |= arg_label
 
+        # Vary the dict's iteration order (#3906).  We shuffle after choosing
+        # the optional keys, so only order varies, not the set of keys.
+        cu.fisher_yates_shuffle(data, pairs)
+        value = type(self.mapping)(pairs)
+
         if arg_labels:
             context.known_object_printers[IDKey(value)].append(
                 _fixeddict_pprinter(arg_labels, self.mapping)

hypothesis/src/hypothesis/strategies/_internal/core.py

@@ -87,6 +87,7 @@
     calc_label_from_name,
     check_sample,
     combine_labels,
+    fisher_yates_shuffle,
     identity,
 )
 from hypothesis.internal.entropy import get_seeder_and_restorer
@@ -1942,13 +1943,8 @@ def __init__(self, values):
         self.values = values
 
     def do_draw(self, data):
-        # Reversed Fisher-Yates shuffle: swap each element with itself or with
-        # a later element.  This shrinks i==j for each element, i.e. to no
-        # change.  We don't consider the last element as it's always a no-op.
         result = list(self.values)
-        for i in range(len(result) - 1):
-            j = data.draw_integer(i, len(result) - 1)
-            result[i], result[j] = result[j], result[i]
+        fisher_yates_shuffle(data, result)
         return result
 
 

hypothesis/src/hypothesis/vendor/pretty.py

@@ -975,8 +975,8 @@ def inner(obj: dict, p: RepresentationPrinter, cycle: bool) -> None:
             return p.text("{...}")
 
         get = lambda k: _get_slice_comment(p, arg_labels, k)
-        # Preserve mapping key order, then any optional keys (deduped)
-        keys = list(dict.fromkeys(k for k in [*mapping, *obj] if k in obj))
+        # Print in the dict's actual (possibly permuted) iteration order.
+        keys = list(obj)
         has_comments = any(get(k) for k in keys)
 
         with p.group(indent=4, open="{", close=""):

hypothesis/tests/cover/test_simple_collections.py

@@ -9,7 +9,6 @@
 # obtain one at https://mozilla.org/MPL/2.0/.
 
 from collections import OrderedDict
-from random import Random
 
 import pytest
 
@@ -28,7 +27,7 @@
     tuples,
 )
 
-from tests.common.debug import assert_simple_property, find_any, minimal
+from tests.common.debug import find_any, minimal
 from tests.common.utils import flaky
 
 
@@ -79,13 +78,11 @@ def test_sets_are_size_bounded(xs):
     assert 2 <= len(xs) <= 10
 
 
-def test_ordered_dictionaries_preserve_keys():
-    r = Random()
-    keys = list(range(100))
-    r.shuffle(keys)
-    assert_simple_property(
-        fixed_dictionaries(OrderedDict([(k, booleans()) for k in keys])),
-        lambda x: list(x.keys()) == keys,
+def test_fixed_dictionaries_vary_key_order():
+    keys = list(range(10))
+    find_any(
+        fixed_dictionaries({k: booleans() for k in keys}),
+        lambda x: list(x.keys()) != keys,
     )
 
 

hypothesis/tests/nocover/test_testdecorators.py

@@ -59,6 +59,6 @@ def bad_test():
 
 
 @given(data=st.data(), keys=st.lists(st.integers(), unique=True))
-def test_fixed_dict_preserves_iteration_order(data, keys):
+def test_fixed_dict_preserves_keys(data, keys):
     d = data.draw(st.fixed_dictionaries({k: st.none() for k in keys}))
-    assert all(a == b for a, b in zip(keys, d, strict=True)), f"{keys=}, {d.keys()=}"
+    assert set(d) == set(keys)