Emit missing-required-key errors in schema-declaration order (#484)

CHANGELOG.md

@@ -5,6 +5,7 @@
 **Fixes**:
 
 * [#539](https://github.com/alecthomas/voluptuous/pull/539): Raise `Invalid` instead of leaking `TypeError`/`ValueError` for non-numeric input to the `Number` validator
+* [#484](https://github.com/alecthomas/voluptuous/issues/484): Emit `required key not provided` errors in a stable schema-declaration order
 
 ## [0.16.0]
 

voluptuous/schema_builder.py

@@ -240,23 +240,25 @@ def _compile_mapping(self, schema, invalid_msg=None):
         """Create validator for given mapping."""
         invalid_msg = invalid_msg or 'mapping value'
 
-        # Keys that may be required
-        all_required_keys = set(
-            key
+        # Keys that may be required. A dict is used as an insertion-ordered set
+        # so that "required key not provided" errors are emitted in a stable,
+        # schema-declaration order rather than an arbitrary set order (#484).
+        all_required_keys = {
+            key: None
             for key in schema
             if key is not Extra
             and (
                 (self.required and not isinstance(key, (Optional, Remove)))
                 or isinstance(key, Required)
             )
-        )
+        }
 
         # Complex required keys that need special validation
-        complex_required_keys = set(
-            key
+        complex_required_keys = {
+            key: None
             for key in all_required_keys
             if isinstance(key, Required) and key.is_complex_key
-        )
+        }
 
         # Keys that may have defaults
         all_default_keys = set(
@@ -325,7 +327,7 @@ def validate_mapping(path, iterable, out):
                     errors.append(er.RequiredFieldInvalid(msg, path + [complex_key]))
                 else:
                     # If at least one candidate key is present, mark this complex requirement as satisfied
-                    required_keys.discard(complex_key)
+                    required_keys.pop(complex_key, None)
             for key, value in key_value_map.items():
                 key_path = path + [key]
                 remove_key = False
@@ -376,12 +378,12 @@ def validate_mapping(path, iterable, out):
                         # key, this means that the key was provided.
                         # Discard the required key so it does not
                         # create an additional, noisy exception.
-                        required_keys.discard(skey)
+                        required_keys.pop(skey, None)
                         break
 
                     # Key and value okay, mark as found in case it was
                     # a Required() field.
-                    required_keys.discard(skey)
+                    required_keys.pop(skey, None)
 
                     break
                 else:

voluptuous/tests/tests.py

@@ -95,6 +95,21 @@ def test_required():
         schema({})
 
 
+def test_required_key_error_order_is_stable():
+    """Missing-required-key errors follow schema declaration order (#484).
+
+    Previously the required keys were stored in a set, so the emitted
+    ``MultipleInvalid`` errors came out in an arbitrary, hash-seed-dependent
+    order. Using several keys makes an accidental correct ordering on the old
+    code astronomically unlikely.
+    """
+    keys = ['value1', 'value2', 'value3', 'value4', 'value5']
+    schema = Schema({Required(k): str for k in keys})
+    with pytest.raises(MultipleInvalid) as ctx:
+        schema({})
+    assert [e.path[0] for e in ctx.value.errors] == keys
+
+
 def test_extra_with_required():
     """Verify that Required does not break Extra."""
     schema = Schema({Required('toaster'): str, Extra: object})