jpmckinney / validictory

🎓 deprecated general purpose python data validator
Other
240 stars 57 forks source link

Where in the structure is the error? #56

Closed edmund-huber closed 10 years ago

edmund-huber commented 11 years ago

FieldValidationErrors have a fieldname, which doesn't reflect where in the structure something went wrong.

I hope this illustrates my point: https://gist.github.com/edmund-huber/5835626 . If I had more than one field named 'ho' I wouldn't know which one went wrong.

jamesturk commented 11 years ago

This is certainly a challenge, and I'd be very interested in hearing ideas for more meaningful error messages here.

dmr commented 11 years ago

I see that this can be confusing but I don't have an idea how to solve this.

Error message right now:

 Value 5 for field 'ho' is not of type string

Maybe better:

 Value 5 for field data['hi']['ho'] is not of type string
jamesturk commented 10 years ago

this is in progress now

nueverest commented 10 years ago

This would definitely save me some time as well.

jamesturk commented 10 years ago

1.0.0a1 is out now, feedback very much appreciated!

jamesturk commented 10 years ago

giving myself some feedback here, what used to be:

Failed to validate field 'classification' list schema: Value 'concurrent memorial' for list item is not in the enumeration: ['bill', 'resolution', 'concurrent resolution', 'joint resolution', 'memorial']

is now

Value 'concurrent memorial' for field '[list item]' is not in the enumeration: ['bill', 'resolution', 'concurrent resolution', 'joint resolution', 'memorial']

that's a regression

dmr commented 10 years ago

The new version is working really good for my use cases. I like the new MultipleValidationError :)

I can reproduce your error message using a class like this but I think valedictory behaves correctly:

class TestArrayWithEnum(TestCase):
    def test_multi_error_regression_wrong_schema(self):
        schema = {
            "type": "object",
            "properties": {
                "name": {"type": "string"},
                "e1": {"type": "array", "enum": ["one", "two"]},
            }
        }
        data = {"name": 2, "e1": ["one", "n"]}

        # ensure it raises an error
        self.assertRaises(validictory.ValidationError, validictory.validate,
                          data, schema, fail_fast=True)

        # ensure it raises a MultiError
        self.assertRaises(validictory.MultipleValidationError, validictory.validate,
                          data, schema, fail_fast=False)

        # ensure that the MultiError has 2 errors
        try:
            validictory.validate(data, schema, fail_fast=False)
        except validictory.MultipleValidationError as mve:
            print mve
            assert len(mve.errors) == 2

        assert 0

    def test_multi_error_regression_works(self):
        schema = {
            "type": "object",
            "properties": {
                "name": {"type": "string"},
                "e2": {"type": "array", "items": {"type": "string", "enum": ["one", "two"]},
                },
            }
        }
        data = {"name": 2, "e2": ["one", "n"]}

        # ensure it raises an error
        self.assertRaises(validictory.ValidationError, validictory.validate,
                          data, schema, fail_fast=True)

        # ensure it raises a MultiError
        self.assertRaises(validictory.MultipleValidationError, validictory.validate,
                          data, schema, fail_fast=False)

        # ensure that the MultiError has 2 errors
        try:
            validictory.validate(data, schema, fail_fast=False)
        except validictory.MultipleValidationError as mve:
            print mve
            assert len(mve.errors) == 2

        assert 0

The TestArrayWithEnum.test_multi_error_regression_wrong_schema method shows the behavior you describe but I think that using enum like this is actually expected behavior. So I cannot reproduce the regression you describe.

What schema did you use?

jamesturk commented 10 years ago

glad to hear the multi-error stuff is working well.

the regression is in the message itself, it doesn't include the name of the field. i'm working on actually using the path to the field now, almost have a fix for this and i'll push out an a2

jamesturk commented 10 years ago

1.0.0a2 is now released- error messages now contain messages that contain the full path to the error, anything that doesn't should be considered a new bug and opened as such

tests.test_other.test_deep_error is a reasonable test of this behavior, but others can/should be added